[Buildroot] [PATCH] infra: add the transient download mechanism
Arnout Vandecappelle
arnout at mind.be
Thu Jan 16 22:49:10 UTC 2020
On 16/01/2020 16:29, Yann E. MORIN wrote:
> Nicolas, All,
>
> On 2020-01-16 09:11 +0000, Nicolas Carrier spake thusly:
>> Hello Yann,
>> One small remark, to be able to reproduce the build, on top of this
>> mechanism, there should be:
>> * a way to store the actual VCS revision used for the build (to at
>> least know what was just built!)
>> * a way to force the build to re-use a particular set of revisions
>>
>> That's features not easy to get right, but which are already handled by
>> tools like google's repo (or partly, by git submodules).
>
> Well, that's already the case with Buildroot: setting _VERSION is what
> guarantees you can rebuild the same thing again and again.
>
> I don't know much of repo, but from what I grabbed, it uses a manifest
> to identify packages, where they are, what changesset to use (and some
> less interesting stuff). This is exactly what one has in Buildroot's
> .mk files. The manifest is just builtin to the buildsystem.
The difference is that repo doesn't need the changeset to use. If no changeset
is specified, then 'repo update' will do a pull of all branches. So the
transient download mechanism pretty much approximates that.
That said, there are clear advantages to using repo during development compared
to the transient download mechanism:
- possibility to branch or tag several trees in one shot;
- easy to fixate the manifest from a specific situation;
- no need to switch to yet another mechanism (OVERRIDE_SRCDIR isntead of
transient) when you want to make modifications.
So for a serious project, repo is a clear win.
However, for small projects, it's a bit overkill. For a smaller project, you'd
typically have only a few trees to manage (buildroot, external, linux,
application) and most of them are not terribly active, so the features of repo
don't bring a lot of added value. And managing the manifest file is definitely
more complicated than just specifying the linux git in your defconfig.
Bottom line: even though repo is the better tool to manage the trees both for
development and release, the transient mechanism is a good quick win.
>> That's part of why I don't think that this kind of feature is a good
>> thing to integrate into buildroot. I prefer to delegate it to tools
>> meant for that (using a mix of local packages and OVERRIDE).
>
> You don't need to convince me. But apparently, some people like to aim
> at their feet with a loaded gun, just in case.. ;-)
>
> On a more serious note, as Thomas explained, some people have expressed
> the need for using a branch, and that a build would alkways pick the
> latest version of that branch.
>
> Of course, that means non-reproducibility.
>
> I am even afraid that those people do not realise that they also need to
> update the _VERSION in their packages once they want to tag and release
> versions. And this has virtually no overhead in practice (damn...
> copy-pasting a sha1 is not that complex. It would even be possible to
> have a script that does it automatically...).
>
>> And like said before, I think it's preferable to have a optional
>> feature to prevent branches from being used, making the build fail.
>
> Yeah, that was one of the comment by Thomas, and it is a good extension
> to the transient mehcanism: either one uses a stable ref, or one needs
> to explicitly identify it as transient.
We already have patches for that in patchwork.
With those, we'd need to propagate the -F option down to the git helper, so it
can avoid erroring out.
Regards,
Arnout
>
> Thanks for the feedback.
>
> Regards,
> Yann E. MORIN.
>
>> On Wed, 2020-01-15 at 21:37 +0100, Yann E. MORIN wrote:
>>> NOTE: NOT TO BE COMMITTED!
>>>
>>> This patch is a proof of concept for the transient download
>>> mechanism,
>>> that would help using branches as versions, and keep using the head
>>> of
>>> the branch each time a build started.
>>>
>>> A package declares its download as transient with:
>>> FOO_DOWNLOAD_TRANSIENT = YES
>>>
>>> this means that the download infra will not use any already
>>> downloaded
>>> archive, and will instead always download it as if missing.
>>>
>>> Since the check is done in the download wrapper, we have no TOCTOU
>>> race
>>> in case two bulds would attempt the same transient download: the
>>> archive
>>> is only replaced ato,mically as usual.
>>>
>>> So, if the package uses a branch as version, the branch's HEAD at
>>> that
>>> very moment will be redownloaded.
>>>
>>> This stil has the drawback that two builds in parallel may get
>>> slightly
>>> different content for the same branch, and the first build could end
>>> up
>>> using the download of the second build:
>>>
>>> build-1 build-2
>>>
>>> download
>>> | download
>>> | |
>>> save to dl-dir |
>>> [...] save to dl-dir
>>> extract
>>>
>>> Furthermore, even with a single build, it might get what it expects:
>>>
>>> developer-1 developer-2
>>>
>>> git push branch
>>> trigger CI git push branch
>>> [...]
>>> download
>>>
>>> In that case the build of delopper-1 would get the code of
>>> developper-2
>>> who pushed on the same branch.
>>>
>>> For people who are aiming at their feet, we're now providing them
>>> with
>>> a loaded gun. ;-]
>>>
>>> Signed-off-by: Yann E. MORIN <yann.morin.1998 at free.fr>
>>> Cc: Thomas Petazzoni <thomas.petazzoni at bootlin.com>
>>> Cc: Vincent Fazio <vfazio at xes-inc.com>
>>> Cc: Michael Nazzareno Trimarchi <michael at amarulasolutions.com>
>>> Cc: Chris Packham <judge.packham at gmail.com>
>>> Cc: Nicolas Carrier <nicolas.carrier at orolia.com>
>>> Cc: Adam Duskett <aduskett at gmail.com>
>>> ---
>>> docs/manual/adding-packages-generic.txt | 16 +++++++++++++---
>>> package/busybox/busybox.mk | 1 +
>>> package/pkg-download.mk | 1 +
>>> package/pkg-generic.mk | 8 +-------
>>> support/download/dl-wrapper | 8 +++++---
>>> 5 files changed, 21 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/docs/manual/adding-packages-generic.txt
>>> b/docs/manual/adding-packages-generic.txt
>>> index baa052e31c..afffb09bc8 100644
>>> --- a/docs/manual/adding-packages-generic.txt
>>> +++ b/docs/manual/adding-packages-generic.txt
>>> @@ -206,12 +206,14 @@ information is (assuming the package name is
>>> +libfoo+) :
>>> ** a tag for a git tree +LIBFOO_VERSION = v0.1.2+
>>> +
>>> .Note:
>>> -Using a branch name as +FOO_VERSION+ is not supported, because it
>>> does
>>> -not and can not work as people would expect it should:
>>> +Using a branch name as +FOO_VERSION+, although technically possible,
>>> +is highly discouraged, because it does not and can not work as
>>> people
>>> +would expect it should:
>>> +
>>> 1. due to local caching, Buildroot will not re-fetch the
>>> repository,
>>> so people who expect to be able to follow the remote repository
>>> - would be quite surprised and disappointed;
>>> + would be quite surprised and disappointed (but see
>>> + +LIBFOO_DOWNLOAD_TRANSIENT+, later);
>>> 2. because two builds can never be perfectly simultaneous, and
>>> because
>>> the remote repository may get new commits on the branch
>>> anytime,
>>> two users, using the same Buildroot tree and building the same
>>> @@ -342,6 +344,14 @@ not and can not work as people would expect it
>>> should:
>>> submodules when they contain bundled libraries, in which case we
>>> prefer to use those libraries from their own package.
>>>
>>> +* +LIBFOO_DOWNLOAD_TRANSIENT+ can be set to +YES+ or +NO+ (the
>>> default).
>>> + If set to +YES+, the download for that package will be attempted
>>> at
>>> + every build; any already downloaded archive is ignored as if
>>> missing.
>>> + This may help when a branch is specified in +LIBFOO_VERSION+, and
>>> the
>>> + head/tip of the branch is to be built, like a CI pipeline would
>>> need
>>> + for example. This still suffers from the other issues listed above
>>> + about using branches, though.
>>> +
>>> * +LIBFOO_STRIP_COMPONENTS+ is the number of leading components
>>> (directories) that tar must strip from file names on extraction.
>>> The tarball for most packages has one leading component named
>>> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
>>> index 6283bc96ea..ee17febe48 100644
>>> --- a/package/busybox/busybox.mk
>>> +++ b/package/busybox/busybox.mk
>>> @@ -9,6 +9,7 @@ BUSYBOX_SITE = http://www.busybox.net/downloads
>>> BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2
>>> BUSYBOX_LICENSE = GPL-2.0
>>> BUSYBOX_LICENSE_FILES = LICENSE
>>> +BUSYBOX_DOWNLOAD_TRANSIENT = YES
>>>
>>> define BUSYBOX_HELP_CMDS
>>> @echo ' busybox-menuconfig - Run BusyBox menuconfig'
>>> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
>>> index de619ba90a..c3a98ce7b0 100644
>>> --- a/package/pkg-download.mk
>>> +++ b/package/pkg-download.mk
>>> @@ -108,6 +108,7 @@ define DOWNLOAD
>>> -n '$($(2)_BASENAME_RAW)' \
>>> -N '$($(2)_RAWNAME)' \
>>> -o '$($(2)_DL_DIR)/$(notdir $(1))' \
>>> + $(if $($(2)_DOWNLOAD_TRANSIENT),-F) \
>>> $(if $($(2)_GIT_SUBMODULES),-r) \
>>> $(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u
>>> $(uri)) \
>>> $(QUIET) \
>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>> index 268d999efb..7c2e9db604 100644
>>> --- a/package/pkg-generic.mk
>>> +++ b/package/pkg-generic.mk
>>> @@ -158,13 +158,7 @@ $(BUILD_DIR)/%/.stamp_downloaded:
>>> @$(call step_start,download)
>>> $(call prepare-per-package-
>>> directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
>>> $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call
>>> $(hook))$(sep))
>>> -# Only show the download message if it isn't already downloaded
>>> - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \
>>> - if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \
>>> - $(call MESSAGE,"Downloading") ; \
>>> - break ; \
>>> - fi ; \
>>> - done
>>> + @$(call MESSAGE,"Downloading") ; \
>>> $(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call
>>> DOWNLOAD,$(p),$(PKG))$(sep))
>>> $(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call
>>> $(hook))$(sep))
>>> $(Q)mkdir -p $(@D)
>>> diff --git a/support/download/dl-wrapper b/support/download/dl-
>>> wrapper
>>> index 3315bd410e..ab22ca7e4f 100755
>>> --- a/support/download/dl-wrapper
>>> +++ b/support/download/dl-wrapper
>>> @@ -21,11 +21,12 @@ export
>>> BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
>>>
>>> main() {
>>> local OPT OPTARG
>>> - local backend output hfile recurse quiet rc
>>> + local backend output hfile recurse quiet rc force
>>> local -a uris
>>>
>>> # Parse our options; anything after '--' is for the backend
>>> - while getopts ":c:d:D:o:n:N:H:rf:u:q" OPT; do
>>> + force=false
>>> + while getopts ":c:d:D:o:n:N:H:rf:u:qF" OPT; do
>>> case "${OPT}" in
>>> c) cset="${OPTARG}";;
>>> d) dl_dir="${OPTARG}";;
>>> @@ -38,6 +39,7 @@ main() {
>>> f) filename="${OPTARG}";;
>>> u) uris+=( "${OPTARG}" );;
>>> q) quiet="-q";;
>>> + F) force=true;;
>>> :) error "option '%s' expects a mandatory argument\n"
>>> "${OPTARG}";;
>>> \?) error "unknown option '%s'\n" "${OPTARG}";;
>>> esac
>>> @@ -67,7 +69,7 @@ main() {
>>> # - matches all its hashes: do not download it again and exit
>>> promptly
>>> # - fails at least one of its hashes: force a re-download
>>> # - there's no hash (but a .hash file): consider it a hard error
>>> - if [ -e "${output}" ]; then
>>> + if ! ${force} && [ -e "${output}" ]; then
>>> if support/download/check-hash ${quiet} "${hfile}"
>>> "${output}" "${output##*/}"; then
>>> exit 0
>>> elif [ ${?} -ne 2 ]; then
>>> --
>>> 2.20.1
>>>
>>> ATTENTION: This email came from an external source.
>>> Do not open attachments or click on links from unknown senders or
>>> unexpected emails.
>>
>
More information about the buildroot
mailing list