-
Notifications
You must be signed in to change notification settings - Fork 262
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
Use classic names for bison generated files (in oc2/ only) #2925
base: main
Are you sure you want to change the base?
Conversation
aa18d4e
to
6af65f2
Compare
6af65f2
to
b64569e
Compare
The makeparser action in Makefile is normally only carried out when the .y file changes.
You should verify that your changes do not require bison to be installed. |
They still don't require it, there are no changes in CMake / Makefiles logic. One still can regenerate the files by running
(the latter actually suggests using automake, but simple make works as well for the purpose). |
Did some testing. The current situation is that dap.tab.[ch] is deleted automatically by make DISTCLEAN, |
I assumed that cmake is the modern way to build netcdf-c, isn't it? CMake does not have any instructions for generating bison code on the fly, it just uses the files from the repo. |
Just my $0.02 that this is why you don't check in any generated files into git. The source distribution should certainly contain them if you'd like to avoid users needing to build them, but that should be done as part of the release process. |
There are a few irregularly-regenerated netCDF files; these are not generated with any regularity, and are checked in to version control because they always have been, from back in the pre-me subversion days. These are the files targeted by
Can you elaborate on this? I'm trying to suss out what the problem is that is being solved, at the cost of introducing additional complexity into our version control system. I'm not suggesting there isn't any value here, I'm just trying to weigh one against the other, especially as I'm trying to juggle some stuff to get the next release out already :). @DennisHeimbigner Re: a user submitting a PR, it will be incumbent upon us to review any PR's to make sure a user hasn't pushed a PR that deletes necessary files. But hopefully we've been doing that so far, and if they were to push such a PR, presumably the CI would fail, as these files are not generated automatically. @dopplershift These generated files are different than most typical use cases, insofar as they aren't generated with any regularity, but are instead generated due to the underlying combinatorics of data type conversions, testing, etc. So they are generated as opposed to written by hand (since the combinatorics follow a predictable pattern), but I think they are rightfully excepted from the 'don't keep generated files in version control' rule, especially since as it stands, there is no user dependency on That said, it's possible the files are generated if they are missing in the first place, or if the target is manually invoked. Either way, since what we have works, and is transparent to the end user, and system-portable, I'm just curious about what is being addressed from a prioritization standpoint :). Thanks all! |
We are using custom build system (yatool, to be precise) which provides user with high-level primitives as it knows how to build many specific file formats (including native support for The user just lists all the sources and everything else is done by the build system itself.
Passing
I don't quite see any additional complexity here. Could you explain me where this PR adds to it? |
@WardF, lets proceed with the discussion / merging of this PR? |
I just noticed that you made the change for the .y file in oc2. |
There is still no approval from the upstream if they are ready to apply this change at all. I might proceed with this PR once it will get some degree of approval. |
I did an experiment of applying this change to a second bison .y files (dce.y). |
This simplifies the command a bit and makes it easier to repeat the build routine in a different build system.