| Message ID | 1438285661-19086-1-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Fri, Jul 31, 2015 at 7:47 AM, Simon Glass <sjg@chromium.org> wrote: > Commit 488d19c (patman: add distutils based installer) has the side effect > of making patman run twice with each invocation. Fix this by checking for > 'main program' invocation in patman.py. This is good practice in any case. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- Reviewed-by: Chris Packham <judge.packham@gmail.com> I did (kind of) think about that at the time when I had to handle the in-tree vs out-of-tree usage. One solution would have been to move most of the code to a module ("patch-manager" say) and have the patman script import that. The same would work for anything else that wanted to bring in bits of patman (buildman perhaps?). > > tools/patman/patman.py | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/tools/patman/patman.py b/tools/patman/patman.py > index e76fc42..6fe8fe0 100755 > --- a/tools/patman/patman.py > +++ b/tools/patman/patman.py > @@ -74,8 +74,11 @@ specified by tags you place in the commits. Use -n to do a dry run first.""" > settings.Setup(parser, options.project, '') > (options, args) = parser.parse_args() > > +if __name__ != "__main__": > + pass > + > # Run our meagre tests > -if options.test: > +elif options.test: > import doctest > > sys.argv = [sys.argv[0]] > -- > 2.5.0.rc2.392.g76e840b >
Hi Chris, On 30 July 2015 at 23:34, Chris Packham <judge.packham@gmail.com> wrote: > Hi Simon, > > On Fri, Jul 31, 2015 at 7:47 AM, Simon Glass <sjg@chromium.org> wrote: >> Commit 488d19c (patman: add distutils based installer) has the side effect >> of making patman run twice with each invocation. Fix this by checking for >> 'main program' invocation in patman.py. This is good practice in any case. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- > > Reviewed-by: Chris Packham <judge.packham@gmail.com> > > I did (kind of) think about that at the time when I had to handle the > in-tree vs out-of-tree usage. One solution would have been to move > most of the code to a module ("patch-manager" say) and have the patman > script import that. The same would work for anything else that wanted > to bring in bits of patman (buildman perhaps?). Ah OK. We could do this, but what is the benefit? Buildman currently imports the particular modules it needs and doesn't use the top-level tool. [snip] Regards, Simon
On Sat, Aug 1, 2015 at 12:06 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Chris, > > On 30 July 2015 at 23:34, Chris Packham <judge.packham@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Jul 31, 2015 at 7:47 AM, Simon Glass <sjg@chromium.org> wrote: >>> Commit 488d19c (patman: add distutils based installer) has the side effect >>> of making patman run twice with each invocation. Fix this by checking for >>> 'main program' invocation in patman.py. This is good practice in any case. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> --- >> >> Reviewed-by: Chris Packham <judge.packham@gmail.com> >> >> I did (kind of) think about that at the time when I had to handle the >> in-tree vs out-of-tree usage. One solution would have been to move >> most of the code to a module ("patch-manager" say) and have the patman >> script import that. The same would work for anything else that wanted >> to bring in bits of patman (buildman perhaps?). > > Ah OK. We could do this, but what is the benefit? Buildman currently > imports the particular modules it needs and doesn't use the top-level > tool. > That may play into the de-coupling of u-boot and patman. Some of these common bits could be yet another re-usable component that both patman and buildman use. But then you'd have to come up with a name for such a component which we all know is a NP-hard problem :). I haven't really looked at buildman and only have a vague understanding of what it does. Do you think it too would be useful outside of u-boot? > [snip] > > Regards, > Simon
Hi Chris, On 1 August 2015 at 03:32, Chris Packham <judge.packham@gmail.com> wrote: > On Sat, Aug 1, 2015 at 12:06 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Chris, >> >> On 30 July 2015 at 23:34, Chris Packham <judge.packham@gmail.com> wrote: >>> Hi Simon, >>> >>> On Fri, Jul 31, 2015 at 7:47 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Commit 488d19c (patman: add distutils based installer) has the side effect >>>> of making patman run twice with each invocation. Fix this by checking for >>>> 'main program' invocation in patman.py. This is good practice in any case. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> --- >>> >>> Reviewed-by: Chris Packham <judge.packham@gmail.com> >>> >>> I did (kind of) think about that at the time when I had to handle the >>> in-tree vs out-of-tree usage. One solution would have been to move >>> most of the code to a module ("patch-manager" say) and have the patman >>> script import that. The same would work for anything else that wanted >>> to bring in bits of patman (buildman perhaps?). >> >> Ah OK. We could do this, but what is the benefit? Buildman currently >> imports the particular modules it needs and doesn't use the top-level >> tool. >> > > That may play into the de-coupling of u-boot and patman. Some of these > common bits could be yet another re-usable component that both patman > and buildman use. But then you'd have to come up with a name for such > a component which we all know is a NP-hard problem :). > > I haven't really looked at buildman and only have a vague > understanding of what it does. Do you think it too would be useful > outside of u-boot? Potentially although it would be a bit of work. How many projects build for 1000 boards and want to collate common bisectability errors between them? Regards, Simon
On 30 July 2015 at 23:34, Chris Packham <judge.packham@gmail.com> wrote: > Hi Simon, > > On Fri, Jul 31, 2015 at 7:47 AM, Simon Glass <sjg@chromium.org> wrote: >> Commit 488d19c (patman: add distutils based installer) has the side effect >> of making patman run twice with each invocation. Fix this by checking for >> 'main program' invocation in patman.py. This is good practice in any case. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- > > Reviewed-by: Chris Packham <judge.packham@gmail.com> Applied to u-boot-x86 and now in mainline.
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index e76fc42..6fe8fe0 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -74,8 +74,11 @@ specified by tags you place in the commits. Use -n to do a dry run first.""" settings.Setup(parser, options.project, '') (options, args) = parser.parse_args() +if __name__ != "__main__": + pass + # Run our meagre tests -if options.test: +elif options.test: import doctest sys.argv = [sys.argv[0]]
Commit 488d19c (patman: add distutils based installer) has the side effect of making patman run twice with each invocation. Fix this by checking for 'main program' invocation in patman.py. This is good practice in any case. Signed-off-by: Simon Glass <sjg@chromium.org> --- tools/patman/patman.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)