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

Fix elfutils +debuginfod installation issue #36758

Merged
merged 7 commits into from Apr 26, 2023

Conversation

tmadlener
Copy link
Contributor

Fixes #36710

  • Add new versions for libmicrohttpd. Also add an older one to make elfutils configure with +debuginfod
  • Require libarchive~iconv in order to avoid another configure error.
  • Make elfutils depend on pkgconfig explicitly to avoid picking up the system one.

0.9.50 is necessary for elfutils
To avoid picking up the system one
- Correct version for libmicrohttpd
- libarchive without iconv to avoid configure error
# NB: Waiting on an elfutils patch before we can use libmicrohttpd@0.9.51
depends_on("libmicrohttpd@0.9.33:0.9.50", type="link", when="+debuginfod")
# libarchive with iconv doesn't configure: https://github.com/spack/spack/issues/36710
depends_on("libarchive@3.1.2: ~iconv", type="link", when="+debuginfod")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#36710 (comment):

I can confirm that the problem initially presented itself with libarchive@3.6.2. Using the previous version allows elfutils to build.

maybe we should make this version dependent then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made libarchive@3.6.2 +iconv a conflict. Building against libarchive@3.5.2+iconv works for me as well, and the concretizer is smart enough to figure out libarchive@3.6.2 ~iconv by default.

@G-Ragghianti
Copy link
Contributor

This resolves my issues in building gdb.

@tmadlener
Copy link
Contributor Author

@hainest would you be OK with this temporary fix until uptream libarchive is fixed?

@hainest
Copy link
Contributor

hainest commented Apr 11, 2023

@tmadlener This works for me. Thanks for working on it!

hainest
hainest previously approved these changes Apr 11, 2023
@mwkrentel
Copy link
Member

This is, at best a temporary solution until libarchive sorts itself out, right?

There was an active discussion of this in the libarchive pull requests
in Dec/Jan. The problem centers around iconv being provided by glibc
and how to handle that case (where no extra lib is used) vs a separate
libiconv.

libarchive/libarchive#1825
libarchive/libarchive#1817
libarchive/libarchive#1813
libarchive/libarchive#1812

All of the above are short but a bit different for how they solve the
problem. People argued forcefully for and against different patches.

The last one, 1825 got merged after 3.6.2 was released, although some
people argued that it was the wrong solution.

Anyone know enough about pkgconfig to say what is the "right" solution
here (I don't). As I said, 1825 is now in the libarchive code base,
so maybe go with that?

@tmadlener
Copy link
Contributor Author

Yes, I would also consider this a temporary solution. I am also no expert in any way in pkgconfig, so I can't weigh in either on which would be the proper way to fix this.

I have however entirely missed that they merged a PR to "fix" this (for a definition of fix). We could also try to apply that patch to libarchive@3.6.2 via spack and see if that let's us remove the conflict here.

@scheibelp scheibelp self-assigned this Apr 12, 2023
@mwkrentel
Copy link
Member

@tmadlener @G-Ragghianti @scheibelp
I've been experimenting with possible fixes, including 1825 from
libarchive and I'm going around in circles and getting flustrated.
I believe there is a better fix, but probably not one I'll come
up with in a reasonable time.

So, with the understanding that this is a temporary workaround until
we get the "right fix," I suggest we should merge this and move on.

@scheibelp Why is CI failing? The patch includes:

conflicts("libarchive@3.6.2 +iconv", when="+debuginfod")

And the error message is:

Run . share/spack/setup-env.sh
PKG-DIRECTIVES: 1 issue found
1. elfutils: wrong variant in "conflicts" directive
    the variant 'iconv' does not exist
    in /home/runner/work/spack/spack/var/spack/repos/builtin/packages/elfutils/package.py
Error: Process completed with exit code 1.

Is spack assigning +iconv to elfutils instead of libarchive?
Do I need to write this as ^libarchive +iconv instead of libarchive +iconv
to force iconv to go with libarchive??

Note: libarchive has a +iconv variant, but elfutils does not.

mwkrentel
mwkrentel previously approved these changes Apr 18, 2023
@G-Ragghianti
Copy link
Contributor

Do I need to write this as ^libarchive +iconv instead of libarchive +iconv

Yes I think you just need this change.

@mwkrentel
Copy link
Member

@tmadlener I checked with Massimiliano at the Wed spack meeting. Yes, you need
to add the carat ^ to libarchive, that will fix the CI failure.

conflicts("^libarchive@3.6.2 +iconv", when="+debuginfod")

Without the ^, this is interpreted as libarchive@3.6.2 AND +iconv,
that is, two separate things.

@tmadlener tmadlener dismissed stale reviews from mwkrentel and hainest via 40f6d34 April 19, 2023 16:52
@tmadlener
Copy link
Contributor Author

Thanks for the hint. Fixed now, and CI seems happier.

mwkrentel
mwkrentel previously approved these changes Apr 19, 2023
@tmadlener
Copy link
Contributor Author

@mwkrentel @hainest I think this needs approval again, after fixing the merge conflict

@tmadlener
Copy link
Contributor Author

@scheibelp is there anything still missing for this to get merged?

@scheibelp scheibelp merged commit eb45525 into spack:develop Apr 26, 2023
12 checks passed
@scheibelp
Copy link
Member

Thanks!

joequant pushed a commit to joequant/spack that referenced this pull request Apr 26, 2023
* elfutils cannot build against libarchive@3.62+iconv 
* elfutils needs libmicrohttpd version 0.9.50 or older
* elfutils: explicitly depend on pkg-config
* libmicrohttpd: Add several new versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation issue: elfutils
5 participants