westes / flex

The Fast Lexical Analyzer - scanner generator for lexing in C and C++
Other
3.54k stars 529 forks source link

gitignore doc/flex and doc/flex.exe #647

Closed Explorer09 closed 3 months ago

Explorer09 commented 3 months ago

Following commit 7e53fea7dc086dec34522e73f442705879773a9d, 'flex' executable program might now reside in doc directory. Add the file names to gitignore list.

westes commented 3 months ago

I had a look at help2man documentation. Their suggestion assumes that the Makefile building the man pages is in the same directory where the binary is built. That sounds like a good idea. That would reduce the complexity back to the MSWindows insistance on $(EXEEXT).

Explorer09 commented 3 months ago

@westes

Their suggestion assumes that the Makefile building the man pages is in the same directory where the binary is built. That sounds like a good idea. That would reduce the complexity back to the MSWindows insistance on $(EXEEXT).

I think for flex's case, the complexity isn't on where the binary is built or invoked, but the cross-directory dependency that is difficult to handle with the "recursive makefile" structure. (That's one of the reasons against recursive makefile and favor non-recursive ones.)

The aim of my patch 7e53fea7dc086dec34522e73f442705879773a9d is to make the man page generation deterministic, so that the Reproducible Builds people could be happy.

Mightyjo commented 3 months ago

That's a noble goal. Stripping away the autotools/libtool build system is not the way to achieve it. Lay out what you're trying to achieve in an issue so we can discuss it as a group.

On Fri, May 3, 2024, 07:04 Kang-Che Sung (宋岡哲) @.***> wrote:

@westes https://github.com/westes

Their suggestion assumes that the Makefile building the man pages is in the same directory where the binary is built. That sounds like a good idea. That would reduce the complexity back to the MSWindows insistance on $(EXEEXT).

I think for flex's case, the complexity isn't on where the binary is built or invoked, but the cross-directory dependency that is difficult to handle with the "recursive makefile" structure. (That's one of the reasons against recursive makefile and favor non-recursive ones.)

The aim of my patch 7e53fea https://github.com/westes/flex/commit/7e53fea7dc086dec34522e73f442705879773a9d is to make the man page generation deterministic, so that the Reproducible Builds https://reproducible-builds.org/ people could be happy.

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

Explorer09 commented 3 months ago

@Mightyjo

That's a noble goal. Stripping away the autotools/libtool build system is not the way to achieve it.

I'm not sure what you are talking about. I didn't strip anything about Autotools/Libtool build system.

Mightyjo commented 3 months ago

I realize you don't see it. Maybe that's why we're butting heads. Your changes favor Make idioms over Automake's and Libtool's. You want repeatable builds but you don't see how relying on Make suffix rules impedes that goal.

Biggest thing, and this is no one's fault, you're trying to work around the fact that the distribution tarball is bootstrapping stage1flex. That isn't supposed to happen. The automake prefix rules you removed recently were part of the plumbing to check whether bootstrapping was needed and prevent it in the distribution tarball. They weren't working when you removed them, but they can't work if they're gone.

I wish I'd noticed the build system was broken sooner so I could have saved us all a bunch of trouble.

On Fri, May 3, 2024, 10:08 Kang-Che Sung (宋岡哲) @.***> wrote:

@Mightyjo https://github.com/Mightyjo

That's a noble goal. Stripping away the autotools/libtool build system is not the way to achieve it.

I'm not sure what you are talking about. I didn't strip anything about Autotools/Libtool build system.

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

Explorer09 commented 3 months ago

@Mightyjo

You want repeatable builds but you don't see how relying on Make suffix rules impedes that goal.

Explanation please? Because I don't see any "impedance".

The "bootstrap" in flex does not refer to the bootstrapping of the flex program (that's unnecessary), but only bootstrapping of the scanner code. Flex's scanner code requires a version of itself to be rebuilt. The other parts of the code are in C/C++ and no bootstrap are needed there.

Biggest thing, and this is no one's fault, you're trying to work around the fact that the distribution tarball is bootstrapping stage1flex. That isn't supposed to happen. The automake prefix rules you removed recently were part of the plumbing to check whether bootstrapping was needed and prevent it in the distribution tarball. They weren't working when you removed them, but they can't work if they're gone.

Nope. The file name prefixes in Automake is nothing to do with bootstrapping. stage1flex should always be built unless user configures with --disable-bootstrap. This behavior was intended in the beginning (of #129). The fact that distribution tarball comes the pregenerated scanner (scan.c) should not stop the build system from building stage1flex or the scanner (named stage1scan.c here to distinguish the scanner pregenerated from the distribution). This was working as intended, and I don't think it should change for one reason: User might build the scanner (stage1scan.c) with custom LFLAGS, and we should respect them.

Flex bootstrap diagram

Explorer09 commented 3 months ago

@Mightyjo Off-topic. But I think what you were trying to achieve is to skip building stage1flex when the scanner is present in the distribution. Something like including the stage1-generated scanner in the distribution tarball. Here's the hack that might work for you, but I won't submit it as a PR. The lack of stage1scan.c in the distribution is not a bug for me.

--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -56,7 +56,7 @@ nodist_flex_SOURCES = \
        $(SKELINCLUDES)

 if ENABLE_BOOTSTRAP
-nodist_flex_SOURCES += stage1scan.c
+flex_SOURCES += stage1scan.c
 else
 flex_SOURCES += scan.l
 endif
Mightyjo commented 3 months ago

@Mightyjo

You want repeatable builds but you don't see how relying on Make suffix rules impedes that goal.

Explanation please? Because I don't see any "impedance".

Quick discussion: We support at least three Make variants (GNU, Mac, ... Sun used to be a close third), each with their own quirks. Automake and Libtool compensate for those quirks so we don't have to think about them. The Make utilities themselves are fairly consistent, but they are more tightly coupled to their systems' build toolchains than we want when writing a project that works everywhere. With suffix rules, the commands that generate an object are ephemeral and may be suppressed entirely. We may not be able to predict what they'll be.

Automake and Libtool generate a big Makefile.in that uses Make for dependency tracking and directly calls the build tools that Autoconf finds. With Automake and Libtool's rules, the commands that generate an object are distributed with the sources (modulo the results of ./configure) and can be statically audited at will.

For a thorough discussion: Automake manual section 2.1 Introducing the GNU Build System

The "bootstrap" in flex does not refer to the bootstrapping of the flex program (that's unnecessary), but only bootstrapping of the scanner code. Flex's scanner code requires a version of itself to be rebuilt. The other parts of the code are in C/C++ and no bootstrap are needed there.

Strongly agree. I've been confused why we seem to disagree about this but I see the problem now. Thanks for the diagram.

Flex bootstrap diagram

I see now. What you call 'stage1scan.c' I called 'scan.c'. That helps.

Nope. The file name prefixes in Automake is nothing to do with bootstrapping.

The overridden prefix rules were involved in bootstrapping when I wrote them, but it looks like they were broken when you found them. The prefixes were needed because Libtool and Automake add them when you use target-specific CFLAGS, LFLAGS, LDFLAGS, etc. variables. If you then want to override any Automake generated rules regarding an object file you have to specify it with the target prefix. I called this a "best practice" the other day. The best practice is naming your targets, not using the prefixes for overriding object file rules.

stage1flex should always be built unless user configures with --disable-bootstrap. This behavior was intended in the beginning (of #129). The fact that distribution tarball comes the pregenerated scanner (scan.c) should not stop the build system from building stage1flex or the scanner (named stage1scan.c here to distinguish the scanner pregenerated from the distribution).

Thanks to your diagram, I agree with you. Rephrasing my position in terms of your names, your "scan.c" is what I've been thinking of as "stage1scan.c". I used "scan_stage1.c" as the name of the bootstrapped source file that I didn't distribute. That's what's been confusing me.

This was working as intended, and I don't think it should change for one reason: User might build the scanner (stage1scan.c) with custom LFLAGS, and we should respect them.

Okay, I'm on board with this now. Is LFLAGS is automake's 'lex' program flags? Do we do the same for YFLAGS?

@Mightyjo Off-topic. But I think what you were trying to achieve is to skip building stage1flex when the scanner is present in the distribution. Something like including the stage1-generated scanner in the distribution tarball. Here's the hack that might work for you, but I won't submit it as a PR. The lack of stage1scan.c in the distribution is not a bug for me.

--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -56,7 +56,7 @@ nodist_flex_SOURCES = \
        $(SKELINCLUDES)

 if ENABLE_BOOTSTRAP
-nodist_flex_SOURCES += stage1scan.c
+flex_SOURCES += stage1scan.c
 else
 flex_SOURCES += scan.l
 endif

I think I'm following your logic, now. In your name scheme, it's scan.c that I wouldn't include in the tarball. Your stage1scan.c is the file I would absolutely include.

I want to skip building up from scan.c and parse.c (I'll check the naming on this, too) in the distribution tarball. My reasoning: We maintainers configured and built stage1scan.c and the parser source when we made the tarball. Users shouldn't need build them again in part because we don't want users to install old Flex and Bison packaged to build a newer Flex and in part because we want to know how flex itself behaves when we're supporting users. I.e. If we send re-entrant flex sources into the world that's the API we will expect to support.

Thanks for talking this through with me.

Explorer09 commented 3 months ago

@Mightyjo

Just FYI, you used to name what is now "stage1scan.c" as "scan_stage1.c" (in PR #20 and 26e46589a87f9b4fb502a052e425e2f8b2763ef1). The naming wasn't changed significantly, but I hope this clarify things. (When you look at my diagram again.)

Mightyjo commented 3 months ago

@Explorer09 Updated my blob up above. Meant to start a new comment but I haven't had enough coffee this morning.

Explorer09 commented 3 months ago

@Mightyjo

I saw your updated comment.

Quick discussion: We support at least three Make variants (GNU, Mac, ... Sun used to be a close third), each with their own quirks. Automake and Libtool compensate for those quirks so we don't have to think about them. The Make utilities themselves are fairly consistent, but they are more tightly coupled to their systems' build toolchains than we want when writing a project that works everywhere. With suffix rules, the commands that generate an object are ephemeral and may be suppressed entirely. We may not be able to predict what they'll be.

The suffix rules are POSIX. It should work regardless of the make implementation you have. Automake made it so that suffix rules like .c.o (generating any foo.o file from foo.c) and .l.c (generating any foo.c file from foo.l) are all explicit. So no dependency on implicit rules of any make implementation. What we really depend on is Automake, not make of any implementation.

Speaking of make implementation, I was recently trying to build flex with bmake (NetBSD make) shipped with Ubuntu, to test how many GNU make specific syntaxes that flex's makefiles still depend on. (With GitHub Actions CI, I can test building these all remotely. :) )

Strongly agree. I've been confused why we seem to disagree about this but I see the problem now. Thanks for the diagram.

You're welcome.

I think I'm following your logic, now. In your name scheme, it's scan.c that I wouldn't include in the tarball. Your stage1scan.c is the file I would absolutely include.

I want to skip building up from scan.c and parse.c (I'll check the naming on this, too) in the distribution tarball. My reasoning: We maintainers configured and built stage1scan.c and the parser source when we made the tarball. Users shouldn't need build them again in part because we don't want users to install old Flex and Bison packaged to build a newer Flex and in part because we want to know how flex itself behaves when we're supporting users. I.e. If we send re-entrant flex sources into the world that's the API we will expect to support.

Hmm. When I looked at the flex bootstrap procedure (that's years ago), I thought the rebuilding of the flex scanner ("stage1scan.c" here) was intentional at the start. I kept this vision in mind when I made patches that improved the bootstrap procedure. Now when I look at it, I don't want it to be changed, with the LFLAGS reason I stated.

Okay, I'm on board with this now. Is LFLAGS is automake's 'lex' program flags? Do we do the same for YFLAGS?

Section "Yacc and Lex support" in Automake manual

The LFLAGS and YFLAGS are standard in GNU build system already, with one caveat: Since the parser and scanner code are pregenerated and including in distribution tarball by default already (Autotools default; they assume the users build machine don't come with lex or yacc). The parsers and scanners won't get rebuilt automatically unless the users force them by deleting the pregenerated scanner and parser files.

As flex introduced the bootstrap, we allowed user to rebuild flex's internal scanner without requiring a version of flex pre-installed on the users machine. And when flex's internal scanner is getting rebuilt, it would be natural to respect the users' LFLAGS.

(And by the way, the generation of flex's "scan.c" and "stage1scan.c" are all deterministic now. If the user specifies empty LFLAGS during the bootstrap build, the "scan.c" and "stage1scan.c" would be bit-identical. You can try verifying that yourself.)

westes commented 3 months ago

On MSWindows, if we call "flex" and what exists is "flex.exe", won't it still run? (assuming it's got enough directory specified or that it's on the path)?

Explorer09 commented 3 months ago

@westes

On MSWindows, if we call "flex" and what exists is "flex.exe", won't it still run? (assuming it's got enough directory specified or that it's on the path)?

Yes it would run. (Not sure about the context why you ask this question.)

westes commented 3 months ago

Thanks for submitting this. I made the whole "even if cross compiling" and "even on MSWindows" things work in a little different way and included this there, so it is on master as a part of that work.