Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tc-build: Improvements around '--shallow-clone' #83

Merged

Conversation

nathanchance
Copy link
Member

Addresses #81 (cc @dileks) and some other things that I have thought of while investigating it.

I've written fairly detailed commit messages for each individual change. Some hammering on this would be appreciated as I think I have covered every corner case I can think of but I might have missed something.

@kdrag0n, please make sure this won't break your automated workflow.

As it stands now, '--shallow-clone' does not allow switching to a branch
other than master. This is because '--depth=1' implies '--single-branch'
so we only fetch the default (master).

To support switching branches with '--shallow-clone', we add
'--no-single-branch' if the user specifies a branch other than master.

There is one snag though: this does not allow a user to switch branches
if they clone with '--shallow-clone' because '--[no-]single-branch' is a
cloning time decision. There will be a future patch that improves the
error reporting as well as help option to let users know that unless
they are seriously bandwidth restricted, they should be doing a full
clone.

Closes: ClangBuiltLinux#81
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Make it clear that this is an option that needs to be used with caution:

1. It is hard to flip around on branches due to an implied
   '--single-branch' during cloning by default.

2. It is hard to checkout to just a SHA1 (like when using
   '--use-good-revision' or tracking down an issue in the
   middle of master).

This is really an option designed for CIs where a one off clone is
needed for speed purposes. Manually managing the repo and taking the
script's heuristic out of it is usually a better option.

While we're here, alphabetize the position of '--shallow-clone'.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
GitHub does not support cloning straight from an arbitrary SHA1. Forbid
using these two options together as they will not work.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Our update workflow is:

1. Fetch the origin remote.

2. Checkout the ref requested by the user.

3. See if the ref is a branch.

4. Pull if it is.

Both full and shallow clones will fail at step 2 when the ref does not
exist. However, a ref might exist for a full clone and not for a shallow
clone and the error message might be confusing.

For example, if a user uses '--shallow-clone' with no '--branch'
argument, they will only get access to the default branch (master)
because of the implicit '--single-branch'. If they were to run the
script with '-b release/10.x' after that, the checkout would fail even
though they have technically supplied what would be a valid ref.

We could just live with this failure since the user did shoot themselves
in the foot by not following the advice in the '--shallow-clone' help
text. However, for a better UX, error before attempting a checkout when
.git/shallow is present and git show-branch errors on the ref. This
allows us to tell the user to either:

1. Remove the 'llvm-project' folder and re-run the script with
   '--shallow-clone' + '-b <branch>' so that '--no-single-branch' is
   passed.

2. Run 'git -C llvm-project fetch --unshallow origin' to get all of the
   history.

3. Just do the management themselves and pass in '--no-update'.

If the repo is unshallow, the 'git checkout' error is sufficient enough
to explain what went wrong.

This also adds a small note to the '--branch' help text that explains
'--shallow-clone' does not allow a commit hash to be passed to it
because GitHub does not support '--depth=1' from arbitrary commits.
Checking if a ref is a commit hash programmatically has its flaws so it
is not explicitly checked for.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
@dileks
Copy link
Contributor

dileks commented Apr 15, 2020

What to do with "tc-build: Check ref exists before checking out when repo is shallow" patch?
Does not apply on top of 4/4 patchset.

@dileks
Copy link
Contributor

dileks commented Apr 15, 2020

I tried (sorry partly German):

$ cd tc-build

$ ./build-llvm.py --branch release/10.x --shallow-clone

======================
== Downloading LLVM ==
======================

Klone nach '/home/dileks/src/llvm-toolchain/tc-build/llvm-project' ...
remote: Enumerating objects: 416201, done.
remote: Counting objects: 100% (416201/416201), done.
remote: Compressing objects: 100% (229094/229094), done.
remote: Total 416201 (delta 250839), reused 314868 (delta 179706), pack-reused 0
Empfange Objekte: 100% (416201/416201), 378.70 MiB | 2.48 MiB/s, Fertig.
Löse Unterschiede auf: 100% (250839/250839), Fertig.
Prüfe Konnektivität: 416201, Fertig.
Aktualisiere Dateien: 100% (91086/91086), Fertig.
Aktualisiere Dateien: 100% (28959/28959), Fertig.
Branch 'release/10.x' folgt nun Remote-Branch 'release/10.x' von 'origin'.
Zu neuem Branch 'release/10.x' gewechselt

$ cd llvm-project

$ git branch 
  master
* release/10.x

