diff mbox series

devshell: introduce intercepts as per native / nativesdk

Message ID 20220420151141.25027-1-paul.gortmaker@windriver.com
State New
Headers show
Series devshell: introduce intercepts as per native / nativesdk | expand

Commit Message

Paul Gortmaker April 20, 2022, 3:11 p.m. UTC 
In a devshell, recent versions of git will complain if the repo is owned
by someone other than the current UID - consider this example:

 ------
  bitbake -c devshell linux-yocto

  [...]

  kernel-source#git branch
  fatal: unsafe repository ('/home/paul/poky/build-qemuarm64/tmp/work-shared/qemuarm64/kernel-source' is owned by someone else)
  To add an exception for this directory, call:

        git config --global --add safe.directory /home/paul/poky/build-qemuarm64/tmp/work-shared/qemuarm64/kernel-source
  kernel-source#
 ------

Of course the devshell has UID zero and the "real" UID is for "paul" in
this case.  And so recent git versions complain.

As the whole purpose of the devshell is to invoke a shell where development
can take place, having a non-functional git is clearly unacceptable.

Richard suggested we could use PSEUDO_UNLOAD=1 to evade this issue, and I
suggested we probably will see other similar instances like this and should
make use of PATH to intercept via devshell wrappers - conveniently we already
have examples of this.

Here, we copy the existing "ar" example and tune it to the needs of git to
combine Richard's suggestion and mine.

As such we now also can store commit logs and use send-email with our user
specific settings, instead of "root", so in additon to fixing basic
commands like "git branch" it should also increase general usefulness.

Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Comments

Richard Purdie April 21, 2022, 9:53 a.m. UTC | #1 
On Wed, 2022-04-20 at 11:11 -0400, Paul Gortmaker wrote:
> In a devshell, recent versions of git will complain if the repo is owned
> by someone other than the current UID - consider this example:
> 
>  ------
>   bitbake -c devshell linux-yocto
> 
>   [...]
> 
>   kernel-source#git branch
>   fatal: unsafe repository ('/home/paul/poky/build-qemuarm64/tmp/work-shared/qemuarm64/kernel-source' is owned by someone else)
>   To add an exception for this directory, call:
> 
>         git config --global --add safe.directory /home/paul/poky/build-qemuarm64/tmp/work-shared/qemuarm64/kernel-source
>   kernel-source#
>  ------
> 
> Of course the devshell has UID zero and the "real" UID is for "paul" in
> this case.  And so recent git versions complain.
> 
> As the whole purpose of the devshell is to invoke a shell where development
> can take place, having a non-functional git is clearly unacceptable.
> 
> Richard suggested we could use PSEUDO_UNLOAD=1 to evade this issue, and I
> suggested we probably will see other similar instances like this and should
> make use of PATH to intercept via devshell wrappers - conveniently we already
> have examples of this.
> 
> Here, we copy the existing "ar" example and tune it to the needs of git to
> combine Richard's suggestion and mine.
> 
> As such we now also can store commit logs and use send-email with our user
> specific settings, instead of "root", so in additon to fixing basic
> commands like "git branch" it should also increase general usefulness.
> 
> Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> diff --git a/meta/classes/devshell.bbclass b/meta/classes/devshell.bbclass
> index 62dc958d9a4a..7ac134e0950f 100644
> --- a/meta/classes/devshell.bbclass
> +++ b/meta/classes/devshell.bbclass
> @@ -2,6 +2,8 @@ inherit terminal
>  
>  DEVSHELL = "${SHELL}"
>  
> +PATH:prepend = "${COREBASE}/scripts/devshell-intercept:"
> +

I just realised, we can't do that! It would apply globally and mean builds
change behaviour depending on whether devshell is inherited or not. It also
means all git calls run through the wrapper, not just devshell ones.

I've also realised we have a bigger problem since some software is running git
to get the version during do_install.

My proposal is to change scripts/devshell-intercept to scripts/git-intercept,
make the PATH change in the do_devshell function below but also add something
like:

PATH:prepend:task-do_install = "${COREBASE}/scripts/git-intercept:"

in common code.

Since this hasn't merged yet, I can tweak the patch if that works for you (or
you can)?

Cheers,

