fix(tests): tests/Makefile unnecessarily regenerates #653
Conversation
|
Example build log of what I called "dependency tracking error" The error part: EDIT: Oops. It seems like I mixed two |
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 regeneration 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.)
|
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:
#653
Commit Summary
- a24d0db
<a24d0db>
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
<#653>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVJXILOJSCFTEMAOE6CTC3ZCBZOPAVCNFSM6AAAAABHTWZOXCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI4TEMJUGQZTQMA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
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. |
|
@westes 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: |
|
@westes Excuse me again, but if I proposed a workaround to the problem, would you guys accept it? |
|
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
<#653 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVJXINXAVCK22GC5JSH4K3ZCDJ65AVCNFSM6AAAAABHTWZOXCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBXHEZTSMZUGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Let's not think in terms of workarounds. Let's actually express the dependencies and build process correctly. |
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 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.) |
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 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 |
|
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. |
|
@Mightyjo You have already explained clear enough. But my concern is usability. If Many developers (from other projects) don't even know what |
|
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 |
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.
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 |
|
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) |
@Mightyjo No. That wasn't what I mean. |
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. |
|
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. |
|
@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. |
|
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. |
|
@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. |
|
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. |
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. |
|
"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. |
tests/ruleset.amshould not depend on the modification dates of the*.rulesfiles in the directory.bmake(NetBSD make; also available in some Linux distros), can occasionally treatruleset.amas 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.amonly depends on the names and existences of the*.rulesfiles, not their contents or modification dates.)