Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: Add option to always automatically regenerate ncgen source #2822

Merged
merged 5 commits into from
Mar 21, 2024

Conversation

ZedThree
Copy link
Contributor

As part of this:

  • ensure flex and bison are available
  • run commands from top level directory so #line directives in generated files include the ncgen subdirectory -- gives better tracing of warnings
  • include sed commands for fixing filename in #line directives

I found that when trying to fix some warnings from ncgen.y that it was quite difficult to correctly regenerate ncgeny.c, and I think this was basically because the custom command didn't list all the files it creates. So this PR fixes that, as well as make it easier to regenerate these files -- turning on the option GENERATE_NCGEN causes CMake to automatically regenerate them as part of the build process if required.

I've not done the same thing for ncgen3/ncgen.y as it looks like that hasn't been regenerated in some time and would result in significant changes. @WardF should I regenerate it too?

Also:
- ensure `flex` and `bison` are available
- run commands from top level directory so `#line` directives in
  generated files include the `ncgen` subdirectory -- gives better
  tracing of warnings
- include `sed` commands for fixing filename in `#line` directives
@ZedThree
Copy link
Contributor Author

I think the test failures are spurious:

The following tests FAILED:
	 21 - ncdump_tst_output (Failed)
	 22 - ncdump_tst_nccopy3 (Failed)

as they only failed in one configuration, and they then pass when immediately rerun

@WardF WardF self-assigned this Dec 11, 2023
@WardF
Copy link
Member

WardF commented Dec 19, 2023

This looks good; I may change the option name to keep it consistent with the autotools-based system (from which make makeparser was inherited. As long as the option remains off by default (it's poor practice to include frequently-changed generated files in version control) we should be good. Thanks!

* main: (209 commits)
  Bump Visual Studio appveyor, for the brief period before we swap out to GitHub actions.
  Make a change in support of Unidata#2879
  Revert "fix cmake build with ENABLE_HDF4 and hdf requiring jpeg"
  removing need for global compile definition
  fix cmake build with ENABLE_HDF4 and hdf requiring jpeg
  Removed assumption that we are linking against static HDF5 when building a static library. While it's reasonable to provide a mechanism to specify this, it is not necessarily true. We should also perhaps rename the NC_FIND_SHARED option, since LIBS implies it will look for static or shared libraries for all dependencies, but this logic only looks for HDF5. In any case, commenting this out for now until we can rework it.
  Re-adding global add-definition for the time being.  Its lack introduces an unnecessary roadblock (at the moment).  Re-formulated logic for determining what tests to run when.  Need to figure out why plugins are turned off when MINGW is true, but that's a different issue.  As of this push, all tests succeed on local windows system.
  Update cmake-based plugins and test logic.
  Modify messages to be more clear.
  setting dll export on each target
  Correct lingering compilation issue under Visual Studio. Hopefully I haven't broken the Linux build
  Addressing a handful of issues encountered in Visual Studio re: linking, setting compiler flags for VC, etc.
  removing c++ header file from c header
  Corrected a dependencies issue linking against libcurl and finding curl/curl.h using Visual Studio.  There's another issue to correct, but this is getting us a lot closer.
  Correct(?) syntax with target_compile_options() in top level CMakeLists.txt.  Correct the logic flow in libncxml/CMakeLists.txt to not try to include non-existant directory when libxml2 is not found.
  removing unused cmake
  updating tests to use correct cmake variable
  removing possibly redundant cmake for shared library
  updating release notes
  bumping cmake version
  ...
@ZedThree
Copy link
Contributor Author

Just to note that in the autotools build, makeparser is a target that is always available that can be used to manually update the generated files.

Here, this is a configure-time option to enable automatic regeneration of the files. As you note, it's off by default, as this bit of the code rarely changes and means that users don't need to have flex and bison installed normally.

NETCDF_ prefix. Thanks!

Merge remote-tracking branch 'upstream/main' into cmake-ncgen-generate
@WardF WardF added this to the 4.9.3 milestone Mar 21, 2024
@WardF WardF merged commit 2849b70 into Unidata:main Mar 21, 2024
103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants