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

Add --progress to git submodule commands in x.py #56933

Merged
merged 1 commit into from
Dec 23, 2018

Conversation

clarfonthey
Copy link
Contributor

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2018
@clarfonthey clarfonthey changed the title Add --progress to git submodule update in x.py Add --progress to git submodule commands in x.py Dec 17, 2018
@nikomatsakis
Copy link
Contributor

cc @rust-lang/infra -- I have no strong opinion about this. When was this flag added? Are we concerned about compatibility with older git installations? Maybe this can cause problems with non-interactive runs?

r? @Mark-Simulacrum to assign someone appropriate =)

@Mark-Simulacrum
Copy link
Member

I personally think we can probably just go ahead and do this -- if we get bug reports then we can revert the PR. AFAICT, the option is present in the git version packaged with Ubuntu 18.04 (latest LTS) so in that regard I'd expect it to be fairly widely available.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 19, 2018

📌 Commit 6130fc8 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 19, 2018
@clarfonthey
Copy link
Contributor Author

Did some digging and this was added in git 2.18.0, which was released around the end of June. The latest version is 2.20.0, so, this was two versions ago.

This isn't really to change the decision, but considering how I looked this up, I figured I'd mention it for good measure.

@Mark-Simulacrum
Copy link
Member

I thought so too, but it seemed to work for me on git 2.17... so it feels like perhaps that information is misleading. I thought I'd noted it in my comment but I think I forgot to do so.

@clarfonthey
Copy link
Contributor Author

Oh, I appear to be mistaken… the flag was added to git submodule add in 2.18.0, but to git submodule update in 2.11.0. We should have nothing to worry about, then.

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 20, 2018
…lacrum

Add --progress to git submodule commands in x.py

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 21, 2018
…lacrum

Add --progress to git submodule commands in x.py

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 21, 2018
…lacrum

Add --progress to git submodule commands in x.py

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.
bors added a commit that referenced this pull request Dec 21, 2018
Rollup of 20 pull requests

Successful merges:

 - #56802 (Add DoubleEndedIterator::nth_back)
 - #56842 (Add unstable VecDeque::rotate_{left|right})
 - #56869 (Reduce search-index.js size)
 - #56887 (Disable field reordering for repr(int).)
 - #56892 (rustc: Update Clang used to build LLVM on Linux)
 - #56909 (static eval: Do not ICE on layout size overflow)
 - #56914 (Ignore ui/target-feature-gate on sparc, sparc64, powerpc, powerpc64 and powerpc64le)
 - #56917 (Simplify MIR generation for logical operations)
 - #56919 (Remove a wrong multiplier on relocation offset computation)
 - #56933 (Add --progress to git submodule commands in x.py)
 - #56941 (deny intra-doc link resolution failures in libstd)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56970 (Mem uninit doc ptr drop)
 - #56973 (make basic CTFE tracing available on release builds)
 - #56979 (Adding unwinding support for x86_64_fortanix_unknown_sgx target.)
 - #56981 (miri: allocation is infallible)
 - #56984 (A few tweaks to dropck_outlives)
 - #56989 (Fix compiletest `trim` deprecation warnings)
 - #56992 (suggest similar lint names for unknown lints)
 - #57002 (Stabilize Vec(Deque)::resize_with)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Dec 21, 2018
Rollup of 20 pull requests

Successful merges:

 - #56802 (Add DoubleEndedIterator::nth_back)
 - #56842 (Add unstable VecDeque::rotate_{left|right})
 - #56869 (Reduce search-index.js size)
 - #56887 (Disable field reordering for repr(int).)
 - #56892 (rustc: Update Clang used to build LLVM on Linux)
 - #56909 (static eval: Do not ICE on layout size overflow)
 - #56914 (Ignore ui/target-feature-gate on sparc, sparc64, powerpc, powerpc64 and powerpc64le)
 - #56917 (Simplify MIR generation for logical operations)
 - #56919 (Remove a wrong multiplier on relocation offset computation)
 - #56933 (Add --progress to git submodule commands in x.py)
 - #56941 (deny intra-doc link resolution failures in libstd)
 - #56964 (Remove `TokenStream::JointTree`.)
 - #56970 (Mem uninit doc ptr drop)
 - #56973 (make basic CTFE tracing available on release builds)
 - #56979 (Adding unwinding support for x86_64_fortanix_unknown_sgx target.)
 - #56981 (miri: allocation is infallible)
 - #56984 (A few tweaks to dropck_outlives)
 - #56989 (Fix compiletest `trim` deprecation warnings)
 - #56992 (suggest similar lint names for unknown lints)
 - #57002 (Stabilize Vec(Deque)::resize_with)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2018
…lacrum

Add --progress to git submodule commands in x.py

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.
kennytm added a commit to kennytm/rust that referenced this pull request Dec 22, 2018
…lacrum

