[pacman-dev] [PATCH] makepkg: Use a recursive, shallow clone for git://
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 <sir@cmpwn.com> --- 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
This is worth noting https://bugs.archlinux.org/task/23065 On Tue, Oct 28, 2014 at 7:13 PM, Drew DeVault <sircmpwn@gmail.com> 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 <sir@cmpwn.com> --- 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
On 10/28/2014 06:19 PM, Daniel Wallace wrote:
This is worth noting
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.
On Tue, Oct 28, 2014 at 7:19 PM, Drew DeVault <sircmpwn@gmail.com> wrote:
On 10/28/2014 06:19 PM, Daniel Wallace wrote:
This is worth noting
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
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
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
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.
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
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?
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
On Tue, 28 Oct 2014 18:13:12 -0600 Drew DeVault <sircmpwn@gmail.com> 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.
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.
participants (6)
-
Andrew Gregory
-
Dan McGee
-
Daniel Wallace
-
Dave Reisner
-
Doug Newgard
-
Drew DeVault