Richard
Peter Kjellerstedt April 21, 2022, 10:10 a.m. UTC | #2 
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Andre McCurdy
> Sent: den 21 april 2022 01:25
> To: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: OE Core mailing list <openembedded-core@lists.openembedded.org>;
> Richard Purdie <richard.purdie@linuxfoundation.org>
> Subject: Re: [OE-core] [PATCH] devshell: introduce intercepts as per
> native / nativesdk
> 
> On Wed, Apr 20, 2022 at 8:12 AM Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
> >
> > In a devshell, recent versions of git will complain if the repo is owned
> > by someone other than the current UID - consider this example:
> >
> >  ------
> >   bitbake -c devshell linux-yocto
> >
> >   [...]
> >
> >   kernel-source#git branch
> >   fatal: unsafe repository ('/home/paul/poky/build-qemuarm64/tmp/work-
> shared/qemuarm64/kernel-source' is owned by someone else)
> >   To add an exception for this directory, call:
> >
> >         git config --global --add safe.directory /home/paul/poky/build-
> qemuarm64/tmp/work-shared/qemuarm64/kernel-source
> >   kernel-source#
> >  ------
> >
> > Of course the devshell has UID zero and the "real" UID is for "paul" in
> > this case.  And so recent git versions complain.
> >
> > As the whole purpose of the devshell is to invoke a shell where
> development
> > can take place, having a non-functional git is clearly unacceptable.
> >
> > Richard suggested we could use PSEUDO_UNLOAD=1 to evade this issue, and
> I
> > suggested we probably will see other similar instances like this and
> should
> > make use of PATH to intercept via devshell wrappers - conveniently we
> already
> > have examples of this.
> >
> > Here, we copy the existing "ar" example and tune it to the needs of git
> to
> > combine Richard's suggestion and mine.
> >
> > As such we now also can store commit logs and use send-email with our
> user
> > specific settings, instead of "root", so in additon to fixing basic
> > commands like "git branch" it should also increase general usefulness.
> 
> It looks like this will address the performance issues of using git
> from a devshell too. Trying to run git log, git blame, etc for
> largeish repos in a devshell is annoyingly slow due to the overhead of
> pseudo.
> 
> I always wondered what people do in a devshell which actually _does_
> need to run under pseudo...

To look at the output from, e.g., do_install to see it the way bitbakes 
sees it when it comes to users/groups of the files.

//Peter

> 
> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> >
> > diff --git a/meta/classes/devshell.bbclass
> b/meta/classes/devshell.bbclass
> > index 62dc958d9a4a..7ac134e0950f 100644
> > --- a/meta/classes/devshell.bbclass
> > +++ b/meta/classes/devshell.bbclass
> > @@ -2,6 +2,8 @@ inherit terminal
> >
> >  DEVSHELL = "${SHELL}"
> >
> > +PATH:prepend = "${COREBASE}/scripts/devshell-intercept:"
> > +
> >  python do_devshell () {
> >      if d.getVarFlag("do_devshell", "manualfakeroot"):
> >         d.prependVar("DEVSHELL", "pseudo ")
> > diff --git a/scripts/devshell-intercept/git b/scripts/devshell-
> intercept/git
> > new file mode 100755
> > index 000000000000..8adf5c9ecb71
> > --- /dev/null
> > +++ b/scripts/devshell-intercept/git
> > @@ -0,0 +1,19 @@
> > +#!/usr/bin/env python3
> > +#
> > +# Wrapper around 'git' that doesn't think we are root
> > +
> > +import os
> > +import shutil
> > +import sys
> > +
> > +os.environ['PSEUDO_UNLOAD'] = '1'
> > +
> > +# calculate path to the real 'git'
> > +path = os.environ['PATH']
> > +path = path.replace(os.path.dirname(sys.argv[0]), '')
> > +real_git = shutil.which('git', path=path)
> > +
> > +if len(sys.argv) == 1:
> > +    os.execl(real_git, 'git')
> > +
> > +os.execv(real_git, sys.argv)
> > --
> > 2.17.1
> >
> >
> >
> >
diff mbox series

Patch

diff --git a/meta/classes/devshell.bbclass b/meta/classes/devshell.bbclass
index 62dc958d9a4a..7ac134e0950f 100644
--- a/meta/classes/devshell.bbclass
+++ b/meta/classes/devshell.bbclass
@@ -2,6 +2,8 @@  inherit terminal
 
 DEVSHELL = "${SHELL}"
 
+PATH:prepend = "${COREBASE}/scripts/devshell-intercept:"
+
 python do_devshell () {
     if d.getVarFlag("do_devshell", "manualfakeroot"):
        d.prependVar("DEVSHELL", "pseudo ")
diff --git a/scripts/devshell-intercept/git b/scripts/devshell-intercept/git
new file mode 100755
index 000000000000..8adf5c9ecb71
--- /dev/null
+++ b/scripts/devshell-intercept/git
@@ -0,0 +1,19 @@ 
+#!/usr/bin/env python3
+#
+# Wrapper around 'git' that doesn't think we are root
+
+import os
+import shutil
+import sys
+
+os.environ['PSEUDO_UNLOAD'] = '1'
+
+# calculate path to the real 'git'
+path = os.environ['PATH']
+path = path.replace(os.path.dirname(sys.argv[0]), '')
+real_git = shutil.which('git', path=path)
+
+if len(sys.argv) == 1:
+    os.execl(real_git, 'git')
+
+os.execv(real_git, sys.argv)