Repository navigation
Ticket #4803: tests - link via LDADD instead of LIBS - #4806
Conversation
egmontkob
left a comment
There was a problem hiding this comment.
Verified again that it fixes my problem, thanks! :)
Can't comment on the implementation details.
|
The annoying Ubuntu issue must be because of the great Debian ideas around |
|
Do you mean this CI failure? Unfortunately I have no idea. I'm on Ubuntu 25.10, from a clean repo with your patch |
Yes, exactly. Our CI runs on Ubuntu 24.04 LTS. Maybe they finally decided to stop the nonsense by the time 25.10 got released? If yes, too bad it didn't make it into the LTS. :( |
|
Works for me on my 24.04 computer, too. It must be something different, some compile flags or LDFLAGS or some other difference in the enrivonment. |
Try |
|
Haha, sorry. Broken on 25.10 too. |
|
How co cancel my approval? I have an error with |
Each of test executables must be linked with all required libraries, but $(GLIB_LIBS) and @E2P_LIBS@ are removed now. |
|
the linking error probably stems from unintentional reliance on a transitive dependency, that is, some module uses glib, but fails to explicitly link it. note that libs/modules should use LIBADD, not LDADD, iiuc. |
I don't think that you can remove your approval with GitHub, but what you can do instead is to leave a new review requesting changes. This effectively cancels your previous approval. |
Yes, me removing these statements is correct and intentional. The question is how do you organize the linking of your executables? When you link your executable with an internal library that is part of your project like So either, you can define a variable that contains all the libraries that your executable needs, like Or you can organize your code in internal libraries and specify the dependencies for these libraries, like Our build system uses this second way: The tests, for whatever reason, used low-level The consequence was that you had to specify transitive dependencies yourself in addition to the libraries themselves, and also that if source files were changing the tests were not correctly rebuilt. I didn't investigate who introduced that code and why, but clearly, it's a misguided idea, to stay civil. Now I changed it to depend on the libraries in the correct way, fixing the source dependency tracking and automatic transitive dependency lookup. However, it has the consequence of breaking the build if The reason why this is a questionable idea in the first place is explained at length in #4803. Of course, in addition to that, we are obviously doing it wrong because I didn't check exactly why it happens, but either we are generally doing it wrong or it fails due to the interaction with Debian libtool patches specifically. In any case, I think I've had enough of this nonsense. It's time to remove the generation of the useless shared library. I will create a separate ticket for that. |
I'm not quite sure what you mean here. The tests are binaries, not libraries, so I'm using |
|
but it could be relevant for the missing transitive dep. just a random idea. |
ed4c8be to
2782cc3
Compare
|
I have rebased this one on top of #5192. This should fix the build failure with |
2782cc3 to
7913e39
Compare
LIBS with LDADD to fix tests deps and linkageLDADD instead of LIBS
|
/rebase |
Libraries listed in the low-level LIBS variable are passed to the linker as is, but Automake doesn't add them to the dependencies of the programs. Hence, test programs were not relinked when the code of libmc or libinternal changed, and `make check` kept running stale binaries. Use the high-level LDADD variable instead, so that Automake derives the link-time dependencies of the test programs from the libtool libraries listed there, and the tests get relinked whenever these are rebuilt. Assisted-By: Claude Opus 5.5 Signed-off-by: Yury V. Zaytsev <yury@shurup.com>
f7a410b to
6e5ddb7
Compare
Proposed changes
The low-level LIBS variable of autotools does not provide dependency tracking for sources and also does not resolve interdependencies between libraries.
Replace it with high-level LDADD to make sure that tests get re-linked when underlying source files change, and also automatically consider dependent libraries.
make checkdoesn't pick up changes to the code #4803