fix(tests): tests/Makefile unnecessarily regenerates

[Explorer09]

tests/ruleset.am should not depend on the modification dates of the *.rules files in the directory. bmake (NetBSD make; also available in some Linux distros), can occasionally treat ruleset.am as outdated and unnecessarily regenerates it, causing a chain of regeneraion of Makefiles or even dependency tracking error.

(This might look like a bug in bmake's side, but we should remove false dependencies in makefiles anyway. ruleset.am only depends on the names and existences of the *.rules files, not their contents or modification dates.)

[Explorer09]

Example build log of what I called "dependency tracking error" https://github.com/Explorer09/flex/actions/runs/9055071761/job/24875643475

The error part:

Making install in tests
[...]
make[1]: Entering directory '/home/runner/work/flex/flex/tests'
[...]
( cd . && /bin/bash ruleset.sh nr r c99 go >ruleset.am )
 cd .. && /bin/bash /home/runner/work/flex/flex/build-aux/missing automake-1.16 --foreign tests/Makefile
 cd .. && /bin/bash ./config.status tests/Makefile depfiles
config.status: creating tests/Makefile
config.status: executing depfiles commands
config.status: error: in `/home/runner/work/flex/flex':
config.status: error: Something went wrong bootstrapping makefile fragments
    for automatic dependency tracking.  If GNU make was not used, consider
    re-running the configure script with MAKE="gmake" (or whatever is
    necessary).  You can also try re-running configure with the
    '--disable-dependency-tracking' option to at least be able to build
    the package (albeit without support for automatic dependency tracking).
See `config.log' for more details
make[1]: *** [Makefile:3507: Makefile] Error 1
make[1]: Leaving directory '/home/runner/work/flex/flex/tests'
make: *** [Makefile:1832: install-recursive] Error 1

EDIT: Oops. It seems like I mixed two make implementations (GNU make & BSD make) during the make install test in the example build log. The top-level makefile handled by gmake, and tests/Makefile handled by bmake. My point about the false dependencies and unnecessary makefile regeneration still stands though.

[Mightyjo]

It's a real dependency. When the rules change the build system usually needs an autoreconf.

If you take this out you'll introduce bugs you haven't encountered yet.

@westes Recommend rejecting this.

On Mon, May 13, 2024, 04:18 Kang-Che Sung (宋岡哲) @.***> wrote:

tests/ruleset.am should not depend on the modification dates of the *.rules files in the directory. bmake (NetBSD make; also available in some Linux distros), can occasionally treat ruleset.am as outdated and unnecessarily regenerates it, causing a chain of regeneraion of Makefiles or even dependency tracking error.

(This might look like a bug in bmake's side, but we should remove false dependencies in makefiles anyway. ruleset.am only depends on the names and existences of the *.rules files, not their contents or modification dates.)

You can view, comment on, or merge this pull request online at:

https://github.com/westes/flex/pull/653 Commit Summary

• a24d0db https://github.com/westes/flex/pull/653/commits/a24d0db3e3b0dafb70e16ce02ece0ad39ddb2619 fix(tests): tests/Makefile unnecessarily regenerates

File Changes

(1 file https://github.com/westes/flex/pull/653/files)

• M tests/Makefile.am https://github.com/westes/flex/pull/653/files#diff-32103f666ff2fb42b025a47ccf1b959bbcc6db89f217e5943b1de73c81a4f9db (2)

Patch Links:

• https://github.com/westes/flex/pull/653.patch
• https://github.com/westes/flex/pull/653.diff

— Reply to this email directly, view it on GitHub https://github.com/westes/flex/pull/653, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVJXILOJSCFTEMAOE6CTC3ZCBZOPAVCNFSM6AAAAABHTWZOXCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4TEMJUGQZTQMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[westes]

I'm thinking that we'll introduce something overly subtle given what this file is doing. So let's not muck with this right now.

[Explorer09]

@westes I don't like when @Mightyjo says that "it's a real dependency" without explaining in detail why the problem I described is invalid.

Because I see it breaks in my build, and not the theoretical "if I do this it might introduce bugs" thing.

And please look at my comment and explain it a bit: ruleset.am and ruleset.sh currently only depend on the names of the *.rules file. What are you going to introduce so that the contents of the rules would also depend on then?

[Explorer09]

@westes Excuse me again, but if I proposed a workaround to the problem, would you guys accept it? A workaround I have in mind is to force regeneration of tests/ruleset.am in 'autogen.sh', or just "touch" the ruleset.am file, so that it doesn't look out of date, before running 'autoreconf'.

[Mightyjo]

I'll explain after work

On Mon, May 13, 2024, 11:12 Kang-Che Sung (宋岡哲) @.***> wrote:

@westes https://github.com/westes Excuse me again, but if I proposed a workaround to the problem, would you guys accept it? A workaround I have in mind is to force regeneration of tests/ruleset.am in 'autogen.sh', or just "touch" the ruleset.am file, so that it doesn't look out of date, before running 'autoreconf'.

— Reply to this email directly, view it on GitHub https://github.com/westes/flex/pull/653#issuecomment-2107939344, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVJXINXAVCK22GC5JSH4K3ZCDJ65AVCNFSM6AAAAABHTWZOXCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBXHEZTSMZUGQ . You are receiving this because you were mentioned.Message ID: @.***>

[westes]

Let's not think in terms of workarounds. Let's actually express the dependencies and build process correctly.

[Mightyjo]

I'll explain after work

Looking at tests/ruleset.sh:39 we see that ruleset.am is build from all the *.rules files in the test/ directory. Therefore, we have to rebuild ruleset.am whenever those RULESETS files change. Neither Make nor Automake recognize this dependency from RULESETS' presence in EXTRA_DIST because files named only in EXTRA_DIST aren't tracked as dependencies. Looking at tests/Makefile.am:440 we see that rulesets.am is included into the Makefile. So we have a bootstrap problem and sometimes need to run autoreconf during a build of the test suite. However, this is rare in practice. It should only happen when a developer is editing the rules files and executing make check without reconfiguring the source tree before each build (because autoreconf will do it for them).

We can't avoid a reconfig when Makefile.am is out of date. It's not a common occurrence in practice, though. I can force it to happen first thing in the build process if that's preferable.

(edit: fixed weird formatting error.)

[Explorer09]

Looking at tests/ruleset.sh:39 we see that ruleset.am is build from all the *.rules files in the test/ directory. Therefore, we have to rebuild ruleset.am whenever those RULESETS files change. Neither Make nor Automake recognize this dependency from RULESETS' presence in EXTRA_DIST because files named only in EXTRA_DIST aren't tracked as dependencies. Looking at tests/Makefile.am:440 we see that rulesets.am is included into the Makefile. So we have a bootstrap problem and sometimes need to run autoreconf during a build of the test suite. However, this is rare in practice. It should only happen when a developer is editing the rules files and executing make check without reconfiguring the source tree before each build (because autoreconf will do it for them).

We can't avoid a reconfig when Makefile.am is out of date. It's not a common occurrence in practice, though. I can force it to happen first thing in the build process if that's preferable.

The problem I am facing is that, during a git checkout or clone, both the "*.rules" files and the ruleset.am may be updated, but the timing of the files might be confusing to make, and make could see ruleset.am as being outdated. Normally for developers, seeing ruleset.am as well as Makefile.am being rebuilt is not a problem, but for people who build from a checkout (nightly build), they expect the ruleset.am should be up to date, and that Makefile.am doesn't need to rebuild unnecessarily.

Since our goal is to ensure ruleset.am being up to date, after a git checkout, we should make the regeneration of ruleset.am part of the ./autogen.sh process. That way people who build from the checkout won't need to worry about this. And I would argue this is what autogen.sh is for (prepare everything that is needed to generate makefiles and "configure" script).

[Mightyjo]

After a checkout, it's assumed ruleset.am is outdated. It's always necessary to rebuild tests/Makefile.am. It's a "bootstrap" problem, but not the same one as in src/.

I can't explain this any more clearly.

Out.

[Explorer09]

@Mightyjo You have already explained clear enough. But my concern is usability. If autogen.sh wasn't there for handling the job, then the usual workflow of ./autogen.sh && ./configure && make to build the project would be broken. Developers / builders now have to remember one extra thing to do to keep the build system up to date; that's a cognitive load.

Many developers (from other projects) don't even know what autoreconf is when it sees a project using Autotools. Even though most Autotools build systems can be regenerated using a autoreconf command, it is a good practice to put it in autogen.sh (or "bootstrap", which is GNU preferred naming of the script) so that developers can be free from that cognitive load and do the real development.

[Mightyjo]

They don't have to remember anything right now. It happens automatically. If we did accept your patch anyone working on the test suite would have to remember to run autogen.sh each time they update the rules, before make check. Otherwise they wouldn't be running the tests they just wrote.

[Mightyjo]

Many developers (from other projects) don't even know what autoreconf is when it sees a project using Autotools.

I'm going to say this plainly, but I'm not directing it at you. I don't care at all what developers who don't understand Autotools say about an Autotools build system. It's the preferred GNU build system. If they can't learn it they can work on other parts of the project.

Even though most Autotools build systems can be regenerated using a autoreconf command, it is a good practice to put it in autogen.sh (or "bootstrap", which is GNU preferred naming of the script) so that developers can be free from that cognitive load and do the real development.

I don't know what you're talking about. Autoreconf is automatically called for the out of date Makefile.am as required. Developers just run make <target> and it's done.

[Mightyjo]

Quick follow up. I love your code on the whole.

When I gripe about developers who blah blah blah I'm thinking about the people who turn up every year telling us to re-write the build system in cmake.

(edit: -grow +gripe)

[Explorer09]

They don't have to remember anything right now. It happens automatically. If we did accept your patch anyone working on the test suite would have to remember to run autogen.sh each time they update the rules, before make check. Otherwise they wouldn't be running the tests they just wrote.

@Mightyjo No. That wasn't what I mean. I mean to keep this line in tests/Makefile.am intact but add another line in autogen.sh so the ruleset.am file can be rebuilt in either way. While I personally think the current ruleset.am mechanism is fragile in terms of the build system, that's the problem to be solved the other day. For now, I just want to see the unnecessary rebuilding of ruleset.am file go away (caught on CI multiple times, at least in my repo fork) -- by making it up to date prior to generaing "configure".

[Explorer09]

When I gripe about developers who blah blah blah I'm thinking about the people who turn up every year telling us to re-write the build system in cmake.

Off-topic. While I don't have a say whether Flex would introduce cmake in it's build system, if it's me I won't object. The point is people made good contributions and lots of work to make Flex build in cmake; those efforts deserve respect. There are packages that aim to support both Autotools and cmake; either build system have its merits and weaknesses, and it could be beneficial if a project supports both.

[westes]

I keep hearing how wonderful cmake is. Then each time I have investigated it, it has lacked some utterly basic feature that flex's build system depends on.

[Explorer09]

@westes Well, the point isn't whether cmake is wonderful, it's how people can solve limitations of cmake to fit Flex's requirements. No build systems are perfect, and what I appreciate is contributions that can solve limitations of any build system. People won't file up PR or patches if they turn out they don't work. That's my point, at least.

[westes]

cmake adds nothing to flex. Cmake lacks basic features that flex needs. People keep requesting cmake even though it has been repeatedly pointed out that cmake is worthless from flex's point of view.

[Explorer09]

@westes Well, think in the perspective of a contributor. If we don't keep one PR about CMake support alive, then contributors would gonna waste time reworking on the same things. Also, it isn't obvious why CMake doesn't fit Flex's build requirements. If Flex is going to permanently reject CMake, the best I would do is to state the reasons (technical ones, not "it's hard to maintain" kind of reasons that are irrelevant to contributors), and preferably put them in the documentation of the source tree, which at least can tell other developers / contributors to stop trying.

[westes]

The first test that any would-be contribution to a project needs to pass is: Does the proposed material actually contribute something to the project that is relevant to the project. To date, none of the cmake proposals have done so. Additionally, they've come from people who don't understand the flex build systems needs and goals. I'm not interested in diverting my limited time to e.g. releasing a new version of flex and building the infrastructure to make flex releases easier to generate to explaining to people the fundamentals of contributing to a project something that is actually relevant to that project. "Is this relevant" is such a basic and obvious question that if one doesn't think to ask it, I'm not convinced that a reminder to do so will help.

[Explorer09]

The first test that any would-be contribution to a project needs to pass is: Does the proposed material actually contribute something to the project that is relevant to the project. [...] "Is this relevant" is such a basic and obvious question that if one doesn't think to ask it, I'm not convinced that a reminder to do so will help.

No offense, but I don't like the attitude suggesting someone's time is more limited than others. And for the question about "is this relavent", I would personally reword the question to this: Relavent to whom? Those who contribute at least know something is relevant to themselves, or else they don't bother contributing at all. If upstream maintainers reject patches without good reasons, downstream developers would gonna maintain forks on their own to keep whatever is relevant to them relevant and up to date. Ultimately it waste their time anyway and would give an impression that upstream is unfriendly. Not a good attitude for developing FOSS.

[westes]

"relevant" to the project to which they're proporting to contribute.

My time is limited. I have to choose how to allocate my time. Cmake, to date, has only proven to be unnecessary and unhelpful to flex. The alledged benefits turn out not to exist in fact.

Flex's build system works and the cmake patches have 1: not added anything that flex is missing and 2: have (due to cmake itself, not the quality of the patches we've received) created feature gaps that cmake might or might not fill in in the future.

The only response I have observed to my pointing out these facts is an assertion that cmake is wonderful, the next big thing or some other such thing that does not benefit the flex project.

The entire point of free and open-source software is that people can maintain their own forks if that is needful for their use cases. I'm ecstatic if people see an opportunity and take it up.