[git-buildpackage] Would it be possible for pristine-tar to not be an absolute path?

[Guido Günther]

Hi,
On Mon, Sep 18, 2017 at 11:24:21AM -0700, Nish Aravamudan wrote:
> On 30.08.2017 [21:47:08 +0200], Guido Günther wrote:
> > Hi,
> > On Wed, Aug 30, 2017 at 10:18:10AM -0700, Nish Aravamudan wrote:
> > > On 30.08.2017 [19:02:20 +0200], Guido Günther wrote:
> > > > Hi,
> > > > On Wed, Aug 30, 2017 at 09:37:28AM -0700, Nish Aravamudan wrote:
> > > > > Hello,
> > > > > 
> > > > > I am currently using gbp in a snap for `git-ubuntu`, a tool similar to
> > > > > dgit for doing Ubuntu source package work. The snap contains all its
> > > > 
> > > > Can you point me to the snap?
> > > 
> > > Yep; I interpreted your statement in three different ways :)
> > > 
> > > 1) To install the snap:
> > > 
> > >   sudo snap install git-ubuntu --classic
> > > 
> > > 2) Source: 
> > > 
> > > https://code.launchpad.net/~usd-import-team/usd-importer/+git/usd-importer/+ref/master
> > 
> > That's the one I was after. Thanks a lot! Looking at the code it seems
> > there's some overlap with things gbp implements. Note that you can also
> > do "import gbp" and call things directly from python if you prefer. The
> > stuff outside scripts/ aims for a stable API.
> > 
> > > The snap is automatically built from master currently.
> > > 
> > > 3) The error we get (without pristine-tar installed in the host, which
> > > is what I'm trying to avoid requiring by putting it in the snap) is:
> > > 
> > > gbp:debug: /usr/bin/pristine-tar [] ['commit', '/tmp/tmpk6ru7ya5/.git/git-ubuntu-cache/virt-manager_0.5.4.orig.tar.gz', 'c0a614c62bad2d63e671ff3c1ae853380e952cab']
> > > gbp:error: Import of /tmp/tmpk6ru7ya5/.git/git-ubuntu-cache/virt-manager_0.5.4.orig.tar.gz failed: Couldn't commit to 'pristine-tar' with upstream 'c0a614c62bad2d63e671ff3c1ae853380e952cab': execution failed: [Errno 2] No such file or directory
> > > 
> > > ...
> > > 
> > > The host doesn't have pristine-tar, but it is available at:
> > > 
> > > /snap/git-ubuntu/current/usr/bin/pristine-tar.
> > > 
> > > The former ends up in snaps as $SNAP and we have a wrapper around our
> > > commands that ensures $SNAP/usr/bin (and a bunch of other common paths)
> > > are in $PATH.
> > > 
> > > To be clear, we only use gbp for orig tarball handling currently (we
> > > import-orig the tarballs published for Debian & Ubuntu, and use
> > > buildpackage (eventually export-orig, once it's available) to get the
> > > tarballs back out).
> > 
> > gbp export-orig is already in the versions in experimental. It will gain
> > some more features in the near future (like checking out arbitrary
> > versions without having a working copy).
> > 
> > > > > dependencies, including pristine-tar at paths within the snap (which I
> > > > > ensure are in PATH when my tool runs), but gbp has a hard-coded path
> > > > > to pristine-tar:
> > > > > 
> > > > > gbp/pkg/pristinetar.py:
> > > > > ...
> > > > > class PristineTar(Command):
> > > > >     """The pristine-tar branch in a git repository"""
> > > > >     cmd = '/usr/bin/pristine-tar'
> > > > > 
> > > > > Is that strictly necessary for the way Command works? Would it be
> > > > > possible to allow the cli (since it may not be override-able
> > > > > otherwise) to specify where pristine-tar lives rather than it being
> > > > > hardcoded?
> > > > 
> > > > Is pristine-tar in your $PATH? As with subprocess when not going through
> > > > a shell we need an absolute path. I'm happy to add logic to check in
> > > > $PATH or different locations but I rather understand the problem first.
> > > > (I'd be great if we wouldn't need even more cli options).
> > > 
> > > Yeah, pristine-tar is in $PATH (within the context of the snap wrapper
> > > scripts, which are the only entry points to our programs anyways).
> > > 
> > > I see what you're saying, since we aren't using a shell, we don't have a
> > > PATH to resolve?
> > 
> > Exactly. I've pushed a patch that checks $PATH, this also helps when
> > testing unreleased pristine-tar versions.
> 
> Python question, as I ended up needing to do something very similar in
> our code to call out to LXD ;)
> 
> https://git.sigxcpu.org/cgit/git-buildpackage/commit/?id=4c6f8fce0dd9b89947b6b123bf38e0568ce6d83c
> 
> seems buggy in _find_in_path?
> 
> Specifically
> 
>    for p in os.getenv("PATH", "/usr/bin:/bin"):
> 
> should be
> 
>    for p in os.getenv("PATH", "/usr/bin:/bin").split(":"):
> 
> otherwise p ends up iterating the characters in PATH?

You are perfectly right! So I wondered why things worked at all after
the change and it turns out that python3 does a path lookup by itself:

    https://github.com/python/cpython/blob/master/Lib/subprocess.py#L1257
    
so I could remove the complete path lookup code.
Cheers,
 -- Guido