removed windows-specific headers

[sergiud]

Eliminate Windows specific headers:

• Avoids duplication headers which should be platform agnostic
• Reduces maintenance burden by requiring changes in a single file only

[sergiud]

@drigz I would like to remove Windows specific header files which Bazel requires. Is there a way to avoid this?

[drigz]

Let me take a look at whether I can expand the header templates without introducing a new dependency to the Windows+Bazel build. Since the reference to hash/hash_compare in preprocess.sh appears to be a red herring, I think we can get away with fixed-string-replacements, so this could be possible.

[sergiud]

@drigz For some reason, AppVeyor builds do not show up under merge checks anymore. Do you happen to know what is the problem?

[drigz]

I'm afraid I'm not familiar with AppVeyor.

[sergiud]

I believe this is a permission problem in the Github project configuration which I don't have access to. Could you check? (I assume you can see the settings since you're a group member).

[sergiud]

@drigz I would like to drop windows/config.h as well. Is there a way to reuse the config.h generated by Bazel?

[drigz]

I believe this is a permission problem in the Github project configuration which I don't have access to. Could you check? (I assume you can see the settings since you're a group member).

I can see the Settings but I can't see anything problematic: AppVeyor is listed under https://github.com/google/glog/settings/installations, and https://www.appveyor.com/blog/2018/10/02/github-apps-integration/ suggests nothing else is required. Do you have access to the AppVeyor account/UI? Maybe that would have more info.

I would like to drop windows/config.h as well. Is there a way to reuse the config.h generated by Bazel?

Why do you want to remove it? It should be possible but I expect it would be more complicated than just leaving the file there.

[sergiud]

@drigz Thanks for checking. Unfortunately, I don't have access to the AppVeyor account. I believe only google members have. I tried contacting @ukai already. However, without a response so far.

Regarding src/windows/config.h: it duplicates the one generated from src/config.h.cmake.in. Since Bazel already uses the latter as a template to generate a config.h, I don't see any reason for keeping a Windows specific configuration header anymore. With my limited Bazel knowledge, however, I could not find a way to reuse the generated header for Windows Bazel builds. I believe the required changes are actually trivial.

In the long term, I'd like to remove Windows specific workarounds anyway particularly once we switch to C++17. Keeping the above header is therefore not an option.

I've added a rationale for the PR in my initial comment.

[drigz]

Unfortunately, I don't have access to the AppVeyor account.

I tried logging into AppVeyor with my GitHub SSO - I couldn't see any existing projects, though - maybe something got deleted. I found googleapis/google-cloud-python#6253 which could be related. Do you know what AppVeyor is needed/used for?

In the long term, I'd like to remove Windows specific workarounds anyway particularly once we switch to C++17. Keeping the above header is therefore not an option.

The non-trivial element is that windows/config.h sets GOOGLE_GLOG_DLL_DECL - do you have any plans to change how the DLL macros are set/interpreted? Or should I look for a way to set that in the Bazel windows build that matches the current windows/config.h?

[sergiud]

Actually, CMake already generates an export.h which selects the definitions for *_DLL_DECL based on the used toolchain. Which is also a reason why I would like to avoid yet another config.h which currently overrides CMake definitions.

For Bazel, this does need to be this complicated. You can use something along the lines of

glog/bazel/glog.bzl

Line 149 in 82320a8

"@bazel_tools//src/conditions:windows": ["GOOGLE_GLOG_DLL_DECL=__declspec(dllexport)"],

I do not intend to change how these macros are interpreted.

As for AppVeyor, we use it for Windows CI builds. Travis builds only for Linux. It seems, @google-admin manages the glog AppVeyor account.