Skip to content
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

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

Closed
wants to merge 1 commit into from

Conversation

Explorer09
Copy link
Contributor

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

Following commit 7e53fea, 'flex'
executable program might now reside in doc directory.
Add the file names to gitignore list.
@westes
Copy link
Owner

westes commented May 3, 2024

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
Copy link
Contributor Author

@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 is to make the man page generation deterministic, so that the Reproducible Builds people could be happy.

@Mightyjo
Copy link
Contributor

Mightyjo commented May 3, 2024 via email

@Explorer09
Copy link
Contributor Author

@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
Copy link
Contributor

Mightyjo commented May 3, 2024 via email

@Explorer09
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Contributor

Mightyjo commented May 3, 2024

@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
Copy link
Contributor Author

@Mightyjo

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

@Mightyjo
Copy link
Contributor

Mightyjo commented May 3, 2024

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

@Explorer09
Copy link
Contributor Author

@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
Copy link
Owner

westes commented May 3, 2024

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
Copy link
Contributor Author

Explorer09 commented May 3, 2024

@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
Copy link
Owner

westes commented May 9, 2024

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.

@westes westes closed this May 9, 2024
@Explorer09 Explorer09 deleted the doc-gitignore branch May 9, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants