rename GOOGLE_GLOG_DLL_DECL to GLOG_EXPORT

[sergiud]

Use a consistent naming and avoid platform specific terms. Also remove copy and paste code.

@drigz Is there a way to define GLOG_EXPORT=__declspec(dllimport) for Windows builds only when glog is being used (as opposed to being compiled)?

[codecov-commenter]

Codecov Report

Merging #764 (24cdfde) into master (aa94e6b) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #764   +/-   ##
=======================================
  Coverage   72.78%   72.78%           
=======================================
  Files          17       17           
  Lines        3259     3259           
=======================================
  Hits         2372     2372           
  Misses        887      887           

Impacted Files	Coverage Δ
src/glog/logging.h.in	80.00% <ø> (ø)
src/logging.cc	73.70% <ø> (ø)
src/windows/port.h	100.00% <ø> (ø)
src/vlog_is_on.cc	70.22% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa94e6b...24cdfde. Read the comment docs.

[drigz]

Is there a way to define GLOG_EXPORT=__declspec(dllimport) for Windows builds only when glog is being used (as opposed to being compiled)?

I'm aware that copts affects only the build actions for glog targets, and defines affects both glog actions and actions for targets that depend on glog.

IIUC you are asking if there is something that affects targets that depend on glog but not glog actions themselves. If so, I'm not aware of anything. You could see if copts overrides defines, and if not, you could try something like copts = ["-DACTUALLY_PLEASE_IGNORE_GLOG_EXPORT", ...] and adapt the source code accordingly. Does that answer your question?

[sergiud]

I'd like to avoid defining export macros in headers. Would it be possible to define GLOG_EXPORT multiple times such that its last definition occurence wins (which should be GLOG_EXPORT=__declspec(dllimport)?

Also, I'm wondering why the Bazel build succeeds in the first place as there's no __declspec(dllimport) currently defined. Does Bazel build a static library version of glog?

[drigz]

I'm afraid I know very little about how the Windows build works, and I don't understand the effect of the dll* strings.

Would it be possible to define GLOG_EXPORT multiple times such that its last definition occurence wins (which should be GLOG_EXPORT=__declspec(import)?

I tried the following:

cc_library(
    # ...
    defines = ["FOO=one"] + ...,
    copts = ["-UFOO", "-DFOO=two"] + ...,
    # ...
)

This led to the following command line:

/usr/bin/gcc ... '-DFOO=one' ... -UFOO '-DFOO=two' ... -c src/symbolize.cc -o bazel-out/k8-fastbuild/bin/_objs/glog/symbolize.pic.o

So I think that pattern should work if you want to use different settings for the two actions.

[sergiud]

@drigz Does Bazel always build a static library of glog? I'm asking because building the Bazel glog example from current master produces several linker errors on Windows related to gflags if I add linkstatic = False to the cc_test rule.

[drigz]

@drigz Does Bazel always build a static library of glog? I'm asking because building the Bazel glog example from current master produces several linker errors on Windows related to gflags if I add linkstatic = False to the cc_test rule.

That's surprising to me, but I know very little about the behavior on Windows. For cc_test, I'd expect dynamic linking by default (IIRC it does this for faster incremental build/test cycles) whereas cc_binary is mostly-static by default (ie gflags linked statically, libc linked dynamically). I checked this on Linux and it seems to be correct: building a cc_test uses something like gcc -o main main.pic.o -lglog -lgflags -lstdc++ ... whereas the cc_binary uses gcc -o main main.pic.o glog/*.pic.o gflags/*.pic.o -lstdc++ ....

If this is blocking you, send me a repro and I can take a look, it might also be worth trying the bazel-discuss mailing list.

[sergiud]

@drigz For some reason, AppVeyor builds are active again and cause failing checks. I guess, these should be removed completely.

[drigz]

I don't know the "right way" to disable AppVeyor. I ticked a box "Skip branches without appveyor.yml" which will hopefully do the trick.

[sergiud]

Unfortunately, that did not help.

[drigz]

It seems that I don't have access to the top-level project: I can't change settings at https://ci.appveyor.com/project/google-admin/glog.

IIRC I "added" glog to my AppVeyor account when trying to address your last request: I've deleted that, which might put us back where we were before. I'll see if I can find out anything more useful.

[sergiud]

@drigz Any updates?

[drigz]

No response, just tried a different list so we'll see if that helps. I also spotted the failed checks on this project are from @ukai's project, so maybe they can disable that.

If this doesn't work, we could try to figure out how the checks get started: there's no webhook enabled on this repo, but maybe there's a webhook or polling enabled at some other level...

[sergiud]

Thanks for the update!

[sergiud]

@drigz Seems to be working again.

I would merge this PR as is. PTAL and let me know if you have any concerns.