Use classic names for bison generated files (except for ncgen3/)

[georgthegreat]

This simplifies the command a bit and makes it easier to repeat the build routine in a different build system.

[CLAassistant]

All committers have signed the CLA.

[DennisHeimbigner]

The makeparser action in Makefile is normally only carried out when the .y file changes.
The renaming was done to avoid the builder having to have bison installed. Using the original *.tab.[ch]
names would, as I recall, have required this because:

1. autoconf will forcibly delete files of the name .tab. (I assume it still does)
2. the forcible deletion means that the person building the parser has to have bison installed.

You should verify that your changes do not require bison to be installed.

[georgthegreat]

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.
I assume this is also confirmed by 106 successful checks from the CI.

One still can regenerate the files by running

cd netcdf-c/os2
make -f Makefile.am makeparser

(the latter actually suggests using automake, but simple make works as well for the purpose).

[DennisHeimbigner]

Did some testing. The current situation is that dap.tab.[ch] is deleted automatically by make DISTCLEAN,
but not make CLEAN.
So, we have to decide: it is likely that a random user will execute DISTCLEAN; probably not.
So our assumption now becomes that if some user does DISTCLEAN, then he knows enough
to install bison and flex. This seems reasonable to me.
But there is a danger here. I could imagine some user doing DISTCLEAN just before pushing a PR
for some proposed change as a cleanup action. In that case, the *.tab.[ch] files will be missing
from the PR when they should be there.
Also, this change should be propagated to the other .y and .l files in e.g. ncgen.
Also, we need to document this bison requirement somewhere.

[georgthegreat]

The current situation is that dap.tab.[ch] is deleted automatically by make DISTCLEAN,
but not make CLEAN.

I assumed that cmake is the modern way to build netcdf-c, isn't it?
Or are you supporting both?

CMake does not have any instructions for generating bison code on the fly, it just uses the files from the repo.

[dopplershift]

But there is a danger here. I could imagine some user doing DISTCLEAN just before pushing a PR
for some proposed change as a cleanup action. In that case, the *.tab.[ch] files will be missing
from the PR when they should be there.

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.

[WardF]

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 makeparser.

This simplifies the command a bit and makes it easier to repeat the build routine in a different build system.

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 bison unless they want build the makeparser target.

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!

[georgthegreat]

Can you elaborate on this?

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 .y grammars).

The user just lists all the sources and everything else is done by the build system itself.
What I trying to achieve is to simplify netcdf-c description to the following:

SRCS(
    oc2/daplex.c
    oc2/dapparse.c
    oc2/dap.y
    oc2/oc.c
    ...
)

Passing oc2/dap.y will run bison under the hood, catch it output (known to the build system) and then compile this output.
While the build system is customisable, every customisation comes at a cost which I am trying to avoid to removing every customisation from netcdf-c itself.

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 don't quite see any additional complexity here. Could you explain me where this PR adds to it?
From my point of view everything got (a bit) more simple than is was.

[georgthegreat]

@WardF, lets proceed with the discussion / merging of this PR?

[DennisHeimbigner]

I just noticed that you made the change for the .y file in oc2.
You need to apply the same change to all the other .y files
in the netcdf-c code base. This would be to ensure that
no conflicts arise.

[georgthegreat]

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.

[DennisHeimbigner]

I did an experiment of applying this change to a second bison .y files (dce.y).
I wanted to make sure that there were no potential name conflicts, and there are none.
So, we can do this change (extended to all the .y files in the netcdf code base.
My caveat above about DISTCLEAN vs CLEAN still applies, however.

[georgthegreat]

@DennisHeimbigner, I have fixed three dirs: oc2, libdap2 and ncgen.

As for ncgen3, I would like to get is updated in a separate PR, after merging this one into main branch.
Juggling this many interconnected changes (and jumping accross the machines to find a fresh bison binary) is a bit complicated to me.

[georgthegreat]

@WardF @DennisHeimbigner, can we proceed with merging this?

[DennisHeimbigner]

The open question is still the DISTCLEAN issue and requiring bison+flex.
Suppose we kept copies of the dap.tab.[ch] files (and the flex equivalents)
in the repository and releases.
Then if someone accidentally deleted their copies, they would have a backup
location to get them without having to install bison+flex.
BTW did you extend your changes to include the flex file output?

[georgthegreat]

The open question is still the DISTCLEAN issue and requiring bison+flex.

Why should this project cover the bugs in another one?
And, btw, CMake is the default choice, isn't it?

«Building with autotools» in the project readme results in 404 and nobody cares.

BTW did you extend your changes to include the flex file output?

No, I did not.
I already spent some time without getting this merged.

One I will get this merged, I can apply these changes to the remaining outputs.