$ git branch -r
  origin/HEAD -> origin/master
  origin/master
  origin/release/1.0.x
  origin/release/1.1.x
  origin/release/1.2.x
  origin/release/1.3.x
  origin/release/1.4.x
  origin/release/1.5.x
  origin/release/1.6.x
  origin/release/1.7.x
  origin/release/1.8.x
  origin/release/1.9.x
  origin/release/10.x
  origin/release/2.0.x
  origin/release/2.1.x
  origin/release/2.2.x
  origin/release/2.3.x
  origin/release/2.4.x
  origin/release/2.5.x
  origin/release/2.6.x
  origin/release/2.7.x
  origin/release/2.8.x
  origin/release/2.9.x
  origin/release/3.0.x
  origin/release/3.1.x
  origin/release/3.2.x
  origin/release/3.3.x
  origin/release/3.4.x
  origin/release/3.5.x
  origin/release/3.6.x
  origin/release/3.7.x
  origin/release/3.8.x
  origin/release/3.9.x
  origin/release/4.x
  origin/release/5.x
  origin/release/6.x
  origin/release/7.x
  origin/release/8.x
  origin/release/9.x

$ git log --oneline -1
edbe962459da (grafted, HEAD -> release/10.x, origin/release/10.x) [COFF] Don't treat DWARF sections as GC roots

Copy link
Member

@msfjarvis msfjarvis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@kdrag0n kdrag0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior hasn't changed in my workflow.

@dileks
Copy link
Contributor

dileks commented Apr 15, 2020

Just curious and bandwith limitation does not allow me to test...

What happens when a branch does not exist?

$ ./build-llvm.py --branch release/11.x --shallow-clone

Can I do a shallow-clone on master (default) and in a 2nd step for release/10.x?

$ ./build-llvm.py --shallow-clone
$ ./build-llvm.py --shallow-clone --branch release/10.x

...and get the same result like doing step #2 alone?

Is there an option for clone-only to shallow-clone without starting to build the toolchain?

@msfjarvis
Copy link
Member

Just curious and bandwith limitation does not allow me to test...

What happens when a branch does not exist?

$ ./build-llvm.py --branch release/11.x --shallow-clone

It fails after fetching master.

======================
== Downloading LLVM ==
======================

Cloning into '/mnt/mediahell/tc-build/llvm-project'...
remote: Enumerating objects: 416246, done.
remote: Counting objects: 100% (416246/416246), done.
remote: Compressing objects: 100% (229139/229139), done.
remote: Total 416246 (delta 250889), reused 314835 (delta 179704), pack-reused 0
Receiving objects: 100% (416246/416246), 378.67 MiB | 3.87 MiB/s, done.
Resolving deltas: 100% (250889/250889), done.
Updating files: 100% (91098/91098), done.
error: pathspec 'release/11.x' did not match any file(s) known to git
Traceback (most recent call last):
  File "./build-llvm.py", line 994, in <module>
    main()
  File "./build-llvm.py", line 987, in main
    ref)
  File "./build-llvm.py", line 519, in fetch_llvm_binutils
    subprocess.run(["git", "checkout", ref], check=True, cwd=cwd)
  File "/usr/lib/python3.6/subprocess.py", line 438, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['git', 'checkout', 'release/11.x']' returned non-zero exit status 1.

Can I do a shallow-clone on master (default) and in a 2nd step for release/10.x?

No. The script will tell you its impossible for it to do and that its time for manual intervention.

===================
== Updating LLVM ==
===================


Supplied ref (release/10.x) does not exist, cannot checkout.

To proceed, either:

	1. Manage the repo yourself and pass '--no-update' to the script.

	2. Run 'git -C /mnt/mediahell/tc-build/llvm-project fetch --unshallow origin' to get a complete repository.

	3. Delete '/mnt/mediahell/tc-build/llvm-project' and re-run the script with '-s' + '-b <ref>' to get a full set of refs.

Is there an option for clone-only to shallow-clone without starting to build the toolchain?

No, that makes little sense in what is supposed to be a build script.

@nathanchance nathanchance merged commit e8ad1cf into ClangBuiltLinux:master Apr 16, 2020
@nathanchance nathanchance deleted the shallow-clone-with-branch branch April 16, 2020 18:59
@dileks
Copy link
Contributor

dileks commented Apr 16, 2020

Thanks you @msfjarvis and @nathanchance for making this happen!

We are very close to get a Green-IT award.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants