All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] core/download: remove support for special git refs
@ 2016-10-26 20:08 Yann E. MORIN
  2016-10-26 22:27 ` Vivien Didelot
  2016-10-27  0:01 ` Carlos Santos
  0 siblings, 2 replies; 8+ messages in thread
From: Yann E. MORIN @ 2016-10-26 20:08 UTC (permalink / raw)
  To: buildroot

f8b8251a (support/download: fetch all refs on full git clone) added
support for fetching so-called special refs, to retrieve changesets from
code-review tools like Gerrit or Github PRs.

However, the use-case for using those special refs is not entirely
clear, and is getting more and more complex (with still-pending patches
about that).

Those special refs are from code-review tools. As such, they are very
volatile in nature: re-pushing a rebased branch to github would change
the content fetched with such a ref (Gerrit seems to be more
conservative, but still). So, they cannot be used for reproducible
builds:
  - they can't be used in a .mk file (normal packages)
  - they can't be used in a .config file (linux, bootloader...)

So, going back to the black board, two main use-cases have been
identified:
  - a developper that wants to manually test a PR
  - an automated build farm that automatically builds a known
    configuration against PRs

In either case, there is a better solution that we've been advertising
all along: the override-srcdir mechanism.

Whether by a build farm or a developper, it looks like it would be much
easier to do the clone (or a fetch from an existing repo), write a
local.mk with an override-srcdir, and kick off the build, rather than do
the various tweaking (in .mk and/or .config).

(note: a developper that wants to test a PR is probably already active
on that project, so will already have a local clone, so the fetch would
only grab very few blobs, wo will be very fast).

So, just remove the support for such special refs altogether.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
Cc: Henrique Marks <henrique.marks@datacom.ind.br>
---
 support/download/git | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/support/download/git b/support/download/git
index 7921411..c46422d 100755
--- a/support/download/git
+++ b/support/download/git
@@ -61,17 +61,6 @@ fi
 
 pushd "${basename}" >/dev/null
 
-# Try to get the special refs exposed by some forges (pull-requests for
-# github, changes for gerrit...). There is no easy way to know whether
-# the cset the user passed us is such a special ref or a tag or a sha1
-# or whatever else. We'll eventually fail at checking out that cset,
-# 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
-    printf "Could not fetch special ref '%s'; assuming it is not special.\n" "${cset}"
-fi
-
 # Checkout the required changeset, so that we can update the required
 # submodules.
 _git checkout -q "'${cset}'"
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-26 20:08 [Buildroot] [PATCH] core/download: remove support for special git refs Yann E. MORIN
@ 2016-10-26 22:27 ` Vivien Didelot
  2016-10-27  7:10   ` Thomas Petazzoni
  2016-10-27  0:01 ` Carlos Santos
  1 sibling, 1 reply; 8+ messages in thread
From: Vivien Didelot @ 2016-10-26 22:27 UTC (permalink / raw)
  To: buildroot

Hi Yann,

"Yann E. MORIN" <yann.morin.1998@free.fr> writes:

> f8b8251a (support/download: fetch all refs on full git clone) added
> support for fetching so-called special refs, to retrieve changesets from
> code-review tools like Gerrit or Github PRs.
>
> However, the use-case for using those special refs is not entirely
> clear, and is getting more and more complex (with still-pending patches
> about that).
>
> Those special refs are from code-review tools. As such, they are very
> volatile in nature: re-pushing a rebased branch to github would change
> the content fetched with such a ref (Gerrit seems to be more
> conservative, but still). So, they cannot be used for reproducible
> builds:
>   - they can't be used in a .mk file (normal packages)
>   - they can't be used in a .config file (linux, bootloader...)
>
> So, going back to the black board, two main use-cases have been
> identified:
>   - a developper that wants to manually test a PR
>   - an automated build farm that automatically builds a known
>     configuration against PRs
>
> In either case, there is a better solution that we've been advertising
> all along: the override-srcdir mechanism.
>
> Whether by a build farm or a developper, it looks like it would be much
> easier to do the clone (or a fetch from an existing repo), write a
> local.mk with an override-srcdir, and kick off the build, rather than do
> the various tweaking (in .mk and/or .config).
>
> (note: a developper that wants to test a PR is probably already active
> on that project, so will already have a local clone, so the fetch would
> only grab very few blobs, wo will be very fast).
>
> So, just remove the support for such special refs altogether.

I don't see why it is hard to maintain. A Github pull request ref or a
Gerrit change ref, as well as a custom ref, are valid git references.

That can totally end up in a customer defconfig.

What you are raising here is just a design choice. Whether to guide
developers to mechanism such as override-srcdir because such "special"
refs are to be considered not production-ready, or to be very flexible.

Thus I have no opinion on that. It's up to you guys.

Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-26 20:08 [Buildroot] [PATCH] core/download: remove support for special git refs Yann E. MORIN
  2016-10-26 22:27 ` Vivien Didelot
