Discussion:
[Buildroot] [PATCH 1/2] git: fix handling of git repos using master as version
Robert Beckett
2018-06-04 13:32:29 UTC
Permalink
Since changing to git init followed by fetch to initialise
git cache, when using master branch as the target version
for a package, git refuses to fetch in to currently checkout
out branch or master.

git init always creates a master branch, so this will always
be an issue for any package targeting master.
Add -u option to git fetch command to allow it to ignore that
check. We know that it is okay to overwrite master as we know that
it either contains nothing after the initial git init command, or
is tracking the remote master after a successful fetch.
We always want it to overwrite it.

Signed-off-by: Robert Beckett <***@netvu.org.uk>
---
support/download/git | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/download/git b/support/download/git
index 11bb52c..c33e692 100755
--- a/support/download/git
+++ b/support/download/git
@@ -122,7 +122,7 @@ _git fetch origin -t
# below, if there is an issue anyway. Since most of the cset we're gonna
# have to clone are not such special refs, consign the output to oblivion
# so as not to alarm unsuspecting users, but still trace it as a warning.
-if ! _git fetch origin "'${cset}:${cset}'" >/dev/null 2>&1; then
+if ! _git fetch -u origin "'${cset}:${cset}'" >/dev/null 2>&1; then
printf "Could not fetch special ref '%s'; assuming it is not special.\n" "${cset}"
fi
--
2.7.4
Bob Beckett
2018-06-04 14:59:29 UTC
Permalink
Is this philosophy set in stone? I can conceive of useful situations where
branches are preferable.
During large projects, where the rootfs is being developed at the same time
as some custom packages,
it is quite useful to have a branch as the target. Usually, I just do a
dirclean and delete the tar when I know
something has changed.

If you wanted to make it more predictable, then maybe name the tar based on
the version (branch name in this case) plus the sha, this way new tar
packages would be created if the branch advances and also if a tag (or
other supported refspec) changes via a rebase etc.

The particular thing I like to avoid is multiple version bumps in the
package spec and tags in the package source repos during development. When
many people are working on package project, expecting them all to take time
to publish tags for every little change gets unrealistic.

The question I would ask is is there any harm in supporting that if it can
be done reliably and predictably?

Anyway, if this design is locked down, then I will just maintain this patch
locally for us to use :)

Thanks

Bob


On Mon, Jun 4, 2018 at 3:33 PM, Thomas Petazzoni <
Hello,
Post by Robert Beckett
Since changing to git init followed by fetch to initialise
git cache, when using master branch as the target version
for a package, git refuses to fetch in to currently checkout
out branch or master.
I think we don't want to support using branches as Git versions, as it
doesn't make sense, because it won't behave as the user expects.
If you use a branch as _VERSION for a package, Buildroot will only
download once, and never update to newer commits in this branch,
because it will already have a cached tarball in its DL_DIR.
So I'm not sure we want to add some code to support a use case that we
don't want support :-)
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Thomas Petazzoni
2018-06-05 07:57:35 UTC
Permalink
Hello,
Maybe a variable _IN_DEVELOPMENT or something could be added to a package's
makefile to indicate that the version is always considered out of date, and
should be re-fetched.
We're never going to consider something *always* out of date, I think. We might
only consider it out-of-date when you do an explicit foo-rebuild.
Well, never say "never" :-)

During the development phase, if you have 50 or 100 in-house
components, it may be annoying to do a "foo-rebuild" for all of them in
order to get their latest version. But this can probably be resolved by
having a custom target (implemented locally by people who want that)
that triggers the rebuild of all packages they are interested in.
That way development branches for package specs could have the package spec
committed with that, while release branches do not and are expected to have a
non-branch version specifier.
With this strategy you would be acknowledging that you are not getting
reproducibility, but you are still getting reliable builds (the 2 reasons I use
buildroot).
The test for taint would then be to check if any packages have an
_IN_DEVELOPMENT variable set.
I'm not so fond of the _IN_DEVELOPMENT idea. What I like more is to extend
_OVERRIDE_SRCDIR to support repositories in addition to local files. It would
then check out directly into the build directory, without passing through DL_DIR
or a tarball.
This would probably be a rather complicated change however.
Is it that complicated to add a:

cd $(pkg_OVERRIDE_SRCDIR); git pull

before doing the rsync of the source code ? Of course, we don't want to
do this unconditionally as soon as <pkg>_OVERRIDE_SRCDIR is set, so
perhaps a separate boolean variable is needed.
Given the current state, if you specify a branch of master, you get a warning
that it doesnt appear to be a special version, but that is all, and the package
doesnt build at all as it failed to get the source in the first place> I think if there is going to be an enforcement of not using branches, then the
build should probably fail in a more explicit way e.g. check to see if the
version is a branch. If it is, then fail the build with a message telling the
user that branches are not to be used as versions
I completely agree with this one, we should simply fail on branches.
Agreed.

Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Thomas Petazzoni
2018-06-05 05:53:44 UTC
Permalink
Hello,
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/heads/master
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/remotes/upstream/master
Yes 'ls-remote' is actually a good option.
It's a bit racy though. By the time you do the actual fetch, the remote branch
may already have been updated by someone else.
And ?

Even if you get the latest commit in the branch, by the time the build
is finished, someone might have pushed some other changes.

There's nothing racy here: you are going to build the latest commit at
the moment of git ls-remote. There might obviously be further commits
made later on, either during the build or after the build. Whether
those commits were made right between ls-remote and the actual build,
or during the build, or after the build doesn't really matter.

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Bob Beckett
2018-06-04 16:54:30 UTC
Permalink
This post might be inappropriate. Click to display it.
Henrique Marks
2018-06-04 17:22:49 UTC
Permalink
Hello All,
Enviadas: Segunda-feira, 4 de junho de 2018 13:54:30
Assunto: Re: [Buildroot] [PATCH 1/2] git: fix handling of git repos using master
as version
On Mon, Jun 4, 2018 at 5:52 PM, Gary Bisson < [
Hi Bob
Thanks for the more in depth explanation.
I agree with all of the reasons outlined for the purpose of being purely a
reproducible build manager, which buildroot only ever aimed to be.
However, people do use it during development, and with a reasonably large
number of custom packages all in development at the same time, using the
_OVERRIDE_SRCDIR method to test latest versions of multiple packages
becomes very unwieldy.
My solution so far has been to point the version at the branch that active
development is taking place on, and removing the build directories and
packages for the specific projects each time.
When I am not the one doing the development on each package's source, but
am developing the rootfs, this means I dont have to manually keep updating
my local git repositories for each project.
Once each package has hit their first release version, they do get tagged
and the release branch for the project's BR2_EXTERNAL directory gets
updated with those versions, and each version thereafter, but the
development branch for the external directory persistently stays on the
development branch head for each package.
Regarding not knowing the sha before a checkout, would git ls-remote not
suffice for this? e.g.
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/heads/master
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/remotes/upstream/master
Yes 'ls-remote' is actually a good option. You could have the following
FOO_VERSION = $(shell git ls-remote URL branch | awk '{ print $$1 }')
Then it would pick up any change automatically until you finish your dev
and change the FOO_VERSION to a proper ID.
Thats actually a cracking idea. I like it (and I'm not sure why I didnt think of
it when suggesting it for detecting outdated named head versions :) )
It would allow you to check for changes in sha without doing a new fetch.
Oh well, not to worry. Ill keep it as a local patch, as I suspect I am
treading old ground of "buildroot is not a development platform" (Ive seen
this discussion come up multiple times before).
Yes that will most likely not be included inside BR infrastructure but that
should be a perfectly fine option for your custom package.
Regards,
Gary
_______________________________________________
buildroot mailing list
http://lists.busybox.net/mailman/listinfo/buildroot
We use buildroot for development AND for integration.

For development, you take your code, download the source and use OVERRIDE_SRCDIR as Thomas described. It works just fine.

For integration, every developer submits the working code to a Robot, that puts the SHA in the version field in the .mk file.

No use for branches, because sometimes the HEAD of the branch is not suitable either for development or integration.

But, just to make a point about differences when using buildroot this way: we are not too much interested in instrumenting buildroot, with those scripts that check the files, because they affect development (pkg-rebuild) and integration (overall time).

So, to please both the project and people using it for development/integration, i think the ideal would be to have the choice of not instrumenting a build, preferably on defconfigs.

Just to point an important (and noticeable) difference when you are developing with buildroot.

