-
Notifications
You must be signed in to change notification settings - Fork 25
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
testing rpath on MacOS during testsuite #49
Conversation
Yay, I'm a first time contributor so need a maintainer to get my workflow run approved :-) Note: this is not intended to be merged directly, if it works out I'll commit this upstream with a ChangeLog entry - if you have any review notes then I'm eager to read them in any case. |
Hi @GitMensch . I've just asked a colleague knowledgeable than me on macos workings for his thoughts on this. |
Ouch, seems like argument to |
It should exists, otherwise it does not make any sense.
Something is added between |
Another workflow to approve - just used the "forward option to linker" this time, which allows comma-separated options because the previous quoting seems to be broken "somewhere" (it always worked with installed versions of GnuCOBOL, so I don't want to put time in that checking about it not working without shell access [I guess there is a way to ssh into GH workers, but I didn't checked if/how it can be done so far]). |
tests/atlocal.in
Outdated
# MacOS workaround (HACK) to embed rpath during test | ||
if test "$GNUCOBOL_ENV_SETUP" != "1" -a "$GNUCOBOL_TEST_LOCAL" != "1"; then | ||
# HACK, may need some version check, should likely be disabled if configure used --without-rpath | ||
case "$OSTYPE" in darwin*) FLAGS="${FLAGS} -Q -Wl,-rpath,${abs_top_builddir}/libcob/.libs";; esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does abs_top_builddir
need quotes in case someone has spaces in the build path? But it's a workaround for the CI so I guess that may be ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, yes - as we now are back to "failing as before" I'll add it, just to check if this breaks the test runs again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work as good as before.
Presumably the rpath got inserted but the link-name is still hard-wired in. If this is the case it likely should be possible to post-remove it - but that's not what we want. I hope your colleague has an idea - here are the commands that are executed and the result:
|
As far as I can understand, here maybe an approach is to tweak the "install name" of the library w.r.t the executable. Apparently there's also a tool to change that. (cf. https://stackoverflow.com/questions/27506450/clang-change-dependent-shared-library-install-name-at-link-time ) |
We don't want to tweak the install name of the library as this would also have an effect on the actual installed one - we should only tweak the testsuite. |
Codecov Report
@@ Coverage Diff @@
## gcos4gnucobol-3.x #49 +/- ##
==================================================
Coverage 66.23% 66.23%
==================================================
Files 32 32
Lines 51948 51948
Branches 13386 13386
==================================================
+ Hits 34406 34407 +1
+ Misses 12150 12149 -1
Partials 5392 5392
Continue to review full report at Codecov.
|
OK, changed the compile to not link against the system libcob at all - but it is still searched. This brings us to another "may be inspected by someone":
|
PR to test that with the MacOS runner