@ 2016-10-27  0:01 ` Carlos Santos
  2016-11-02 19:48   ` Arnout Vandecappelle
  1 sibling, 1 reply; 8+ messages in thread
From: Carlos Santos @ 2016-10-27  0:01 UTC (permalink / raw)
  To: buildroot

> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> To: buildroot at buildroot.org
> Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Ricardo Martincoski"
> <ricardo.martincoski@datacom.ind.br>, "Yann E. MORIN" <yann.morin.1998@free.fr>
> Sent: Wednesday, October 26, 2016 6:08:09 PM
> Subject: [Buildroot] [PATCH] core/download: remove support for special git	refs

> f8b8251a (support/download: fetch all refs on full git clone) added
> support for fetching so-called special refs, to retrieve changesets from
> code-review tools like Gerrit or Github PRs.
> 
> However, the use-case for using those special refs is not entirely
> clear, and is getting more and more complex (with still-pending patches
> about that).

The use case is pretty clear, and you told it: retrieve changesets from
code-review tools like Gerrit or Github PRs. We (DATACOM) have been
doing this for months and I can assure you that it improved our development
process *a lot*. The fact that there are still pending patches only means
that the feature needs improvements and that different people have been
trying to make it better.

> Those special refs are from code-review tools. As such, they are very
> volatile in nature: re-pushing a rebased branch to github would change
> the content fetched with such a ref (Gerrit seems to be more
> conservative, but still). So, they cannot be used for reproducible
> builds:
>  - they can't be used in a .mk file (normal packages)

Yes, they do. They may not be so useful for buildroot official packages
but for packages maintained in-house and/or by distributed development
teams such feature is extremely handy.

>  - they can't be used in a .config file (linux, bootloader...)

Yes they do. I'm currently working on a change that upgrades both linux
and u-boot for a half-dozen boards.

> So, going back to the black board, two main use-cases have been
> identified:
>  - a developper that wants to manually test a PR

A developer who needs to share work in progress with colleagues so they
can test it. One of my colleagues is currently working on a change that
affects around twenty modules used on four different products. He shares
his work by mens of a change in our br2_external repository in which the
package versions in the .mk files are replaced by the current SHA-1 in
the corresponding changes in the modules.

>  - an automated build farm that automatically builds a known
>    configuration against PRs

This is a particular case of the previous one, with the restriction that
it is necessary to automate the retrieval of WIP code my means of some
custom <FOO>_VERSION variables.

> In either case, there is a better solution that we've been advertising
> all along: the override-srcdir mechanism.

The override-srcdir mechanism suffices when you work alone on a few
packages. It becomes much harder when you work on many modules and need
to share intermediary results with other developers.

> Whether by a build farm or a developper, it looks like it would be much
> easier to do the clone (or a fetch from an existing repo), write a
> local.mk with an override-srcdir, and kick off the build, rather than do
> the various tweaking (in .mk and/or .config).

We tried this approach and it became a nightmare. Sooner or later some
developer forgets to update the local copy of one of the modules and
his/her build breaks. Using a build farm for WIP was impossible because
required tricks to fetch custom versions of the source code of some packages.

> (note: a developper that wants to test a PR is probably already active
> on that project, so will already have a local clone, so the fetch would
> only grab very few blobs, wo will be very fast).

This might work for lone developers or small teams, not for dozens of
programmers coding fiercely for a multi-site organization, using a DVCS
like Gerrit. Read this, please:

http://lists.busybox.net/pipermail/buildroot/2015-October/143248.html

