[Buildroot] [PATCH 1/1] support/download/git: add branch support
Yann E. MORIN
yann.morin.1998 at free.fr
Wed May 8 16:23:09 UTC 2019
Catherine, All,
[please keep the buildroot ML in Cc on replies; if you're not subscribed,
please do so, otherwise people can't follow the discussion and wonder
what's going on...]
On 2019-05-08 10:47 -0500, Catherine Garabedian spake thusly:
> Understood. I'm upgrading from an older version of Buildroot, so missed that change.
> I'll have to update our configurations to adjust to the new behavior.
Note that there was no change; that has always been the case. It's just
that it's now documented.
> I would like to update this code to add an explicit error message
> instead ("Change sets may not be referred to by branch name"). Should
> that be submitted as a revision to this patch, or as an entirely
> separate patch?
I already posted a series to that effect in the past, and it is not
trivial to do.
The problem is that for some time, we allowed (and still allow) for
so-called special refs, like github PR, or gerrit reviews, and so on.
Due to a bug in the git download backend, when the cset to use is a
sha1, this does create local branches named after that sha1.
So, if we add a check that a cset is a branch name or not, then even if
the cset is a sha1, it can match a local branch (that was created by an
older version of Buildroot).
As such, we first have to clean up the git cache.
I have two WIP branches to address this issue [0] and [1], but I haven't
come to terms with previous reviews (which I can't seem to find again in
the ML archives, and I don;t even remember why the '3' is shorter than
the '2', which looks more complete, sigh...) You can get a better
picture of the issue by reading the commit logs in those series.
[0] https://git.buildroot.org/~ymorin/git/buildroot/log/?h=yem/dl-git-no-branch-2
[1] https://git.buildroot.org/~ymorin/git/buildroot/log/?h=yem/dl-git-no-branch-3
I shall revive those and get them to their conclusion. Soon, I hope,
soon (famous last words)...
Regards,
Yann E. MORIN.
> Thanks,
> Catherine Garabedian
> On Wed, May 8, 2019 at 9:43 AM Yann E. MORIN < [1]yann.morin.1998 at free.fr> wrote:
>
> CAtherine, All,
>
> On 2019-05-08 14:22 +0000, [2]catherine at kubos.co spake thusly:
> > From: Catherine Garabedian < [3]catherine at kubos.co>
> >
> > A prior patch added logic to verify that the given cset exists.
> >
> > The command format used fails when attempting to verify a branch name, thus
> > preventing Buildroot from downloading any packages whose source comes from a
> > branch.
>
> We explciitly do not want to support using a branch _by name_, because
> that does not work as people expect it to work, see our manual:
>
> [4]https://buildroot.org/downloads/manual/manual.html#generic-package-reference
>
> Excerpt:
>
> 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;
> 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
> configuration, may get different source, thus rendering the build
> non reproducible, and people would be quite surprised and
> disappointed.
>
> So, using a branch _by name_ can not owrk because of those two reasons,
> and as such we believe this should not be supported.
>
> Instead, there was some effort a few months ago, to actually enforce the
> check that a branch was not used, and fail if that was the case.
>
> Regards,
> Yann E. MORIN.
>
> > This patch adds a secondary check which can validate branches (but not tags).
> >
> > Signed-off-by: Catherine Garabedian < [5]catherine at kubos.co>
> > ---
> > support/download/git | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/support/download/git b/support/download/git
> > index 17ca04e..a25552e 100755
> > --- a/support/download/git
> > +++ b/support/download/git
> > @@ -130,8 +130,10 @@ fi
> > # scratch won't help, so we don't want to trash the repository for a
> > # missing commit. We just exit without going through the ERR trap.
> > if ! _git rev-parse --quiet --verify "'${cset}^{commit}'" >/dev/null 2>&1; then
> > - printf "Commit '%s' does not exist in this repository\n." "${cset}"
> > - exit 1
> > + if ! _git rev-parse --quiet --verify "'origin/${cset}'" >/dev/null 2>&1; then
> > + printf "'%s' does not exist in this repository\n." "${cset}"
> > + exit 1
> > + fi
> > fi
> >
> > # The new cset we want to checkout might have different submodules, or
> > --
> > 2.7.4
> >
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> | [6]http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
>
> Links:
> 1. mailto:yann.morin.1998 at free.fr
> 2. mailto:catherine at kubos.co
> 3. mailto:catherine at kubos.co
> 4. https://buildroot.org/downloads/manual/manual.html#generic-package-reference
> 5. mailto:catherine at kubos.co
> 6. http://ymorin.is-a-geek.org/
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
More information about the buildroot
mailing list