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

xrootd v5.6.0 #5066

Closed
wants to merge 3 commits into from
Closed

xrootd v5.6.0 #5066

wants to merge 3 commits into from

Conversation

adriansev
Copy link
Contributor

@adriansev adriansev requested a review from a team as a code owner June 30, 2023 16:24
@adriansev
Copy link
Contributor Author

@TimoWilken all linux tests died with Failed to find XercesC (missing: XercesC_VERSION) in vgm and the o2/macOS dies in XRootD with:

[ 89%] Linking CXX shared library libXrdCl.dylib
ld: warning: dylib (/usr/local/opt/[email protected]/lib/libssl.dylib) was built for newer macOS version (13.0) than being linked (10.15)
ld: warning: dylib (/usr/local/opt/[email protected]/lib/libcrypto.dylib) was built for newer macOS version (13.0) than being linked (10.15)
Undefined symbols for architecture x86_64:
  "_libintl_gettext", referenced from:
      _random_tell_source in libuuid.a(libuuid_la-randutils.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@adriansev
Copy link
Contributor Author

for reference, the upstream macos built was successful see https://github.com/xrootd/xrootd/actions/runs/5424472406/jobs/9863877946

@adriansev
Copy link
Contributor Author

ping @TimoWilken

@adriansev
Copy link
Contributor Author

@TimoWilken the same as before, the test fail in missing: XercesC_VERSION
except O2_alidist-dataflow-cs8 that have no log and alidist_O2Suite_o2_macOS that have:

[ 89%] Linking CXX shared library libXrdCl.dylib
ld: warning: dylib (/usr/local/opt/[email protected]/lib/libssl.dylib) was built for newer macOS version (13.0) than being linked (10.15)
ld: warning: dylib (/usr/local/opt/[email protected]/lib/libcrypto.dylib) was built for newer macOS version (13.0) than being linked (10.15)
Undefined symbols for architecture x86_64:
  "_libintl_gettext", referenced from:
      _random_tell_source in libuuid.a(libuuid_la-randutils.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@TimoWilken
Copy link
Contributor

The xercesc problem ought to be fixed by #5069.

Not sure about the Mac error.

@TimoWilken
Copy link
Contributor

I've just restarted the checks since a fix for the Xerces problem was merged.

For the Mac problem, I can reproduce this if I run echo 'echo 'extern "C" int random_tell_source(); int main() { random_tell_source(); return 0; }' | cc -xc++ - -o/dev/null -L/build/alice-ci-workdir/alidist-o2/sw/osx_x86-64/UUID/v2.27.1-local1/lib -luuid on the Intel Mac, and it's fixed if I add -L/usr/local/lib -lintl. I'm not sure why this isn't done by default, or why it works on the ARM Mac.

There were multiple changes in xrootd to the way it searches for libuuid, perhaps this was picked up before and one of them changed this? @adriansev is there a way we can tell xrootd to find libintl manually?

@adriansev
Copy link
Contributor Author

I created the issue xrootd/xrootd#2052
let's see what @amadio finds out.

@TimoWilken
Copy link
Contributor

Hi @adriansev, I already tried reinstalling and relinking gettext; that didn't work unfortunately. brew upgrade didn't help either. libintl is already a valid x86_64 library.

I should add that in my working reproducer above, -L/usr/local/lib isn't required; just -L.../UUID/.../lib -luuid -lintl works.

Maybe we should patch UUID's pkgconfig file to include -lintl, but I don't know if that's the right thing to do, since it works on the ARM Mac, and v5.5.3 of XRootD builds fine as-is, without explicitly specifying -lintl...

@TimoWilken
Copy link
Contributor

One more thing to try: I've deleted the libuuid from aliBuild's cache on the machine, just in case it dates from before a gettext update in brew. I've restarted the check on this PR, let's see what it does...

@adriansev
Copy link
Contributor Author

@TimoWilken could you please comment on xrootd/xrootd#2052 issue as there is info that i do not know about

@TimoWilken
Copy link
Contributor

See my comment there @adriansev. What do you want to do about the inconsistency of using our UUID package through PKG_CONFIG_PATH, but unsetting UUID_ROOT on Mac and depending on UUID:(?!osx)? Should we change that to not unset UUID_ROOT and depend on UUID instead?

This should avoid undeclared runtime dependencies on libintl/gettext.
On Mac, we use our UUID anyway since we depend on AliEn-Runtime, which depends
on UUID, and UUID sets PKG_CONFIG_PATH so we find it even with UUID_ROOT
unset.

This is counter-intuitive, so stop fiddling with UUID_ROOT manually.
@adriansev
Copy link
Contributor Author

IMHO it would best to have as little as private customizations possible as the pile-up of corner cases and their customizations can become very fast a nightmare...
So, i would always go toward solution that are the same everywhere BUT this is just my view, and given that i do not have the full overview of ALL the systems and problems that you take care of :) i might be wrong :) So, i would prefer to use the same UUID everywhere if possible ... it would be consistent both technically and on "moral" level :) ... i would also be curious if all system provided uuids (either macos all linux distros ones) works the same 😄

@TimoWilken
Copy link
Contributor

Apparently using the same UUID everywhere is a must, since @ktf remembers some problems with using the system libuuid on Macs in the past.

I've patched uuid.sh and taken the liberty to push to your branch; let's see what the CI thinks of the combination!

Also, I don't know exactly what configure options aren't needed any more, so I'll leave it to you to modernise those if you want @adriansev.

@amadio
Copy link
Contributor

amadio commented Jul 7, 2023

I created an alternative pull request for the update here: #5075. It should take care of modernizing the build options. If you want extras, like support for erasure coding in the XRootD client, we can also work on that.

@adriansev
Copy link
Contributor Author

@TimoWilken Could we close this and move to #5075? what @amadio already did, i was planning to do after this step, but if it's already done let's do the whole dance :)
So, could you cherry pick these commits to #5075 ?

@TimoWilken
Copy link
Contributor

Done! Closing this to reduce load on the CI.

@TimoWilken TimoWilken closed this Jul 7, 2023
@adriansev adriansev deleted the xrootd_update branch August 28, 2023 17:38
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