> So, just remove the support for such special refs altogether.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Cc: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
> Cc: Henrique Marks <henrique.marks@datacom.ind.br>
> ---
> support/download/git | 11 -----------
> 1 file changed, 11 deletions(-)
> 
> diff --git a/support/download/git b/support/download/git
> index 7921411..c46422d 100755
> --- a/support/download/git
> +++ b/support/download/git
> @@ -61,17 +61,6 @@ fi
> 
> pushd "${basename}" >/dev/null
> 
> -# Try to get the special refs exposed by some forges (pull-requests for
> -# github, changes for gerrit...). There is no easy way to know whether
> -# the cset the user passed us is such a special ref or a tag or a sha1
> -# or whatever else. We'll eventually fail at checking out that cset,
> -# 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
> -    printf "Could not fetch special ref '%s'; assuming it is not special.\n"
> "${cset}"
> -fi
> -
> # Checkout the required changeset, so that we can update the required
> # submodules.
> _git checkout -q "'${cset}'"
> --
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



Carlos Santos (Casantos)
DATACOM, P&D

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-26 22:27 ` Vivien Didelot
@ 2016-10-27  7:10   ` Thomas Petazzoni
  2016-10-27 10:43     ` Carlos Santos
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Petazzoni @ 2016-10-27  7:10 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 26 Oct 2016 18:27:57 -0400, Vivien Didelot wrote:

> > So, just remove the support for such special refs altogether.  
> 
> I don't see why it is hard to maintain. A Github pull request ref or a
> Gerrit change ref, as well as a custom ref, are valid git references.

They are valid, but moving references, right? A little bit like a
branch.

And we don't support using Git branches as the Git version. Indeed, if
you do that, Buildroot will clone the Git repo once, tarball it in
$(DL_DIR), and will never download it again, because the name of the
branch doesn't change, so the name of the tarball doesn't change.

And we don't even try to solve this problem, because using a branch
name as the <pkg>_VERSION is just plain wrong.

> That can totally end up in a customer defconfig.

And this is bogus, and precisely why we don't want to support such
thing. Github pull request and Gerrit change references are moving, so
a given Github pull request can one day contain a given version of the
code, and the next day a different version of the code. Hence you're
giving your customers something that is not reproducible. Not good.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-27  7:10   ` Thomas Petazzoni
@ 2016-10-27 10:43     ` Carlos Santos
  2016-10-27 23:11       ` Arnout Vandecappelle
  0 siblings, 1 reply; 8+ messages in thread
From: Carlos Santos @ 2016-10-27 10:43 UTC (permalink / raw)
  To: buildroot

> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Vivien Didelot" <vivien.didelot@savoirfairelinux.com>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>, "Ricardo Martincoski" <ricardo.martincoski@datacom.ind.br>,
> buildroot at buildroot.org
> Sent: Thursday, October 27, 2016 5:10:27 AM
> Subject: Re: [Buildroot] [PATCH] core/download: remove support for special	git refs

> Hello,
> 
> On Wed, 26 Oct 2016 18:27:57 -0400, Vivien Didelot wrote:
> 
>> > So, just remove the support for such special refs altogether.
>> 
>> I don't see why it is hard to maintain. A Github pull request ref or a
>> Gerrit change ref, as well as a custom ref, are valid git references.
> 
> They are valid, but moving references, right? A little bit like a
> branch.
> 
> And we don't support using Git branches as the Git version. Indeed, if
> you do that, Buildroot will clone the Git repo once, tarball it in
> $(DL_DIR), and will never download it again, because the name of the
> branch doesn't change, so the name of the tarball doesn't change.
> 
> And we don't even try to solve this problem, because using a branch
> name as the <pkg>_VERSION is just plain wrong.

As far as I know nobody is asking Buildroot to support branch names in
<pkg>_VERSION.

>> That can totally end up in a customer defconfig.
> 
> And this is bogus, and precisely why we don't want to support such
> thing. Github pull request and Gerrit change references are moving, so
> a given Github pull request can one day contain a given version of the
> code, and the next day a different version of the code. Hence you're
> giving your customers something that is not reproducible. Not good.

I'm not sure about Github pull requests but Gerrit change-ids could not
be used in <pkg>_VERSION because they are not valid Git commit-ids.

Each patchset in a Gerrit change has a unique commmit-id, which is not
a moving reference. It be referred to ether by a SHA-1 or by an explicit
path like "refs/changes/70/24070/1". The later is ugly but still unique
and will be saved be saved as 

   $(DL_DIR)/<pkg>-refs_changes_70_24070_1.tar.gz

