Hi Andrejs,

On Tue, 25 Jun 2024 at 16:16, Andrejs Cainikovs
<andrejs.cainik...@toradex.com> wrote:
>
> On Tue, Jun 25, 2024 at 01:38:07PM +0100, Simon Glass wrote:
> > Hi Andrejs,
> >
> > On Tue, 25 Jun 2024 at 01:06, Andrejs Cainikovs
> > <andrejs.cainik...@toradex.com> wrote:
> > >
> > > On Sun, Jun 23, 2024 at 11:56:20AM -0600, Simon Glass wrote:
> > > > The feature to set the toolchain path does not seem to be needed. It
> > > > causes problems with venv (see [1]). Let's remove it.
> > > >
> > > > Add some tests while we are here.
> > > >
> > > > It does not look like any docs changes are needed for this.
> > > >
> > > > [1] 
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20240621131423.2363294-6-...@chromium.org/
> > > >
> > > > Signed-off-by: Simon Glass <s...@chromium.org>
> > > > Suggested-by: Tom Rini <tr...@konsulko.com>
> > > > ---
> > > >
> > > > Changes in v3:
> > > > - Drop the PATH modification altogether
> > > >
> > > >  tools/buildman/bsettings.py     |  3 ++
> > > >  tools/buildman/builder.py       |  5 +--
> > > >  tools/buildman/builderthread.py |  4 +-
> > > >  tools/buildman/cmdline.py       |  2 -
> > > >  tools/buildman/control.py       |  6 +--
> > > >  tools/buildman/test.py          | 75 +++++++++++++++++++++++++++++++++
> > > >  tools/buildman/toolchain.py     | 20 ++++-----
> > > >  7 files changed, 92 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
> > > > index e225ac2ca0f..1be1d45e0fa 100644
> > > > --- a/tools/buildman/bsettings.py
> > > > +++ b/tools/buildman/bsettings.py
> > > > @@ -31,6 +31,9 @@ def setup(fname=''):
> > > >  def add_file(data):
> > > >      settings.readfp(io.StringIO(data))
> > > >
> > > > +def add_section(name):
> > > > +    settings.add_section(name)
> > > > +
> > > >  def get_items(section):
> > > >      """Get the items from a section of the config.
> > > >
> > > > diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
> > > > index f35175b4598..7c563cddada 100644
> > > > --- a/tools/buildman/builder.py
> > > > +++ b/tools/buildman/builder.py
> > > > @@ -255,7 +255,7 @@ class Builder:
> > > >
> > > >      def __init__(self, toolchains, base_dir, git_dir, num_threads, 
> > > > num_jobs,
> > > >                   gnu_make='make', checkout=True, show_unknown=True, 
> > > > step=1,
> > > > -                 no_subdirs=False, full_path=False, 
> > > > verbose_build=False,
> > > > +                 no_subdirs=False, verbose_build=False,
> > > >                   mrproper=False, per_board_out_dir=False,
> > > >                   config_only=False, squash_config_y=False,
> > > >                   warnings_as_errors=False, work_in_output=False,
> > > > @@ -279,8 +279,6 @@ class Builder:
> > > >              step: 1 to process every commit, n to process every nth 
> > > > commit
> > > >              no_subdirs: Don't create subdirectories when building 
> > > > current
> > > >                  source for a single board
> > > > -            full_path: Return the full path in CROSS_COMPILE and don't 
> > > > set
> > > > -                PATH
> > > >              verbose_build: Run build with V=1 and don't use 'make -s'
> > > >              mrproper: Always run 'make mrproper' when configuring
> > > >              per_board_out_dir: Build in a separate persistent 
> > > > directory per
> > > > @@ -336,7 +334,6 @@ class Builder:
> > > >          self._step = step
> > > >          self._error_lines = 0
> > > >          self.no_subdirs = no_subdirs
> > > > -        self.full_path = full_path
> > > >          self.verbose_build = verbose_build
> > > >          self.config_only = config_only
> > > >          self.squash_config_y = squash_config_y
> > > > diff --git a/tools/buildman/builderthread.py 
> > > > b/tools/buildman/builderthread.py
> > > > index a8599c0bb2a..c23c3254d2d 100644
> > > > --- a/tools/buildman/builderthread.py
> > > > +++ b/tools/buildman/builderthread.py
> > > > @@ -404,7 +404,7 @@ class BuilderThread(threading.Thread):
> > > >                      the next incremental build
> > > >          """
> > > >          # Set up the environment and command line
> > > > -        env = self.toolchain.MakeEnvironment(self.builder.full_path)
> > > > +        env = self.toolchain.MakeEnvironment()
> > > >          mkdir(out_dir)
> > > >
> > > >          args, cwd, src_dir = self._build_args(brd, out_dir, 
> > > > out_rel_dir,
> > > > @@ -569,7 +569,7 @@ class BuilderThread(threading.Thread):
> > > >                  outf.write(f'{result.return_code}')
> > > >
> > > >              # Write out the image and function size information and an 
> > > > objdump
> > > > -            env = 
> > > > result.toolchain.MakeEnvironment(self.builder.full_path)
> > > > +            env = result.toolchain.MakeEnvironment()
> > > >              with open(os.path.join(build_dir, 'out-env'), 'wb') as 
> > > > outf:
> > > >                  for var in sorted(env.keys()):
> > > >                      outf.write(b'%s="%s"' % (var, env[var]))
> > > > diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
> > > > index 03211bd5aa5..5fda90508f2 100644
> > > > --- a/tools/buildman/cmdline.py
> > > > +++ b/tools/buildman/cmdline.py
> > > > @@ -121,8 +121,6 @@ def add_after_m(parser):
> > > >            help="Override host toochain to use for sandbox (e.g. 
> > > > 'clang-7')")
> > > >      parser.add_argument('-Q', '--quick', action='store_true',
> > > >            default=False, help='Do a rough build, with limited warning 
> > > > resolution')
> > > > -    parser.add_argument('-p', '--full-path', action='store_true',
> > > > -          default=False, help="Use full toolchain path in 
> > > > CROSS_COMPILE")
> > > >      parser.add_argument('-P', '--per-board-out-dir', 
> > > > action='store_true',
> > > >            default=False, help="Use an O= (output) directory per board 
> > > > rather than per thread")
> > > >      parser.add_argument('--print-arch', action='store_true',
> > > > diff --git a/tools/buildman/control.py b/tools/buildman/control.py
> > > > index 8f6850c5211..3ca9e2e8761 100644
> > > > --- a/tools/buildman/control.py
> > > > +++ b/tools/buildman/control.py
> > > > @@ -653,10 +653,8 @@ def do_buildman(args, toolchains=None, 
> > > > make_func=None, brds=None,
> > > >      builder = Builder(toolchains, output_dir, git_dir,
> > > >              args.threads, args.jobs, checkout=True,
> > > >              show_unknown=args.show_unknown, step=args.step,
> > > > -            no_subdirs=args.no_subdirs, full_path=args.full_path,
> > > > -            verbose_build=args.verbose_build,
> > > > -            mrproper=args.mrproper,
> > > > -            per_board_out_dir=args.per_board_out_dir,
> > > > +            no_subdirs=args.no_subdirs, 
> > > > verbose_build=args.verbose_build,
> > > > +            mrproper=args.mrproper, 
> > > > per_board_out_dir=args.per_board_out_dir,
> > > >              config_only=args.config_only,
> > > >              squash_config_y=not args.preserve_config_y,
> > > >              warnings_as_errors=args.warnings_as_errors,
> > > > diff --git a/tools/buildman/test.py b/tools/buildman/test.py
> > > > index f92add7a7c5..ae9963eed4f 100644
> > > > --- a/tools/buildman/test.py
> > > > +++ b/tools/buildman/test.py
> > > > @@ -146,6 +146,7 @@ class TestBuild(unittest.TestCase):
> > > >          self.toolchains.Add('arm-linux-gcc', test=False)
> > > >          self.toolchains.Add('sparc-linux-gcc', test=False)
> > > >          self.toolchains.Add('powerpc-linux-gcc', test=False)
> > > > +        self.toolchains.Add('/path/to/aarch64-linux-gcc', test=False)
> > >
> > > Sorry Simon, but me and others love to be consistent.
> >
> > That's fine, but do you have a comment on this patch?
> >
> > Regards,
> > Simon
>
> I was just thinking about aarch64-linux-gcc (should it be
> aarch64-linux-gnu-gcc, btw?)  without path for consistency.
> But this of course is very minor - feel free to disregard my
> comment.

Oh I see...the point here is to test a toolchain which has a path
prepended to it, That is why this test case is not consistent with the
others.

Regards,
Simon

Reply via email to