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

Tweaks to make configuration system more friendly to gentoo. #1852

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

CrazyCasta
Copy link

Intention is that these tweaks do not affect systems that do not call
configure with things like --libdir.

Fixup to add ac_tool_prefix.
Ignore things like mandir, infodir, etc that gentoo passes in and ghdl
doesn't care about.
Allow for --libdir to override the libdir. This is how gentoo handles
things like 32/64 bit.

Description
Currently ghdl has a bit of trouble building on gentoo. This is meant to be a minimal patch to address that.

Expected behaviour
All current uses of configuration will remain unchanged.
Gentoo ebuild will no longer require patches (https://bugs.gentoo.org/679340).

Context
Please, provide the following information:

  • OS: gentoo
  • Origin: trying to package for gentoo

Intention is that these tweaks do not affect systems that do not call
configure with things like --libdir.

Fixup to add ac_tool_prefix.
Ignore things like mandir, infodir, etc that gentoo passes in and ghdl
    doesn't care about.
Allow for --libdir to override the libdir. This is how gentoo handles
    things like 32/64 bit.
@CrazyCasta
Copy link
Author

P.S. While I did check that the Make-lang.in seemed to be correct (minimally different from original in only the expected way), someone that knows how to build the gcc branch might want to test it.

@CrazyCasta
Copy link
Author

Okay, so I've change a bit of stuff here and there. Please provide feedback, once we have something we all agree on I'll rebase it into one patch and shift this PR to that one.

@tgingold
Copy link
Member

tgingold commented Aug 31, 2021 via email

@CrazyCasta
Copy link
Author

Yes, dispconfig was for debugging. AS you might notice it took quite a while to pass all the tests, haha!

I think I had some idea how the other options like datadir were going to be used, like for man files or something like that, but I've forgotten so I'll take the do nothing approach for now like you're suggesting.

Honestly I've completely forgotten where the ac_tool_prefix came from. I'll take it out and make sure it still works for gentoo.

The GHDL_PREFIX stuff was to allow the tests to find the lib dir while testing. It's possible that the last bit that I did, that restored some of the original behaviour wrt relative paths may have obviated the need for that. Let me see if that's the case. The GHDL_PREFIX stuff should only be necessary when the lib dirs are not installed yet (i.e. still in the build directory).

I'll start a new branch and see if I can do a cleaner version of this.

@CrazyCasta
Copy link
Author

Actually, first I'm going to see if I can't figure out how to write an ebuild that will pull from a local git repo so that I can test whether reduced versions of this work. And so that I can integrate it into some CI workflow as a set of commands to run on a docker image.

@CrazyCasta
Copy link
Author

CrazyCasta commented Sep 5, 2021

@tgingold Now that I've swapped this back in I've found the reason why GHDL_PREFIX is sometimes necessary. https://github.com/ghdl/ghdl/blob/master/src/ghdldrv/ghdllocal.adb#L430 If Lib_Prefix is a relative path (as it often is) and exec_prefix is not set it will use a relative path from where the executable is sitting. This is fine except for the case I'm specifically trying to handle which is Lib_Prefix being an absolute path.

Can you give some context to what the problem with the testsuite is? Are you trying to make the testsuite work on both build directories and installed binaries? Currently it doesn't apply the GHDL_PREFIX is GHDL doesn't include '/'. I could try expanding that to say both not containing '/' and not the expected install location (though it might be slightly troublesome to work that out).

@tgingold
Copy link
Member

tgingold commented Sep 6, 2021 via email

@CrazyCasta
Copy link
Author

Well, gentoo can use libdir to mean a lot of different things. It used to be /usr/lib, now I think it's /usr/lib64, but technically it's possible for ppl to be stuck in the past and still use /usr/lib. Let me back out most of my changes as far as the testsuite is concerned and see if I can just tweak the CI as necessary (if necessary).

@umarcor
Copy link
Member

umarcor commented Sep 7, 2021

Ideally we should have a CI to really test this option, otherwise we will break it. Should we add an extra github action ?
@umarcor, your advice is welcome!

I was having a look at the changes of this PR but I'm unsure to understand what's going on 😕. Although I kind of get the idea, the details of the implementation are unclear to me. libdir, libdirsuffix, lib/, lib_prefix... I feel that complexity might be simplified. Do the prefix, the dir and the suffix all need to be handled differently? To my understanding, the lib_prefix/libdir/libdirsuffix should be "constant" for an installation.

Similarly, in the modification to the testsuite, the GHDL_PREFIX is set explicitly to GHDL_PREFIX=${GHDL%/*}/../lib/ghdl if GHDL is set. However, as far as I understand, it should work by specifying GHDL only. I mean, the "fix" for dealing with relative or absolute paths should be "internal", instead of being handled in the testsuite script.

Anyway, I don't have any strong stopper. This PR looks good to me, as long as those details are understood by others.

@CrazyCasta
Copy link
Author

Don't pull it now or anything, I'm working on reducing the tons of extra stuff. Having come back to look at it this was definitely more of a first pass of "how do I get what I want for gentoo and still make the CI pass". I'm working on a better version.

@Xiretza
Copy link
Contributor

Xiretza commented Sep 8, 2021

Don't pull it now or anything

Can you mark the PR as a draft? Wouldn't be the first time something was merged a little too eagerly ;)

@umarcor umarcor marked this pull request as draft September 8, 2021 09:25
@CrazyCasta
Copy link
Author

Thanks, didn't know about that option.

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

4 participants