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

build: make it possible to disable the build of the flex program #256

Closed
wants to merge 1 commit into from

Conversation

tpetazzoni
Copy link

The flex program uses fork(), which isn't available on noMMU
systems. However, the libfl library does not use fork(), and be used
by other programs/libraries.

Therefore, it makes sense to provide an option to disable the build of
the flex program.

Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com

The flex program uses fork(), which isn't available on noMMU
systems. However, the libfl library does not use fork(), and be used
by other programs/libraries.

Therefore, it makes sense to provide an option to disable the build of
the flex program.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
@Explorer09
Copy link
Contributor

I'm considering that building libfl alone should be the corner case, and I don't think having a config option for disabling that benefit much (comparing to the additional Makefile code).
And this doesn't stop configure from checking fork() anyway.

Is there any problem if you run configure like normal, and then do make -C src libfl.a to make only the library?

And by the way, the code in libfl is soooo trivial - you might consider not linking with libfl at all in your code.

@tpetazzoni
Copy link
Author

Buildroot is a embedded Linux build system, which packages a lot of different upstream components. Therefore, we do not decide to link with libfl or not: it's the decision of various upstream components. Currently in Buildroot, we have quite a few packages in this situation: at, checkpolicy, gnuchess, ipsec-tools, libcue, linux-pam, radvd.

Would you accept a patch that instead of adding a configuration option, detects if fork() is available, and disable the building of the flex program in this case ?

Thanks!

@westes
Copy link
Owner

westes commented Aug 26, 2017 via email

@tpetazzoni
Copy link
Author

OK. Let me know if I need to change my patch, or if it is good as it is. Thanks!

@Explorer09
Copy link
Contributor

But what about the testsuite? You probably need to disable the testsuite as well when flex is not built.

@tpetazzoni Can you describe the problem in detail? I still don't understand what really happened in Buildroot. Are there really upstream packages that link with -lfl? And then what's exactly the case that you need to build libfl alone?

I'm considering that dependency to libfl is the worst idea for package's portability. IIRC, libfl is only there for POSIX compatibility 1 and any package should provide replacements instead of depending on it. (%option noyywrap or even a simple -D'yywrap()=((int)1)' would work in place of -lfl most of the time.)

@westes
Copy link
Owner

westes commented Aug 27, 2017 via email

@Explorer09
Copy link
Contributor

@westes What I'm saying is that make check would fail. And if there are automated build systems that do run make check before make install, you know what would happen.

@westes
Copy link
Owner

westes commented Aug 27, 2017 via email

@Explorer09
Copy link
Contributor

I'm still not convinced. I would personally add a make libfl target in the top level makefile rather than complicating the configure script like that. (And probably also make install-libfl -- it's trivial to implement both in Makefile.)
fork() should not be the only reason for not building flex anyway, because ideally we should be able to make flex work with vfork(). And vfork is available in a no-MMU system, isn't it?

@tpetazzoni
Copy link
Author

@Explorer09 Well, I think I already explained the problem in detail: the flex program needs fork(), which means it cannot build on no-MMU systems. However, the libfl library, which is used by a number of other software packages (see the list I provided) does not require fork(). Therefore, it is clearly useful for flex to support the ability to build just its library and not its flex program.

We have been having a hack for this in Buildroot since many, many years. I'm now trying to solve thisi properly, in a form that can be accepted upstream (i.e by you). But well, if you really don't understand our use case, I guess we'll just keep our patch locally and that's it.... but that would really be a shame, as we're trying hard to avoid having patches, by making lots of efforts to submit them to the respective upstream projects.

Also, I really don't see where is the additional complexity in the patch I'm proposing. It's a trivial new configure.ac option, with the default being to keep the existing behavior. I.e, there is no change for anyone, except an improvement for those who are interested in building just the library.

And no, special calling "make libfl" and "make install-libfl" is not really nice. Indeed, the whole point of autoconf/automake is that you have a well-defined ./configure && make && make install procedure. Having to deviate from that is not really nice to maintain.

@westes
Copy link
Owner

westes commented Aug 27, 2017 via email

@Explorer09
Copy link
Contributor

My opinion is that an option for disabling the main program in configure, which, if disabled, would break make check and make doc or turn them to no-ops, is not nice. When I mentioned about the make check breakage in previous post, westes hinted to me that it's "their problem" and we don't need to care. Since it's the same effort for package builders to specify one more configure option as to make only the libfl target, I really don't like the bloat to configure script of one more conditional.

There's no practice of having an option to disable main program in any other package, at least not in gcc or bison. In gcc, if you wish to build only the libstdc++ library, you either run configure script of the libstdc++-v3 subdirectory, or configure the top directory and then run make all-target-libstdc++-v3 -- you can consider both ways are "hacks".

In short, you gotta need some "hacks" anyway when you intend only to build the library. My proposal was to make you convenient to do that, and I think a makefile target is enough for this case. Having a "smooth" make && make install procedure in secondary, IMHO.

@westes
Copy link
Owner

westes commented Aug 27, 2017 via email

Explorer09 added a commit to Explorer09/flex that referenced this pull request Aug 28, 2017
These are wrappers around automake- and libtool-generated targets,
allowing users to build libfl only, without the main flex program.

See westesGH-256 for discussion.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
@westes westes added this to the 2.6.5 milestone Aug 28, 2017
@westes
Copy link
Owner

westes commented Aug 31, 2017

We're not going to do this but instead will add Makefile targets to enable building of libfl as in #258. At time of writing, there are some issues to be fixed with that request, but that or something very much like it will be in 2.6.5.

@westes westes closed this Aug 31, 2017
westes pushed a commit that referenced this pull request Sep 2, 2017
These are wrappers around automake- and libtool-generated targets,
allowing users to build libfl only, without the main flex program.

See GH-256 for discussion.

Signed-off-by: Kang-Che Sung <explorer09@gmail.com>
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.

None yet

3 participants