Carlos Santos (Casantos)
DATACOM, P&D

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-27 10:43     ` Carlos Santos
@ 2016-10-27 23:11       ` Arnout Vandecappelle
  0 siblings, 0 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2016-10-27 23:11 UTC (permalink / raw)
  To: buildroot



On 27-10-16 12:43, Carlos Santos wrote:
>> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
[snip]
>> And this is bogus, and precisely why we don't want to support such
>> thing. Github pull request and Gerrit change references are moving, so
>> a given Github pull request can one day contain a given version of the
>> code, and the next day a different version of the code. Hence you're
>> giving your customers something that is not reproducible. Not good.
> 
> I'm not sure about Github pull requests but Gerrit change-ids could not
> be used in <pkg>_VERSION because they are not valid Git commit-ids.
> 
> Each patchset in a Gerrit change has a unique commmit-id, which is not
> a moving reference. It be referred to ether by a SHA-1 or by an explicit
> path like "refs/changes/70/24070/1". The later is ugly but still unique
> and will be saved be saved as 
> 
>    $(DL_DIR)/<pkg>-refs_changes_70_24070_1.tar.gz

 But this doesn't require any special handling in the git download helper, it's
a ref like any other. The special handling is for sha1's that refer to something
that is not in a published branch or tag.

 Actually, ideally we would want to just handle sha1 and nothing else. But git
doesn't make that easy, unfortunately.

 Regards,
 Arnout

-- 
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-10-27  0:01 ` Carlos Santos
@ 2016-11-02 19:48   ` Arnout Vandecappelle
  2016-11-03  1:01     ` Henrique Marks
  0 siblings, 1 reply; 8+ messages in thread
From: Arnout Vandecappelle @ 2016-11-02 19:48 UTC (permalink / raw)
  To: buildroot



On 27-10-16 02:01, Carlos Santos wrote:
>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> To: buildroot at buildroot.org
>> Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Ricardo Martincoski"
>> <ricardo.martincoski@datacom.ind.br>, "Yann E. MORIN" <yann.morin.1998@free.fr>
>> Sent: Wednesday, October 26, 2016 6:08:09 PM
>> Subject: [Buildroot] [PATCH] core/download: remove support for special git	refs
> 
>> f8b8251a (support/download: fetch all refs on full git clone) added
>> support for fetching so-called special refs, to retrieve changesets from
>> code-review tools like Gerrit or Github PRs.
>>
>> However, the use-case for using those special refs is not entirely
>> clear, and is getting more and more complex (with still-pending patches
>> about that).
> 
> The use case is pretty clear, and you told it: retrieve changesets from
> code-review tools like Gerrit or Github PRs. 

 But you have to admit that such a use case seems weird and complicated. So you
push a change in a package to Gerrit, then you modify your br2_external to refer
to that change, commit it and I suppose also push it to Gerrit, and finally you
go and tell CI to fetch that stuff.

 To me, it would make a lot more sense if your br2_external packages are kept
together in a repository using git submodules, so you can simply do a
commit/review of the integration repository whenever you updated the
subrepositories.

 In fact, I like to keep the buildroot infra inside the package repos
themselves, so the integration repository doubles as a br2_external repository.


> We (DATACOM) have been
> doing this for months and I can assure you that it improved our development
> process *a lot*. The fact that there are still pending patches only means
> that the feature needs improvements and that different people have been
> trying to make it better.

 Well, the problem is that the git download helper is becoming so complicated
that we don't understand it anymore, and that any change we make might break one
of the use cases we purportedly support. So the idea is to actively *not*
support some use cases, which allows us to break them.

 Actually, I would say that what we need to support is: any ref that works with
'git fetch <site> <ref>'.


>> Those special refs are from code-review tools. As such, they are very
>> volatile in nature: re-pushing a rebased branch to github would change
>> the content fetched with such a ref (Gerrit seems to be more
>> conservative, but still). So, they cannot be used for reproducible
>> builds:
>>  - they can't be used in a .mk file (normal packages)
> 
> Yes, they do. They may not be so useful for buildroot official packages
> but for packages maintained in-house and/or by distributed development
> teams such feature is extremely handy.

 Even for in-house development unstables refs are not a good idea. But as
already mentioned several times, gerrit's refs/changes/xx/xxx/N refs are pretty
stable.

> 
>>  - they can't be used in a .config file (linux, bootloader...)
> 
> Yes they do. I'm currently working on a change that upgrades both linux
> and u-boot for a half-dozen boards.
> 
>> So, going back to the black board, two main use-cases have been
>> identified:
>>  - a developper that wants to manually test a PR
> 
> A developer who needs to share work in progress with colleagues so they
> can test it. One of my colleagues is currently working on a change that
> affects around twenty modules used on four different products. He shares
> his work by mens of a change in our br2_external repository in which the
> package versions in the .mk files are replaced by the current SHA-1 in
> the corresponding changes in the modules.

 Which IMHO would be dramatically less work with submodules (simply 'git commit'
rather than first updating 20 files and then doing 'git commit').

 But here really we come to the crux of the problem: you want to use sha1 refs
for things which are not in a normal 'clone'. In other words, you want to do
something which would be really hard to do if you did it manually. No wonder a
lot of special magic is needed in the git download helper to make that work.

> 
>>  - an automated build farm that automatically builds a known
>>    configuration against PRs
> 
> This is a particular case of the previous one, with the restriction that
> it is necessary to automate the retrieval of WIP code my means of some
> custom <FOO>_VERSION variables.
> 
>> In either case, there is a better solution that we've been advertising
>> all along: the override-srcdir mechanism.
> 
> The override-srcdir mechanism suffices when you work alone on a few
> packages. It becomes much harder when you work on many modules and need
> to share intermediary results with other developers.
> 
>> Whether by a build farm or a developper, it looks like it would be much
>> easier to do the clone (or a fetch from an existing repo), write a
>> local.mk with an override-srcdir, and kick off the build, rather than do
>> the various tweaking (in .mk and/or .config).
> 
> We tried this approach and it became a nightmare. Sooner or later some
> developer forgets to update the local copy of one of the modules and
> his/her build breaks. Using a build farm for WIP was impossible because
> required tricks to fetch custom versions of the source code of some packages.
> 
>> (note: a developper that wants to test a PR is probably already active
>> on that project, so will already have a local clone, so the fetch would
>> only grab very few blobs, wo will be very fast).
> 
> This might work for lone developers or small teams, not for dozens of
> programmers coding fiercely for a multi-site organization, using a DVCS
> like Gerrit. Read this, please:
> 
> http://lists.busybox.net/pipermail/buildroot/2015-October/143248.html

 Well, that mail doesn't mention the need for handling special refs, and it
actually predates the patch that made special refs work, so I guess that it
wasn't needed back then?



 Regards,
 Arnout

> 
>> So, just remove the support for such special refs altogether.
>>
>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> Cc: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
>> Cc: Henrique Marks <henrique.marks@datacom.ind.br>
>> ---
>> support/download/git | 11 -----------
>> 1 file changed, 11 deletions(-)
>>
>> diff --git a/support/download/git b/support/download/git
>> index 7921411..c46422d 100755
>> --- a/support/download/git
>> +++ b/support/download/git
>> @@ -61,17 +61,6 @@ fi
>>
>> pushd "${basename}" >/dev/null
>>
>> -# Try to get the special refs exposed by some forges (pull-requests for
>> -# github, changes for gerrit...). There is no easy way to know whether
>> -# the cset the user passed us is such a special ref or a tag or a sha1
>> -# or whatever else. We'll eventually fail at checking out that cset,
>> -# 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
>> -    printf "Could not fetch special ref '%s'; assuming it is not special.\n"
>> "${cset}"
>> -fi
>> -
>> # Checkout the required changeset, so that we can update the required
>> # submodules.
>> _git checkout -q "'${cset}'"
>> --
>> 2.7.4
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 
> 
> 
> Carlos Santos (Casantos)
> DATACOM, P&D
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH] core/download: remove support for special git refs
  2016-11-02 19:48   ` Arnout Vandecappelle