Add --progress to git submodule commands in x.py

This is a relatively new flag, but it means that git will indicate the progress of the update as it would with regular clones. This is especially helpful as some of the submodules are really big and it's difficult to tell if it's hanging or still updating.
bors added a commit that referenced this pull request Dec 22, 2018
Rollup of 25 pull requests

Successful merges:

 - #56802 (Add DoubleEndedIterator::nth_back)
 - #56909 (static eval: Do not ICE on layout size overflow)
 - #56914 (Ignore ui/target-feature-gate on sparc, sparc64, powerpc, powerpc64 and powerpc64le)
 - #56919 (Remove a wrong multiplier on relocation offset computation)
 - #56933 (Add --progress to git submodule commands in x.py)
 - #56936 (rename div_euc -> div_euclid, and mod_euc -> rem_euclid)
 - #56941 (deny intra-doc link resolution failures in libstd)
 - #56945 (Fix rustdoc-js tests)
 - #56967 (Replace current crate's searchIndex when regenerating)
 - #56970 (Mem uninit doc ptr drop)
 - #56973 (make basic CTFE tracing available on release builds)
 - #56979 (Adding unwinding support for x86_64_fortanix_unknown_sgx target.)
 - #56981 (miri: allocation is infallible)
 - #56984 (A few tweaks to dropck_outlives)
 - #56989 (Fix compiletest `trim` deprecation warnings)
 - #56992 (suggest similar lint names for unknown lints)
 - #57002 (Stabilize Vec(Deque)::resize_with)
 - #57011 (rustdoc: add new CLI flag to load static files from a different location)
 - #57027 (Optimize away a move)
 - #57034 (Inline tweaks)
 - #57039 (Update migrate warning wording.)
 - #57040 (Fix feature gate to point to 1.32.0 for `path_from_str`)
 - #57049 (Stabilize #[repr(packed(N))])
 - #57050 (Fixed typo in HashMap documentation)
 - #57052 (Fix stabilization version numbers (exhaustive_integer_patterns + macro_literal_matcher))
@bors bors merged commit 6130fc8 into rust-lang:master Dec 23, 2018
@glaubitz
Copy link
Contributor

glaubitz commented Dec 23, 2018

I think this has caused a regression in Debian:

glaubitz@epyc:..rust/rust-m68k> ./x.py build
Updating only changed submodules
Updating submodule src/llvm
usage: git submodule [--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
   or: git submodule [--quiet] status [--cached] [--recursive] [--] [<path>...]
   or: git submodule [--quiet] init [--] [<path>...]
   or: git submodule [--quiet] deinit [-f|--force] (--all| [--] <path>...)
   or: git submodule [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
   or: git submodule [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
   or: git submodule [--quiet] foreach [--recursive] <command>
   or: git submodule [--quiet] sync [--recursive] [--] [<path>...]
   or: git submodule [--quiet] absorbgitdirs [--] [<path>...]
failed to run: git submodule -q sync --progress src/llvm
Build completed unsuccessfully in 0:00:00
glaubitz@epyc:..rust/rust-m68k>

Has it been verified that this flag already works with the git version in Debian and Ubuntu?

Edit: The manpage for git-submodule mentions --progress, but I am still getting that error messsage. Odd.

@pietroalbini
Copy link
Member

pietroalbini commented Dec 23, 2018

This indeed doesn't work with Ubuntu 16.04's git (git 2.7.4).

@glaubitz
Copy link
Contributor

And reverting 6130fc8 fixes it for me. I'm on Debian unstable with git 2.20.1.

@matthiaskrgr
Copy link
Member

#57082 will revert back to a working state until this is sorted out.

@clarfonthey clarfonthey deleted the xpy_progress branch December 23, 2018 19:15
@nrc
Copy link
Member

nrc commented Jan 11, 2019

I have an older version of Git on my build server (because updating Linux is a shit show) and this breaks my build :-( There's an easy work around for me, so it doesn't need to be backed out, but we might consider users with older Git.

@Mark-Simulacrum
Copy link
Member

I think this has already been reverted? If Ubuntu 16.04 as the previous LTS has turned out to be too old I think a revert is warranted myself.

@nrc
Copy link
Member

nrc commented Jan 11, 2019

No, it was partially reverted. I have 16.10, and that seems to be too old to have --progress

@froydnj
Copy link
Contributor

froydnj commented Apr 25, 2019

@Mark-Simulacrum are you still willing to revert this? The patch was only partially reverted in #57082, and I ran into #57080 today (Ubuntu 16.04 LTS).

@Mark-Simulacrum
Copy link
Member

Yes, I think either reverting or rerunning if the git command fails makes sense.

@clarfonthey
Copy link
Contributor Author

I think that rerunning if it fails is probably best considering how the lack of progress makes it incredibly difficult to see how long the command is going to take, or what part it's hanging on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants