[QScintilla] mk_distinfo.py: wrong logic for INSTALL_ROOT installations

[Davide Pesavento]

On Wed, Aug 29, 2018 at 1:07 PM Phil Thompson
<phil at riverbankcomputing.com> wrote:
>
> On 29 Aug 2018, at 3:39 pm, Davide Pesavento <pesa at gentoo.org> wrote:
> >
> > On Wed, Aug 29, 2018 at 6:10 AM Phil Thompson
> > <phil at riverbankcomputing.com> wrote:
> >>
> >> On 28 Aug 2018, at 10:12 pm, Davide Pesavento <pesa at gentoo.org> wrote:
> >>>
> >>> On Tue, Aug 28, 2018 at 4:59 PM Phil Thompson
> >>> <phil at riverbankcomputing.com> wrote:
> >>>>
> >>>> On 28 Aug 2018, at 9:55 pm, Davide Pesavento <pesa at gentoo.org> wrote:
> >>>>>
> >>>>> On Tue, Aug 28, 2018 at 4:46 PM Phil Thompson
> >>>>> <phil at riverbankcomputing.com> wrote:
> >>>>>>
> >>>>>> On 28 Aug 2018, at 9:21 pm, Davide Pesavento <pesa at gentoo.org> wrote:
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> I'm installing QScintilla with 'make INSTALL_ROOT=/foo/bar install'
> >>>>>>> and the mk_distinfo.py step fails while trying to open non-existent
> >>>>>>> files, e.g.:
> >>>>>>>
> >>>>>>> Traceback (most recent call last):
> >>>>>>> File "mk_distinfo.py", line 101, in <module>
> >>>>>>> fn_f = open(fn, 'rb')
> >>>>>>> FileNotFoundError: [Errno 2] No such file or directory:
> >>>>>>> '/usr/lib/python3.7/site-packages/PyQt5/Qsci.so'
> >>>>>>>
> >>>>>>> (the file is actually at /foo/bar/usr/lib/python3.7/site-packages/PyQt5/Qsci.so)
> >>>>>>>
> >>>>>>> I took a quick look at the build system and noticed that the file
> >>>>>>> paths in installed.txt are not prefixed with INSTALL_ROOT.
> >>>>>>> Unfortunately that file is generated at configure time, when
> >>>>>>> INSTALL_ROOT is not yet known, so this part of the build process may
> >>>>>>> require a redesign.
> >>>>>>
> >>>>>> No, it's intentional. The location is where things will eventually be installed. INSTALL_ROOT is used by (normally Linux) distro packagers.
> >>>>>
> >>>>> I'm aware, I'm one of those distro packagers.
> >>>>>
> >>>>> Ok so installed.txt has the correct paths: the final install location
> >>>>> without INSTALL_ROOT. Then it's mk_distinfo.py's responsibility to
> >>>>> prefix each of those paths with INSTALL_ROOT before trying to open the
> >>>>> file.
> >>>>
> >>>> So who removes it when the package is installed by the user?
> >>>>
> >>>
> >>> I'm not sure I understand the question. The package manager moves the
> >>> files from INSTALL_ROOT to the real root (/). INSTALL_ROOT should not
> >>> appear (e.g. as a string or link) in any installed file. So for
> >>> instance the .dist-info/RECORD file should not contain INSTALL_ROOT.
> >>
> >> I meant the value of INSTALL_ROOT.
> >>
> >> Are you saying that the distro package manager strips the value of INSTALL_ROOT from the RECORD file when it installs the package into it's final location? If that's the case (and it is true of *all* distro package managers) then I'm happy to change the behaviour - afaik nobody else uses this feature.
> >
> > No, that's not what happens. The package manager simply moves the
> > files but doesn't touch them.
>
> That's what I thought.
>
> > QScintilla's build system should not
> > prefix the paths in RECORD with INSTALL_ROOT in the first place.
>
> It doesn't.

I know, I was just clarifying that I'm not suggesting to start doing that.

>
> > We
> > can add workarounds to the distro build scripts but I'd rather have
> > this properly fixed in QScintilla.
> >
> > When I said "it's mk_distinfo.py's responsibility to prefix each of
> > those paths with INSTALL_ROOT before trying to open the file" I meant
> > *temporarily* prefix them, just for the purpose of reading the file to
> > calculate its checksum. The prefixed paths should not be recorded
> > anywhere.
>
> The RECORD file is not checksummed, so why temporarily prefix the lines?

Sorry again, I didn't explain myself clearly. I meant, in
mk_distinfo.py around line 101, the open call should be something
like:

    fn_f = open(os.path.join(INSTALL_ROOT, fn), 'rb')

so that it calculates the SHA256 of the file that was just installed
into INSTALL_ROOT.

(as an aside, I was wondering why mk_distinfo.py doesn't use the
python csv module to write the RECORD file)

>
> >> Note that mk_distinfo.py in the current QScintilla version doesn't properly create relative filenames for files installed in the Python installation (which is most of them). However QScintilla also installs files in the Qt installation and these have to be referenced by absolute filenames so the above issue is still valid.
> >
> > So the .sip files should have an absolute path in RECORD? They don't
> > as far as I can see, e.g. I have lines like
> > ../../../../../../../../../usr/share/sip/PyQt5/Qsci/qscistyle.sip,sha256=vcDOCT7DyYUrvdUgGlkmD6ly3SMVZ5jo3ZJHzthh-bs,1909
>
> The Python installation is defined as anything below the --prefix directory.

That's the same as "import sys; sys.prefix" right? It prints
'/usr/lib/python-exec/python3.6/../../..' (i.e. /usr) on my system,
which explains why the paths in RECORD are relative. Maybe they could
be canonicalized though?

Thanks,
Davide