@ 2016-11-03  1:01     ` Henrique Marks
  0 siblings, 0 replies; 8+ messages in thread
From: Henrique Marks @ 2016-11-03  1:01 UTC (permalink / raw)
  To: buildroot

Hello All

----- Mensagem original -----
> De: "Arnout Vandecappelle" <arnout@mind.be>
> Para: "Carlos Santos" <casantos@datacom.ind.br>, "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "ricardo martincoski"
> <ricardo.martincoski@datacom.ind.br>, buildroot at buildroot.org
> Enviadas: Quarta-feira, 2 de novembro de 2016 17:48:59
> Assunto: Re: [Buildroot] [PATCH] core/download: remove support for special git refs

> On 27-10-16 02:01, Carlos Santos wrote:
>>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> To: buildroot at buildroot.org
>>> Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Ricardo
>>> Martincoski"
>>> <ricardo.martincoski@datacom.ind.br>, "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> Sent: Wednesday, October 26, 2016 6:08:09 PM
>>> Subject: [Buildroot] [PATCH] core/download: remove support for special git	refs
>> 
>>> f8b8251a (support/download: fetch all refs on full git clone) added
>>> support for fetching so-called special refs, to retrieve changesets from
>>> code-review tools like Gerrit or Github PRs.
>>>
>>> However, the use-case for using those special refs is not entirely
>>> clear, and is getting more and more complex (with still-pending patches
>>> about that).
>> 
>> The use case is pretty clear, and you told it: retrieve changesets from
>> code-review tools like Gerrit or Github PRs.
> 
> But you have to admit that such a use case seems weird and complicated. So you
> push a change in a package to Gerrit, then you modify your br2_external to refer
> to that change, commit it and I suppose also push it to Gerrit, and finally you
> go and tell CI to fetch that stuff.
> 
> To me, it would make a lot more sense if your br2_external packages are kept
> together in a repository using git submodules, so you can simply do a
> commit/review of the integration repository whenever you updated the
> subrepositories.
> 
> In fact, I like to keep the buildroot infra inside the package repos
> themselves, so the integration repository doubles as a br2_external repository.
> 
> 
>> We (DATACOM) have been
>> doing this for months and I can assure you that it improved our development
>> process *a lot*. The fact that there are still pending patches only means
>> that the feature needs improvements and that different people have been
>> trying to make it better.
> 
> Well, the problem is that the git download helper is becoming so complicated
> that we don't understand it anymore, and that any change we make might break one
> of the use cases we purportedly support. So the idea is to actively *not*
> support some use cases, which allows us to break them.
> 
> Actually, I would say that what we need to support is: any ref that works with
> 'git fetch <site> <ref>'.
> 
> 
>>> Those special refs are from code-review tools. As such, they are very
>>> volatile in nature: re-pushing a rebased branch to github would change
>>> the content fetched with such a ref (Gerrit seems to be more
>>> conservative, but still). So, they cannot be used for reproducible
>>> builds:
>>>  - they can't be used in a .mk file (normal packages)
>> 
>> Yes, they do. They may not be so useful for buildroot official packages
>> but for packages maintained in-house and/or by distributed development
>> teams such feature is extremely handy.
> 
> Even for in-house development unstables refs are not a good idea. But as
> already mentioned several times, gerrit's refs/changes/xx/xxx/N refs are pretty
> stable.
> 
>> 
>>>  - they can't be used in a .config file (linux, bootloader...)
>> 
>> Yes they do. I'm currently working on a change that upgrades both linux
>> and u-boot for a half-dozen boards.
>> 
>>> So, going back to the black board, two main use-cases have been
>>> identified:
>>>  - a developper that wants to manually test a PR
>> 
>> A developer who needs to share work in progress with colleagues so they
>> can test it. One of my colleagues is currently working on a change that
>> affects around twenty modules used on four different products. He shares
>> his work by mens of a change in our br2_external repository in which the
>> package versions in the .mk files are replaced by the current SHA-1 in
>> the corresponding changes in the modules.
> 
> Which IMHO would be dramatically less work with submodules (simply 'git commit'
> rather than first updating 20 files and then doing 'git commit').
> 
> But here really we come to the crux of the problem: you want to use sha1 refs
> for things which are not in a normal 'clone'. In other words, you want to do
> something which would be really hard to do if you did it manually. No wonder a
> lot of special magic is needed in the git download helper to make that work.
> 
>> 
>>>  - an automated build farm that automatically builds a known
>>>    configuration against PRs
>> 
>> This is a particular case of the previous one, with the restriction that
>> it is necessary to automate the retrieval of WIP code my means of some
>> custom <FOO>_VERSION variables.
>> 
>>> In either case, there is a better solution that we've been advertising
>>> all along: the override-srcdir mechanism.
>> 
>> The override-srcdir mechanism suffices when you work alone on a few
>> packages. It becomes much harder when you work on many modules and need
>> to share intermediary results with other developers.
>> 
>>> Whether by a build farm or a developper, it looks like it would be much
>>> easier to do the clone (or a fetch from an existing repo), write a
>>> local.mk with an override-srcdir, and kick off the build, rather than do
>>> the various tweaking (in .mk and/or .config).
>> 
>> We tried this approach and it became a nightmare. Sooner or later some
>> developer forgets to update the local copy of one of the modules and
>> his/her build breaks. Using a build farm for WIP was impossible because
>> required tricks to fetch custom versions of the source code of some packages.
>> 
>>> (note: a developper that wants to test a PR is probably already active
>>> on that project, so will already have a local clone, so the fetch would
>>> only grab very few blobs, wo will be very fast).
>> 
>> This might work for lone developers or small teams, not for dozens of
>> programmers coding fiercely for a multi-site organization, using a DVCS
>> like Gerrit. Read this, please:
>> 
>> http://lists.busybox.net/pipermail/buildroot/2015-October/143248.html
> 
> Well, that mail doesn't mention the need for handling special refs, and it
> actually predates the patch that made special refs work, so I guess that it
> wasn't needed back then?
> 
> 
> 
> Regards,
> Arnout
> 
>> 
>>> So, just remove the support for such special refs altogether.
>>>
>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>> Cc: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
>>> Cc: Henrique Marks <henrique.marks@datacom.ind.br>
>>> ---
>>> support/download/git | 11 -----------
>>> 1 file changed, 11 deletions(-)
>>>
>>> diff --git a/support/download/git b/support/download/git
>>> index 7921411..c46422d 100755
>>> --- a/support/download/git
>>> +++ b/support/download/git
>>> @@ -61,17 +61,6 @@ fi
>>>
>>> pushd "${basename}" >/dev/null
>>>
>>> -# Try to get the special refs exposed by some forges (pull-requests for
>>> -# github, changes for gerrit...). There is no easy way to know whether
>>> -# the cset the user passed us is such a special ref or a tag or a sha1
>>> -# or whatever else. We'll eventually fail at checking out that cset,
>>> -# 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
>>> -    printf "Could not fetch special ref '%s'; assuming it is not special.\n"
>>> "${cset}"
>>> -fi
>>> -
>>> # Checkout the required changeset, so that we can update the required
>>> # submodules.
>>> _git checkout -q "'${cset}'"
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot at busybox.net
>>> http://lists.busybox.net/mailman/listinfo/buildroot
>> 
>> 
>> 
>> Carlos Santos (Casantos)
>> DATACOM, P&D
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>> 
> 
> --
> 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
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

It will be a little hard to discuss our use case  in the buildroot mailing list. We can include Yan, Arnout, Thomas, and some others in an email showing the complete use case, but it is a LOT simpler than you stated. And it is all automated: developer has new code -> run unit tests-> run integration tests for multiple platform -> integrate the new SHA1 (both in module and br2_external).

But if some developers have new code ? Well, each one runs unit tests and then, to run integration tests, we need to download special refs (they are not integrated YET).

But, in another thread, Ricardo sent a complete Patch-Set that changes the core git infrastructure to use fetches and checkouts, instead of clones. Ricardo also sent a set of (python) tests, that can be used to check the infrastructure itself.

As Ricardo stated in another thread:

Please take a look at this patch series:
http://patchwork.ozlabs.org/patch/690099/
http://patchwork.ozlabs.org/patch/690097/ (please read the WARNING)
http://patchwork.ozlabs.org/patch/690098/

The idea of this patch series were more or less presented in another email thread involving Yan, Arnout, Brendan and Ricardo.

Thanks

-- 
Dr. Henrique Marks
henrique.marks at datacom.ind.br
R. Am?rica, 1000 - Eldorado do Sul - RS
CEP: 92990-000 - Brasil
Fone: +55 51 3933 3000 - Ramal 3466

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-03  1:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26 20:08 [Buildroot] [PATCH] core/download: remove support for special git refs Yann E. MORIN
2016-10-26 22:27 ` Vivien Didelot
2016-10-27  7:10   ` Thomas Petazzoni
2016-10-27 10:43     ` Carlos Santos
2016-10-27 23:11       ` Arnout Vandecappelle
2016-10-27  0:01 ` Carlos Santos
2016-11-02 19:48   ` Arnout Vandecappelle
2016-11-03  1:01     ` Henrique Marks

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.