| Summary: | If DESTDIR is set to an empty string, the dri drivers are not installed | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Pierre Labastie <pierre.labastie> |
| Component: | Other | Assignee: | Dylan Baker <baker.dylan.c> |
| Status: | RESOLVED FIXED | QA Contact: | mesa-dev |
| Severity: | normal | ||
| Priority: | medium | CC: | benoit.pierre, lonewolf |
| Version: | 19.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
attachment-9884-0.html
fix DESTDIR handling in install_megadrivers.py |
||
I never expected anyone to set DESTDIR to '', so this is definitely a bug. At one point meson didn't enforce that libdir had to be under prefix, and os.path.join will ignore any arguments that precede an absolute path. I'll send a patch to fix this. This should be fixed in master, and the patch will be in the next 19.0 and 18.3 releases. Thanks for the quick fix. Hi, dylan your patch broke ArchLinux PKGBUILD for mesa since your patch for some reason bypass fakeinstall paths and try to install directly in /usr/lib, any suggestions? Created attachment 143746 [details] attachment-9884-0.html :( Yes, I know how to fix it. Just need more code. I'm on mobile, please excuse autocorrect fail. On Thu, Mar 21, 2019, 04:34 <bugzilla-daemon@freedesktop.org> wrote: > *Comment # 4 <https://bugs.freedesktop.org/show_bug.cgi?id=110211#c4> on > bug 110211 <https://bugs.freedesktop.org/show_bug.cgi?id=110211> from > Rafael Castillo <jrch2k10@gmail.com> * > > Hi, dylan your patch broke ArchLinux PKGBUILD for mesa since your patch for > some reason bypass fakeinstall paths and try to install directly in /usr/lib, > any suggestions? > > ------------------------------ > You are receiving this mail because: > > - You are the assignee for the bug. > > Created attachment 143747 [details] [review] fix DESTDIR handling in install_megadrivers.py This how I fixed it locally. I only tested the PKGBUILD on Arch Linux (so with DESTDIR set). I tried to ensure Windows is correctly supported to (with the correct drive being used in all cases). agree, also faced with this issue. Reproduced it on Ubuntu 18.04 and Manjaro. (In reply to Benoit Pierre from comment #6) > Created attachment 143747 [details] [review] [review] > fix DESTDIR handling in install_megadrivers.py > > This how I fixed it locally. I only tested the PKGBUILD on Arch Linux (so > with DESTDIR set). I tried to ensure Windows is correctly supported to (with > the correct drive being used in all cases). Looks like your patch comes back to the original issue: This is Ok if DESTDIR is not set, or if DESTDIR is set to a non empty string, but not if DESTDIR is set to an empty string. Note that this is because os.path.join is not consistent: a slash is inserted when doing os.path.join('a','b'), but not when doing os.path.joint('','b')! Note that the fix I proposed is bad (sorry): if you do, for instance, "to = os.path.join('/sources/install','/usr/lib/dri')" in python, then "to" is set to '/usr/lib/dri'... My bad (I thought I had tried). Maybe something like (I'm not sure what to do with win32): --- if os.path.isabs(args.libdir): destdir=os.environ.get('DESTDIR','') if len(destdir) == 0: destdir = '/' to = os.path.join(destdir, args.libdir[1:]) else: ... ---
if os.path.isabs(args.libdir):
destdir=os.environ.get('DESTDIR','')
if len(destdir) == 0:
destdir = '/'
to = os.path.join(destdir, args.libdir[1:])
else:
...
Don't use len there, len() for testing emptiness is a python antipattern :)
just `if not destdir:`
However, I think doing something like
destdir = os.environ.get('DESTDIR')
to = args.libdir
if destdir
to = os.path.join(destdir, to)
is probably sufficient.
Just FYI, we don't do patch review in bugzilla, if you'd like to propose a patch either an MR at gitlab.freedesktop.org/mesa/mesa or a patch sent to mesa-dev@lists.freedesktop.org is the proper way.
(In reply to Dylan Baker from comment #9) > --- > if os.path.isabs(args.libdir): > destdir=os.environ.get('DESTDIR','') > if len(destdir) == 0: > destdir = '/' > to = os.path.join(destdir, args.libdir[1:]) > else: > ... This fixes it for me. Have a look at this MR, which is basically the same patch with some style cleanups: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/505 I told you I was not a Python geek :) The MR seems to work in all cases I can imagine (unset DESTDIR, empty DESTDIR, DESTDIR set to non-empty) when abs.libdir starts with '/' (sorry if this is not the place to comment on the MR). Thanks for the Python lesson, it's my first day with Python (almost) ;) Commenting on an issue does not add you to the CC list? How stupid is that... Should be fixed in master now |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Dear maintainer, The script "bin/install_megadrivers.py" contains the following: --- if os.path.isabs(args.libdir): to = os.path.join(os.environ.get('DESTDIR', '/'), args.libdir[1:]) else: to = os.path.join(os.environ['MESON_INSTALL_DESTDIR_PREFIX'], args.libdir) --- The issue is when libdir is absolute, and DESTDIR is set to the empty string. This is very likely to occur in build scripts of the form (many of us at http://www.linuxfromscratch.org do that): --- #PKG_DEST=/some/where ... DESTDIR=$PKG_DEST ninja install --- where we comment out the PKG_DEST assignment or not, depending on whether we want a DESTDIR install or not. Then the "to" variable becomes a relative path, and the drivers are not installed. This is not really what is expected, I guess. I'd suggest using (but I am far from being a Python geek): --- if os.path.isabs(args.libdir): to = os.path.join(os.environ.get('DESTDIR', ''), args.libdir) --- That worked for me in all cases I have tried (may not have thought of all of them...)