[pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

older
[pacman-dev] [PATCH] Added option...

Drew DeVault

29 Oct 2014 29 Oct '14

12:13 a.m.

makepkg: Use a recursive, shallow clone for git:// This clones git submodules, which are required to build most projects that use them. This change is harmless for packages that don't use them. This also modifies the initial clone to use --depth=1, which avoids checking out the entire history and saves some bandwidth and improves the time required to clone. Signed-off-by: Drew DeVault --- I'm still not used to submitting patches through a mailing list, so I apologize if I've done something wrong. Please don't hesitate to let me know if there's something I could do better. scripts/makepkg.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8a67d94..3b74d4d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -542,7 +542,7 @@ download_git() { if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" - if ! git clone --mirror "$url" "$dir"; then + if ! git clone --depth=1 --mirror "$url" "$dir"; then error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" plain "$(gettext "Aborting...")" exit 1 @@ -612,6 +612,11 @@ extract_git() { plain "$(gettext "Aborting...")" exit 1 fi + if ! git submodule update --init --recursive ; then + error "$(gettext "Failure while initializing submodules of %s %s repo")" "${repo}" "git" + plain "$(gettext "Aborting...")" + exit 1 + fi fi popd &>/dev/null -- 2.1.2

Show replies by date

Daniel Wallace

29 Oct 29 Oct

12:19 a.m.

This is worth noting https://bugs.archlinux.org/task/23065 On Tue, Oct 28, 2014 at 7:13 PM, Drew DeVault wrote:

...

makepkg: Use a recursive, shallow clone for git://

This clones git submodules, which are required to build most projects that use them. This change is harmless for packages that don't use them.

This also modifies the initial clone to use --depth=1, which avoids checking out the entire history and saves some bandwidth and improves the time required to clone.

Signed-off-by: Drew DeVault --- I'm still not used to submitting patches through a mailing list, so I apologize if I've done something wrong. Please don't hesitate to let me know if there's something I could do better.

scripts/makepkg.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8a67d94..3b74d4d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -542,7 +542,7 @@ download_git() {

if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" - if ! git clone --mirror "$url" "$dir"; then + if ! git clone --depth=1 --mirror "$url" "$dir"; then error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" plain "$(gettext "Aborting...")" exit 1 @@ -612,6 +612,11 @@ extract_git() { plain "$(gettext "Aborting...")" exit 1 fi + if ! git submodule update --init --recursive ; then + error "$(gettext "Failure while initializing submodules of %s %s repo")" "${repo}" "git" + plain "$(gettext "Aborting...")" + exit 1 + fi fi

popd &>/dev/null -- 2.1.2

Drew DeVault

12:19 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On 10/28/2014 06:19 PM, Daniel Wallace wrote:

...

This is worth noting

https://bugs.archlinux.org/task/23065

I hadn't seen that bug yet, but Greg seems misguided. I wouldn't send this patch along without testing it - the local clone works fine with a shallow source.

Dan McGee

12:25 a.m.

On Tue, Oct 28, 2014 at 7:19 PM, Drew DeVault wrote:

...

On 10/28/2014 06:19 PM, Daniel Wallace wrote:

...
This is worth noting

https://bugs.archlinux.org/task/23065

I hadn't seen that bug yet, but Greg seems misguided. I wouldn't send this patch along without testing it - the local clone works fine with a shallow source.

https://www.google.com/search?q=site%3Abugs.archlinux.org+git+clone+shallow This change isn't going to happen, sorry. Plenty of reason and rationale at about 5 of those bug reports. -Dan

Dave Reisner

12:26 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On Tue, Oct 28, 2014 at 06:19:30PM -0600, Drew DeVault wrote:

...

On 10/28/2014 06:19 PM, Daniel Wallace wrote:

...
This is worth noting

https://bugs.archlinux.org/task/23065

I hadn't seen that bug yet, but Greg seems misguided. I wouldn't send this patch along without testing it - the local clone works fine with a shallow source.

This is only true of recent versions of git. He was correct at the time it was posted. Does this work with URLs like: git://example.com/repo.git?tag=v1.2.3 d

Drew DeVault

12:32 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On 10/28/2014 06:26 PM, Dave Reisner wrote:

...

This is only true of recent versions of git. He was correct at the time it was posted.

Right, looking at the date on the bug that should be obvious. My bad.

...

Does this work with URLs like:

git://example.com/repo.git?tag=v1.2.3

Those don't work with makepkg as it stands now. It doesn't work after this patch is applied, either. If we wanted to make that work, we could do something more sophisticated to take advantage of the --branch option in git.

Dave Reisner

12:55 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On Tue, Oct 28, 2014 at 06:32:38PM -0600, Drew DeVault wrote:

...

On 10/28/2014 06:26 PM, Dave Reisner wrote:

...
This is only true of recent versions of git. He was correct at the time it was posted.

Right, looking at the date on the bug that should be obvious. My bad.

...
Does this work with URLs like:

git://example.com/repo.git?tag=v1.2.3

Those don't work with makepkg as it stands now. It doesn't work after this patch is applied, either. If we wanted to make that work, we could do something more sophisticated to take advantage of the --branch option in git.

Not following -- this is even documented to work in PKGBUILD(5). Could you provide an example where this fails using makepkg from git? d

Drew DeVault

1:02 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On 10/28/2014 06:55 PM, Dave Reisner wrote:

...

...
...
Does this work with URLs like:

git://example.com/repo.git?tag=v1.2.3

Those don't work with makepkg as it stands now. It doesn't work after this patch is applied, either. If we wanted to make that work, we could do something more sophisticated to take advantage of the --branch option in git.

Not following -- this is even documented to work in PKGBUILD(5). Could you provide an example where this fails using makepkg from git?

Ah, I went and read PKGBUILD(5) and the syntax is `#tag=...`, rather than `?tag=...`. This patch does indeed break that feature. The problem could be avoided by doing the initial clone with --branch, but that fundamentally changes the relationship of the initial clone and the working directory. Currently, the initial clone creates a mirror which the working directory draws from. If we made the mirror use --branch instead, then the working directory would be less of a clone and more of a copy. I don't know the intentions behind this design originally, would that be a bad idea? My gut says to just drop --depth from this patch and change it to only deal with submodules. The goal of this was to deal with submodules, the shallow change was just a nice-to-have. I could be convinced otherwise, though. Thoughts?

Andrew Gregory

2:35 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On 10/28/14 at 07:02pm, Drew DeVault wrote:

...

On 10/28/2014 06:55 PM, Dave Reisner wrote:

...
...
...
Does this work with URLs like:

git://example.com/repo.git?tag=v1.2.3

Those don't work with makepkg as it stands now. It doesn't work after this patch is applied, either. If we wanted to make that work, we could do something more sophisticated to take advantage of the --branch option in git.

Not following -- this is even documented to work in PKGBUILD(5). Could you provide an example where this fails using makepkg from git?

Ah, I went and read PKGBUILD(5) and the syntax is `#tag=...`, rather than `?tag=...`. This patch does indeed break that feature. The problem could be avoided by doing the initial clone with --branch, but that fundamentally changes the relationship of the initial clone and the working directory. Currently, the initial clone creates a mirror which the working directory draws from. If we made the mirror use --branch instead, then the working directory would be less of a clone and more of a copy. I don't know the intentions behind this design originally, would that be a bad idea?

My gut says to just drop --depth from this patch and change it to only deal with submodules. The goal of this was to deal with submodules, the shallow change was just a nice-to-have. I could be convinced otherwise, though. Thoughts?

Checking out submodules was previously discussed here: https://lists.archlinux.org/pipermail/pacman-dev/2013-March/016781.html apg

Doug Newgard

12:23 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On Tue, 28 Oct 2014 18:13:12 -0600 Drew DeVault wrote:

...

makepkg: Use a recursive, shallow clone for git://

This clones git submodules, which are required to build most projects that use them. This change is harmless for packages that don't use them.

This also modifies the initial clone to use --depth=1, which avoids checking out the entire history and saves some bandwidth and improves the time required to clone.

If you had done any research at all, you'd know that this has been rejected over and over.

Drew DeVault

12:23 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://

On 10/28/2014 06:23 PM, Doug Newgard wrote:

...

If you had done any research at all, you'd know that this has been rejected over and over.

And apparently it's not up for discussion again. Sorry I bothered.

3476

Age (days ago)

3476

Last active (days ago)

List overview

Download

10 comments

6 participants

participants (6)

Andrew Gregory
Dan McGee
Daniel Wallace
Dave Reisner
Doug Newgard
Drew DeVault