* [PATCH v2] binman: Get futility by building it
@ 2022-09-12 13:35 Simon Glass
2022-09-13 8:54 ` Quentin Schulz
0 siblings, 1 reply; 3+ messages in thread
From: Simon Glass @ 2022-09-12 13:35 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Alper Nebi Yasak, Quentin Schulz, Stefan Herbrechtsmeier, Simon Glass
A binary download is not great, since it depends on libraries being
present in the system. Build futility from source instead.
Note that this requires two patches to the source repo which are in
progress:
https://issuetracker.google.com/issues/245993083?pli=1
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Pull from github instead, to avoid needing to login / gitcookies
tools/binman/bintool.py | 17 +++++++++++++----
tools/binman/btool/futility.py | 12 ++++++++----
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
index 032179a99de..52ec0030590 100644
--- a/tools/binman/bintool.py
+++ b/tools/binman/bintool.py
@@ -319,7 +319,8 @@ class Bintool:
return result.stdout
@classmethod
- def build_from_git(cls, git_repo, make_target, bintool_path):
+ def build_from_git(cls, git_repo, make_target, bintool_path, flags=None,
+ branch=None):
"""Build a bintool from a git repo
This clones the repo in a temporary directory, builds it with 'make',
@@ -330,6 +331,8 @@ class Bintool:
make_target (str): Target to pass to 'make' to build the tool
bintool_path (str): Relative path of the tool in the repo, after
build is complete
+ flags (list of str): Flags or variables to pass to make, or None
+ branch (str): Branch to build, None for the default
Returns:
tuple:
@@ -339,10 +342,16 @@ class Bintool:
"""
tmpdir = tempfile.mkdtemp(prefix='binmanf.')
print(f"- clone git repo '{git_repo}' to '{tmpdir}'")
- tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir)
+ args = ['git', 'clone', '--depth', '1']
+ if branch:
+ args += ['-b', branch]
+ tools.run(*args, git_repo, tmpdir)
print(f"- build target '{make_target}'")
- tools.run('make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
- make_target)
+ cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
+ make_target]
+ if flags:
+ cmd += flags
+ tools.run(*cmd)
fname = os.path.join(tmpdir, bintool_path)
if not os.path.exists(fname):
print(f"- File '{fname}' was not produced")
diff --git a/tools/binman/btool/futility.py b/tools/binman/btool/futility.py
index 75a05c2ac66..121a366830b 100644
--- a/tools/binman/btool/futility.py
+++ b/tools/binman/btool/futility.py
@@ -160,8 +160,12 @@ class Bintoolfutility(bintool.Bintool):
Raises:
Valuerror: Fetching could not be completed
"""
- if method != bintool.FETCH_BIN:
+ if method != bintool.FETCH_BUILD:
return None
- fname, tmpdir = self.fetch_from_drive(
- '1hdsInzsE4aJbmBeJ663kYgjOQyW1I-E0')
- return fname, tmpdir
+ result = self.build_from_git(
+ 'https://github.com/sjg20/vboot_reference.git',
+ 'all',
+ 'build/futility/futility',
+ flags=['USE_FLASHROM=0'],
+ branch='fut')
+ return result
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] binman: Get futility by building it
2022-09-12 13:35 [PATCH v2] binman: Get futility by building it Simon Glass
@ 2022-09-13 8:54 ` Quentin Schulz
2022-09-14 12:49 ` Simon Glass
0 siblings, 1 reply; 3+ messages in thread
From: Quentin Schulz @ 2022-09-13 8:54 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Alper Nebi Yasak, Stefan Herbrechtsmeier
Hi Simon,
On 9/12/22 15:35, Simon Glass wrote:
> A binary download is not great, since it depends on libraries being
> present in the system. Build futility from source instead.
>
> Note that this requires two patches to the source repo which are in
> progress:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__issuetracker.google.com_issues_245993083-3Fpli-3D1&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=aZRnLGuudyF3wf57Yf7GAac-g9Rgf3zQuq1O9GJPIdIhZ_w2XDHpPsZGFQAIMFT9&s=j0aTJGKOhRncvmyFptYpT_Y9Qb3U3CDUiqG2jO_7hAQ&e=
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Pull from github instead, to avoid needing to login / gitcookies
>
> tools/binman/bintool.py | 17 +++++++++++++----
> tools/binman/btool/futility.py | 12 ++++++++----
> 2 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
> index 032179a99de..52ec0030590 100644
> --- a/tools/binman/bintool.py
> +++ b/tools/binman/bintool.py
> @@ -319,7 +319,8 @@ class Bintool:
> return result.stdout
>
> @classmethod
> - def build_from_git(cls, git_repo, make_target, bintool_path):
> + def build_from_git(cls, git_repo, make_target, bintool_path, flags=None,
> + branch=None):
> """Build a bintool from a git repo
>
> This clones the repo in a temporary directory, builds it with 'make',
> @@ -330,6 +331,8 @@ class Bintool:
> make_target (str): Target to pass to 'make' to build the tool
> bintool_path (str): Relative path of the tool in the repo, after
> build is complete
> + flags (list of str): Flags or variables to pass to make, or None
> + branch (str): Branch to build, None for the default
>
> Returns:
> tuple:
> @@ -339,10 +342,16 @@ class Bintool:
> """
> tmpdir = tempfile.mkdtemp(prefix='binmanf.')
> print(f"- clone git repo '{git_repo}' to '{tmpdir}'")
> - tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir)
> + args = ['git', 'clone', '--depth', '1']
> + if branch:
> + args += ['-b', branch]
I don't like branches too much, a commit hash would probably be better
for reproducibility, we would need a git checkout command though since
git clone command does not allow for commit hashes AFAIR. Up to you I guess.
> + tools.run(*args, git_repo, tmpdir)
> print(f"- build target '{make_target}'")
> - tools.run('make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
> - make_target)
> + cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
> + make_target]
> + if flags:
> + cmd += flags > + tools.run(*cmd)
> fname = os.path.join(tmpdir, bintool_path)
> if not os.path.exists(fname):
> print(f"- File '{fname}' was not produced")
> diff --git a/tools/binman/btool/futility.py b/tools/binman/btool/futility.py
> index 75a05c2ac66..121a366830b 100644
> --- a/tools/binman/btool/futility.py
> +++ b/tools/binman/btool/futility.py
> @@ -160,8 +160,12 @@ class Bintoolfutility(bintool.Bintool):
> Raises:
> Valuerror: Fetching could not be completed
> """
> - if method != bintool.FETCH_BIN:
> + if method != bintool.FETCH_BUILD:
> return None
> - fname, tmpdir = self.fetch_from_drive(
> - '1hdsInzsE4aJbmBeJ663kYgjOQyW1I-E0')
> - return fname, tmpdir
> + result = self.build_from_git(
> + 'https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sjg20_vboot-5Freference.git&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=aZRnLGuudyF3wf57Yf7GAac-g9Rgf3zQuq1O9GJPIdIhZ_w2XDHpPsZGFQAIMFT9&s=4I4vh_QaOw3S9ET9XnKHOHUkf7Qu3SGMGfUKNe5Uxe4&e= ',
> + 'all',
> + 'build/futility/futility',
> + flags=['USE_FLASHROM=0'],
> + branch='fut')
> + return result
Seems to be doing the job, it fetches and builds the futility binary.
Having some hard time figuring out how to test it produces a valid
binary it seems most of the calls to futility in ftest.py are faked/mocked?
Also, would be great to have the DOWNLOAD_DESTDIR part of toolpath as I
found it quite surprising to run binman tool -f missing and still have
the binary not appear when running binman tool --list. But that's not
related to this commit :)
Cheers,
Quentin
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] binman: Get futility by building it
2022-09-13 8:54 ` Quentin Schulz
@ 2022-09-14 12:49 ` Simon Glass
0 siblings, 0 replies; 3+ messages in thread
From: Simon Glass @ 2022-09-14 12:49 UTC (permalink / raw)
To: Quentin Schulz
Cc: U-Boot Mailing List, Alper Nebi Yasak, Stefan Herbrechtsmeier
Hi Quentin,
On Tue, 13 Sept 2022 at 02:55, Quentin Schulz
<quentin.schulz@theobroma-systems.com> wrote:
>
> Hi Simon,
>
> On 9/12/22 15:35, Simon Glass wrote:
> > A binary download is not great, since it depends on libraries being
> > present in the system. Build futility from source instead.
> >
> > Note that this requires two patches to the source repo which are in
> > progress:
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__issuetracker.google.com_issues_245993083-3Fpli-3D1&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=aZRnLGuudyF3wf57Yf7GAac-g9Rgf3zQuq1O9GJPIdIhZ_w2XDHpPsZGFQAIMFT9&s=j0aTJGKOhRncvmyFptYpT_Y9Qb3U3CDUiqG2jO_7hAQ&e=
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Pull from github instead, to avoid needing to login / gitcookies
> >
> > tools/binman/bintool.py | 17 +++++++++++++----
> > tools/binman/btool/futility.py | 12 ++++++++----
> > 2 files changed, 21 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
> > index 032179a99de..52ec0030590 100644
> > --- a/tools/binman/bintool.py
> > +++ b/tools/binman/bintool.py
> > @@ -319,7 +319,8 @@ class Bintool:
> > return result.stdout
> >
> > @classmethod
> > - def build_from_git(cls, git_repo, make_target, bintool_path):
> > + def build_from_git(cls, git_repo, make_target, bintool_path, flags=None,
> > + branch=None):
> > """Build a bintool from a git repo
> >
> > This clones the repo in a temporary directory, builds it with 'make',
> > @@ -330,6 +331,8 @@ class Bintool:
> > make_target (str): Target to pass to 'make' to build the tool
> > bintool_path (str): Relative path of the tool in the repo, after
> > build is complete
> > + flags (list of str): Flags or variables to pass to make, or None
> > + branch (str): Branch to build, None for the default
> >
> > Returns:
> > tuple:
> > @@ -339,10 +342,16 @@ class Bintool:
> > """
> > tmpdir = tempfile.mkdtemp(prefix='binmanf.')
> > print(f"- clone git repo '{git_repo}' to '{tmpdir}'")
> > - tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir)
> > + args = ['git', 'clone', '--depth', '1']
> > + if branch:
> > + args += ['-b', branch]
>
> I don't like branches too much, a commit hash would probably be better
> for reproducibility, we would need a git checkout command though since
> git clone command does not allow for commit hashes AFAIR. Up to you I guess.
Well I plan land the required patch upstream so will be able to use
the master branch. It's a shame that Chromium git requires an account.
>
> > + tools.run(*args, git_repo, tmpdir)
> > print(f"- build target '{make_target}'")
> > - tools.run('make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
> > - make_target)
> > + cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
> > + make_target]
> > + if flags:
> > + cmd += flags > + tools.run(*cmd)
> > fname = os.path.join(tmpdir, bintool_path)
> > if not os.path.exists(fname):
> > print(f"- File '{fname}' was not produced")
> > diff --git a/tools/binman/btool/futility.py b/tools/binman/btool/futility.py
> > index 75a05c2ac66..121a366830b 100644
> > --- a/tools/binman/btool/futility.py
> > +++ b/tools/binman/btool/futility.py
> > @@ -160,8 +160,12 @@ class Bintoolfutility(bintool.Bintool):
> > Raises:
> > Valuerror: Fetching could not be completed
> > """
> > - if method != bintool.FETCH_BIN:
> > + if method != bintool.FETCH_BUILD:
> > return None
> > - fname, tmpdir = self.fetch_from_drive(
> > - '1hdsInzsE4aJbmBeJ663kYgjOQyW1I-E0')
> > - return fname, tmpdir
> > + result = self.build_from_git(
> > + 'https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sjg20_vboot-5Freference.git&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=aZRnLGuudyF3wf57Yf7GAac-g9Rgf3zQuq1O9GJPIdIhZ_w2XDHpPsZGFQAIMFT9&s=4I4vh_QaOw3S9ET9XnKHOHUkf7Qu3SGMGfUKNe5Uxe4&e= ',
> > + 'all',
> > + 'build/futility/futility',
> > + flags=['USE_FLASHROM=0'],
> > + branch='fut')
> > + return result
>
> Seems to be doing the job, it fetches and builds the futility binary.
> Having some hard time figuring out how to test it produces a valid
> binary it seems most of the calls to futility in ftest.py are faked/mocked?
Yes I think they all are. It would be easy enough to have a test which
generates a real GBB and vblock and is skipped if futility is missing.
>
> Also, would be great to have the DOWNLOAD_DESTDIR part of toolpath as I
> found it quite surprising to run binman tool -f missing and still have
> the binary not appear when running binman tool --list. But that's not
> related to this commit :)
Yes, would you like to send a patch? The --toolpath arg is intended to
allow multiple paths to be provided, but I think it would be OK to use
it for what you say here. Perhap also support a BINMAN_TOOLPATH env
var?
Regards,
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-14 12:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-12 13:35 [PATCH v2] binman: Get futility by building it Simon Glass
2022-09-13 8:54 ` Quentin Schulz
2022-09-14 12:49 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.