Regards,
--
Dr. Henrique Marks
***@datacom.ind.br
R. América, 1000 - Eldorado do Sul - RS
CEP: 92990-000 - Brasil
Fone: +55 51 3933 3000 - Ramal 3466
Trent Piepho
2018-06-04 22:45:42 UTC
Permalink
Post by Bob Beckett
Regarding not knowing the sha before a checkout, would git ls-
remote not
suffice for this? e.g.
master
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/heads/master
407fb2fe202aaeb273e4986dc88c30596a7fe8db
refs/remotes/upstream/master
Yes 'ls-remote' is actually a good option. You could have the following
FOO_VERSION = $(shell git ls-remote URL branch | awk '{ print $$1 }')
Then it would pick up any change automatically until you finish your dev
and change the FOO_VERSION to a proper ID.
Thats actually a cracking idea. I like it (and I'm not sure why I
didnt think of it when suggesting it for detecting outdated named
head versions :) )
You will lose reproducibility of course. There won't be any way to
recreate last week's nightly build since you'll get the current master
of every package when you try. I would hate to try to track down a
regression in such a setup.

You'll also get builds that change from minute to minute as new commits
appear on any package's master branch. I would think this would be
very annoying for developers. I expect that my source code is only
changed when I run git pull, not every time I run make.

I also find that integration is not so easy as this. Often commits in
one package require changes to another package. Pulling in new commits
to the product the instant they are merged in one package will often
break.

If you don't like updating package versions, here's a design that
avoids that and also doesn't suffer from the above problems.

product
-mypackage
--source code
-br-extern
--packages
---mypackage
----mypackage.mk
-----[MYPACKAGE_SITE_METHOD = local]
-----[MYPACKAGE_SITE = ../mypackage]
--configs
---myproduct_defconfig
-buildroot (submodule)
Makefile

The top level, "product", is a git repo. There is a sub-directory for
each of the product's packages that contains the source. One develops
the code by editing it in this location, and git commit/pull operations
on done on the code here as part of the product repo.

The by-extern directory is also part of the product repo. It has the
buildroot files for the packages. They are setup to use the "local"
SITE_METHOD and a relative path that points to the mypackage directory
that is part of the product repo, described previously.

buildroot itself is a git submodule of the product repo.

There is (probably) a top-level Makefile that has targets to do the
initial make command to buildroot, e.g. make -C buildroot
BR2_EXTERNAL=$(CURDIR)/br-extern O=$(CURDIR)/output/myproduct
myproduct_defconfig

To build, one git clones (recursively) the product repo. This gives
you all the product source code and the buildroot version that will
build it.

Checking out an old version gives you the old code (any maybe an older
buildroot version too).

One develops by editing the source in this repo check out. There is
nothing to setup (e.g. srcdir_override) to be in development mode. One
commits to git directly from the checkout.

Once a commit is merged to master, it's part of the product. There is
no additional step to update a git hash in a .mk file.

There is no difficulty if a single commit, or a series of commits part
of a single pull request, touch multiple packages (assume mypackage is
not the only package..). This is how integration is done. Multiple
commits to multiple packages could get merged into a topic branch, then
merged to master when the whole thing is ready.

A drawback here is that buildroot will not automatically rebuild a
package when the code is updated. It's necessary to use the target
"mypackage-rebuild" to get that to happen. A target that does this for
every package with SIT_METHOD == "local" is very handy.

I think buildroot could be made automatically detect changes by
comparing the timestamp of the .stamp_rsynced file vs the local
MYPACKAGE_SITE directory's timestamp.

Arnout Vandecappelle
2018-06-05 19:36:05 UTC
Permalink
Hi Arnout,
Thanks for the more in depth explanation.
I agree with all of the reasons outlined for the purpose of being purely a
reproducible build manager, which buildroot only ever aimed to be.
However, people do use it during development, and with a reasonably large
number of custom packages all in development at the same time, using the
_OVERRIDE_SRCDIR method to test latest versions of multiple packages
becomes very unwieldy.
My solution so far has been to point the version at the branch that active
development is taking place on, and removing the build directories and
packages for the specific projects each time.
When I am not the one doing the development on each package's source, but
am developing the rootfs, this means I dont have to manually keep updating
my local git repositories for each project.
Once each package has hit their first release version, they do get tagged
and the release branch for the project's BR2_EXTERNAL directory gets
updated with those versions, and each version thereafter, but the
development branch for the external directory persistently stays on the
development branch head for each package.
Regarding not knowing the sha before a checkout, would git ls-remote not
suffice for this? e.g.
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/heads/master
407fb2fe202aaeb273e4986dc88c30596a7fe8db refs/remotes/upstream/master
Yes 'ls-remote' is actually a good option.
It's a bit racy though. By the time you do the actual fetch, the remote branch
may already have been updated by someone else.
Someone might update the branch while you are fetching as well. As long as you
fetch a valid commit ID that is known to have been at the branch tip at some
point in the recent past, it should be fine from the user perspective.
Indeed, I was too quick, I hadn't understood that the idea was to use the
ls-remote result to set the version to check out.
baruch
You could have the following
FOO_VERSION = $(shell git ls-remote URL branch | awk '{ print $$1 }')
This might actually work pretty well... Better than my _OVERRIDE_SRCDIR
extension idea. So maybe something like

