Fix getopt.in.h for MSVC

kiyolee commented 11 months ago

Pull request checklist

Please check if your PR fulfills the following requirements:

[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been reviewed and added / updated if needed (for bug fixes / features)
[X] Build was run locally and without warnings or errors
[X] All previous and new tests pass

Pull request type

Please check the type of change your PR introduces: - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting, renaming, typo fix) - [ ] Refactoring (no functional changes, no api changes) - [X] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior?

cmake build with MSVC failed as "getopt.in.h" included which does not exist for MSVC.

Related Issue URL:

What is the new behavior?

cmake build with MSVC succeeded.

Does this introduce a breaking change?

[ ] Yes
[X] No

Other information

kiyolee commented 11 months ago

Would you consider renaming "getopt.in.h" to "getopt.h"? There is nothing in "getopt.in.h" that needs to be generated. The generated "getopt.h" by cmake build is exactly the same as "getopt.in.h".

Larhzu commented 11 months ago

Would you consider renaming "getopt.in.h" to "getopt.h"?

getopt.in.h is done like that in Gnulib. A package that uses Gnulib may have many .in.h files. Which files will be needed is detected when running configure and copied to .h name. Gnulib has replacements for many system headers to aid portability. With the .in.h -> .h method only the specificic headers can be overriden.

Obviously XZ Utils only include getopt from Gnulib at the moment. In the early days I didn't know if more modules would be needed. So moving the getopt files to lib/getopt/ and putting that to include path when needed could be fine if we are certain that the module list won't grow. If many Gnulib modules (or similar things from other sources) were needed then this wouldn't work because the include path would grow too long and the modules can have intermodule dependencies too.

I will discuss this with Jia. We plan to update getopt code with the current Gnulib too (it's still LGPLv2.1 so no license changes).

I have seen your other messages. I will get back to them later this week.

Larhzu commented 11 months ago

The getopt.in.h change is in xz_for_msvc branch now. Thanks!

tukaani-project / xz