ifneq ($$($(2)_OVERRIDE_GIT_REPO),)
$(2)_VERSION = $$($(2)_OVERRIDE_GIT_REF)
$(2)_SITE = $$($(2)_OVERRIDE_GIT_REPO)
$(2)_SITE_METHOD = git
endif

and of course defaulting OVERRIDE_GIT_REF to master and defaulting
$(2)_OVERRIDE_GIT_REPO to $(3)_OVERRIDE_GIT_REPO.

Regards,
Arnout
Then it would pick up any change automatically until you finish your dev
and change the FOO_VERSION to a proper ID.
It would allow you to check for changes in sha without doing a new fetch.
Oh well, not to worry. Ill keep it as a local patch, as I suspect I am
treading old ground of "buildroot is not a development platform" (Ive seen
this discussion come up multiple times before).
Yes that will most likely not be included inside BR infrastructure but that
should be a perfectly fine option for your custom package.
Regards,
Gary
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Robert Beckett
2018-06-04 13:32:30 UTC
Permalink
'+' is a valid character in a url. The current dl-wrapper gets the
URI scheme by dropping everything after the last '+' character, with
the intension of finding 'git' from e.g. 'git+https://uri'.

If a uri has a '+' anywhere in it, it ends up using too much of the
string as a scheme, and fails to match the handler properly.

An example of where this form of URI is used is when using deploy tokens
in gitlab. It uses a form like https://<username>:<password>@gitlab.com/<group>/<repo.git>
where username for deploy token is of the form 'gitlab+deploy-token-<number>'.

Use the %% operator to search backwards until the last '+' character when
dropping the rest of the string as we know that the first '+'
in the string should be the scheme.

Signed-off-by: Robert Beckett <***@netvu.org.uk>
---
support/download/dl-wrapper | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 8d6365e..4059c37 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -88,7 +88,7 @@ main() {
download_and_check=0
rc=1
for uri in "${uris[@]}"; do
- backend=${uri%+*}
+ backend=${uri%%+*}
case "${backend}" in
git|svn|cvs|bzr|file|scp|hg) ;;
*) backend="wget" ;;
--
2.7.4
Thomas Petazzoni
2018-06-04 20:00:08 UTC
Permalink
Hello,
Post by Robert Beckett
'+' is a valid character in a url. The current dl-wrapper gets the
URI scheme by dropping everything after the last '+' character, with
the intension of finding 'git' from e.g. 'git+https://uri'.
If a uri has a '+' anywhere in it, it ends up using too much of the
string as a scheme, and fails to match the handler properly.
An example of where this form of URI is used is when using deploy tokens
where username for deploy token is of the form 'gitlab+deploy-token-<number>'.
Use the %% operator to search backwards until the last '+' character when
dropping the rest of the string as we know that the first '+'
in the string should be the scheme.
---
support/download/dl-wrapper | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Applied to master, thanks.

Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Yann E. MORIN
2018-06-04 15:56:45 UTC
Permalink
Robert, All,
Post by Robert Beckett
'+' is a valid character in a url. The current dl-wrapper gets the
URI scheme by dropping everything after the last '+' character, with
the intension of finding 'git' from e.g. 'git+https://uri'.
If a uri has a '+' anywhere in it, it ends up using too much of the
string as a scheme, and fails to match the handler properly.
An example of where this form of URI is used is when using deploy tokens
where username for deploy token is of the form 'gitlab+deploy-token-<number>'.
Use the %% operator to search backwards until the last '+' character when
dropping the rest of the string as we know that the first '+'
in the string should be the scheme.
Acked-by: "Yann E. MORIN" <***@free.fr>

Regards,
Yann E. MORIN.
Post by Robert Beckett
---
support/download/dl-wrapper | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 8d6365e..4059c37 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -88,7 +88,7 @@ main() {
download_and_check=0
rc=1
- backend=${uri%+*}
+ backend=${uri%%+*}
case "${backend}" in
git|svn|cvs|bzr|file|scp|hg) ;;
*) backend="wget" ;;
--
2.7.4
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
Loading...