[PATCH v3] pull: pass --signoff/--no-signoff to "git merge"
merge can take --signoff, but without pull passing --signoff down, it
is inconvenient, so let's pass it through.
The order of options in merge-options.txt is mostly alphabetical by
long option since 7c85d274 (Documentation/merge-options.txt: order
options in alphabetical groups, 2009-10-22). The long-option bit
didn't make it into the commit message, but it's under the fold in
[1]. I've put --signoff between --log and --stat to preserve the
alphabetical order.
[1]: https://public-inbox.org/git/87iqe7zspn@jondo.cante.net/
Helped-by: Junio C Hamano
Signed-off-by: W. Trevor King
---
Changes since v2 [1]:
* Replace the two motivational paragraphs with Junio's suggested
sentence [2].
* Drop test_hash_trailer in favor of --pretty='format:%(trailers)'
[3]. It turns out that the builtin tooling I was looking for while
working on test_hash_trailer already exists :).
This patch (like v1 and v2) is based on v2.15.0-rc1, although I expect
it will apply cleanly to anything in that vicinity.
Cheers,
Trevor
[1]:
https://public-inbox.org/git/51d67d6d707182d4973d9961ab29358f26c4988a.1507796638.git.wk...@tremily.us/
[2]: https://public-inbox.org/git/xmqqk200znel@gitster.mtv.corp.google.com/
[3]: https://public-inbox.org/git/xmqq7ew0zkqv@gitster.mtv.corp.google.com/
Documentation/git-merge.txt | 8
Documentation/merge-options.txt | 10 +
builtin/pull.c | 6 ++
t/t5521-pull-options.sh | 45 +
4 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ OPTIONS
---
include::merge-options.txt[]
---signoff::
- Add Signed-off-by line by the committer at the end of the commit
- log message. The meaning of a signoff depends on the project,
- but it typically certifies that committer has
- the rights to submit this work under the same license and
- agrees to a Developer Certificate of Origin
- (see http://developercertificate.org/ for more information).
-
-S[]::
--gpg-sign[=]::
GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
With --no-log do not list one-line descriptions from the
actual commits being merged.
+--signoff::
+--no-signoff::
+ Add Signed-off-by line by the committer at the end of the commit
+ log message. The meaning of a signoff depends on the project,
+ but it typically certifies that committer has
+ the rights to submit this work under the same license and
+ agrees to a Developer Certificate of Origin
+ (see http://developercertificate.org/ for more information).
++
+With --no-signoff do not add a Signed-off-by line.
--stat::
-n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..0413c78a3a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static enum rebase_type opt_rebase = -1;
static char *opt_diffstat;
static char *opt_log;
+static char *opt_signoff;
static char *opt_squash;
static char *opt_commit;
static char *opt_edit;
@@ -142,6 +143,9 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "log", &opt_log, N_("n"),
N_("add (at most ) entries from shortlog to merge commit
message"),
PARSE_OPT_OPTARG),
+ OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
+ N_("add Signed-off-by:"),
+ PARSE_OPT_OPTARG),
OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
N_("create a single commit instead of doing a merge"),
PARSE_OPT_NOARG),
@@ -594,6 +598,8 @@ static int run_merge(void)
argv_array_push(&args, opt_diffstat);
if (opt_log)
argv_array_push(&args, opt_log);
+ if (opt_signoff)
+ argv_array_push(&args, opt_signoff);
if (opt_squash)
argv_array_push(&args, opt_squash);
if (opt_commit)
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..c19d8dbc9d 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,49 @@ test_expect_success 'git pull --allow-unrelated-histories'
'
)
'
+test_expect_success 'git pull does not add a sign-off line' '
+ test_when_finished "rm -fr src dst actual" &&
+ git init src &&
Re: [PATCH v2] pull: pass --signoff/--no-signoff to "git merge"
On Thu, Oct 12, 2017 at 01:46:39AM -0700, W. Trevor King wrote: > The order of options in merge-options.txt isn't clear to me, but > I've put --signoff between --log and --stat as somewhat alphabetized > and having an "add to the commit message" function like --log. The order of options in merge-options.txt was intended to be by "alphabetical groups", at least back in 7c85d274 (Documentation/merge-options.txt: order options in alphabetical groups, 2009-10-22). I'm not quite clear on what that means. After 7c85d274 landed there were already long-option irregularities: $ git grep -h ^-- 7c85d27 -- Documentation/merge-options.txt --commit:: --no-commit:: --ff:: --no-ff:: --log:: --no-log:: --stat:: --no-stat:: --squash:: --no-squash:: --strategy=:: --summary:: --no-summary:: --quiet:: --verbose:: If the order was purely alphabetical, --stat/--no-stat should have after --squash/--no-squash, and --quiet should have been much earlier. And putting --signoff after --log is still alphabetical in v2.15.0-rc1 (ignoring a few outliers). So I don't think it's a reason to change where I'd put the option, but in v3 of this patch I'll update the commit message to cite 7c85d274 when motivating the location. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] Documentation/merge-options.txt: Add -S/--gpg-sign
Pull has supported these since ea230d8 (pull: add the --gpg-sign option, 2014-02-10). Insert in long-option alphabetical order following 7c85d274 (Documentation/merge-options.txt: order options in alphabetical groups, 2009-10-22). Signed-off-by: W. Trevor King --- This patch is based on maint. It will have trivial conflicts with the --signoff docs which landed in 14d01b4f07 (merge: add a --signoff flag, 2017-07-04, v2.15.0-rc0~138^2). Documentation/git-merge.txt | 6 -- Documentation/merge-options.txt | 6 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index f90faf7aaa..1d97a17904 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -64,12 +64,6 @@ OPTIONS --- include::merge-options.txt[] --S[]:: ---gpg-sign[=]:: - GPG-sign the resulting merge commit. The `keyid` argument is - optional and defaults to the committer identity; if specified, - it must be stuck to the option without a space. - -m :: Set the commit message to be used for the merge commit (in case one is created). diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 5b4a62e936..6d85a76872 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -42,6 +42,12 @@ set to `no` at the beginning of them. current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. +-S[]:: +--gpg-sign[=]:: + GPG-sign the resulting merge commit. The `keyid` argument is + optional and defaults to the committer identity; if specified, + it must be stuck to the option without a space. + --log[=]:: --no-log:: In addition to branch names, populate the log message with -- 2.13.6
[PATCH v2] pull: pass --signoff/--no-signoff to "git merge"
e379fdf3 (merge: refuse to create too cool a merge by default, 2016-03-18) gave a reason for *not* passing options from pull to merge: ...because such a "two project merge" would be done after fetching the other project into some location in the working tree of an existing project and making sure how well they fit together... That's certainly an acceptable workflow, but I'd also like to support merge options in pull for folks who want to optimistically pull and then sort out "how well they fit together" after pull exits (possibly with a merge failure). And in cases where an optimistic pull is likely to succeed, suggesting it is easier to explain to Git newbies than a FETCH_HEAD merge or remote-addition/merge/remote-removal. 09c2cb87 (pull: pass --allow-unrelated-histories to "git merge", 2016-03-18) added a pull-to-merge pass for a different option but didn't motivate its change, only citing the reason from e379fdf3 for not adding the pull-to-merge pass for that option. I'm personally in favor of pull-to-merge passing for any unambiguous options, but if the decision for pull-to-merge passes depends on the specific option, then --allow-unrelated-histories is probably the weakest candidate because unrelated-history merged are more likely to have "fit together" issues than the other merge-only options. The test_has_trailer helper gives folks a convenient way check these sorts of things. I haven't gone through and updated existing trailer checks (e.g. the ones in t7614-merge-signoff.sh) to avoid the "only to correct the inconconsistency" issue discussed in SubmittingPatches. Other test may gradually migrate to the new helper if they find it useful. The helper may be useful enough to eventually become a plumbing command (a read version of interpret-trailers with an API similar to 'git config ...'?), but I'm not going that far in this commit ;). The order of options in merge-options.txt isn't clear to me, but I've put --signoff between --log and --stat as somewhat alphabetized and having an "add to the commit message" function like --log. Helped-by: Junio C Hamano Signed-off-by: W. Trevor King --- Changes since v1 [1]: * Dropped "Following" paragraph. Junio took issue with the phrasing [2], and the implementation in v2 has diverged sufficiently from 09c2cb87 and 14d01b4f that I don't think citing them as implementation references is useful anymore. * Lead the commit message with reworked motivation paragraphs, since Junio read the v1 motivation paragraph as off-topic [2]. * Add a test_has_trailer helper, which I'd floated in [3] after Junio's get_signoff suggestion in [2]. * Drop subshells in favor of '-C ' in the tests, as suggested in [2]. * Add tests for the bare pull and lonely --no-signoff cases, as suggested in [2]. With these additions in place, I've dropped v1's "The tests aren't as extensive..." paragraph from the commit message. * Use OPT_PASSTHRU in pull.c. I'm not sure why --allow-unrelated-histories didn't go this route, but there are lots of other pull-to-merge options using OPT_PASSTHRU, so using it for --signoff isn't breaking consistency. Not changed since v1: * The merge-options.txt order paragraph. Junio had suggested it be moved after the break [2], but I think having some commit-message discussion of merge-options.txt ordering is useful, even though I don't have strong opinions on what the ordering should be [3]. This patch (and v1) are based on v2.15.0-rc1, although I expect they'll apply cleanly to anything in that vicinity. Cheers, Trevor [1]: https://public-inbox.org/git/18953f46ffb5e3dbc4da8fbda7fe3ab4298d7cbd.1507752482.git.wk...@tremily.us/ [2]: https://public-inbox.org/git/xmqqefq92mgw@gitster.mtv.corp.google.com/ [3]: https://public-inbox.org/git/20171012053002.gz11...@valgrind.tremily.us/ Documentation/git-merge.txt | 8 Documentation/merge-options.txt | 10 ++ builtin/pull.c | 6 ++ t/t5521-pull-options.sh | 40 ++ t/test-lib-functions.sh | 43 + 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 4df6431c34..0ada8c856b 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -64,14 +64,6 @@ OPTIONS --- include::merge-options.txt[] ---signoff:: - Add Signed-off-by line by the committer at the end of the commit - log message. The meaning of a signoff depends on the project, - but it typically certifies that committer has - the rights to submit this work under the same license and - agrees to a Developer Certificate of Origin - (see htt
Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"
On Thu, Oct 12, 2017 at 02:42:30PM +0900, Junio C Hamano wrote: > "W. Trevor King" writes: > > On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote: > >> "W. Trevor King" writes: > >> > >> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git > >> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge: > >> > add a --signoff flag, 2017-07-04). > >> > >> I cannot find a verb in the above. > > > > I'd meant it as either a continuation of the subject line, or with an > > Never do that. The title should be able to stand on its own, and > must not be an early part of incomplete sentence. “Following” to an imperative “Follow” it is then, unless you want a more drastic rewording. > > Sounds good. I'll add a patch to v2 to make the same change to > > the existing t5521 --allow-unrelated-histories test. > > Please don't, unless you are actively working on the features that > they test. We do not have infinite amount of bandwidth to deal with > changes for the sake of perceived consistency and no other real > gain. By extention, I'm guessing that means that while the: test_has_trailer $OBJECT $TOKEN $VALUE and: test_has_no_trailer $OBJECT $TOKEN test-lib-functions.sh helpers I floated may be acceptable (or not, no need to commit before you've seen a patch), you don't want me updating existing tests to use them. I'll just use them in my new tests, and folks can gradually transition existing tests to them as they touch those tests (if they remember the helpers exist ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"
On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote: > "W. Trevor King" writes: > > > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git > > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge: > > add a --signoff flag, 2017-07-04). > > I cannot find a verb in the above. I'd meant it as either a continuation of the subject line, or with an implicit leading “I did this…” :p. I can reword if you like, maybe just “Following” → “Follow”? Something more drastic? > > The order of options in merge-options.txt isn't clear to me, but > > I've put --signoff between --log and --stat as somewhat > > alphabetized and having an "add to the commit message" function > > like --log. > > > > The tests aren't as extensive as t7614-merge-signoff.sh, but they > > exercises both the --signoff and --no-signoff options. There may > > be a more efficient way to set them up (like > > t7614-merge-signoff.sh's test_setup), but with all the pull > > options packed into a single test script it seemed easiest to just > > copy/paste the duplicate setup code. > > The above two paragraphs read more like "requesting help for hints > to improve this patch" than commit log message. Perhaps move them > below the three-dash line and instead describe what you actually did > here (if they were worth explaining, that is)? I think something about merge-options.txt ordering should end up in the history of that content. Reading through: $ git log Documentation/merge-options.txt only turned up 690b2975 (Documentation/merge-options.txt: group "ff" related options together, 2012-02-22) discussing option order (it suggested grouping similar options together, although --ff and --ff-only would also be close alphabetically). I agree that the first paragraph you quote above doesn't have me coming down firmly in favor of a particular ordering strategy, but I think having something like it in the Git history will help whoever ends up giving merge-options.txt a well-defined strategy by showing I didn't have any strong opinions to account for ;). Silence can mean “doesn't have a strong opinion”, but sometimes it means “feels the choice is so obvious that it doesn't need explicit motivation”. I'm fine moving the second paragraph you quote below the fold in a v2, although you're calling for more tests below, and it won't apply anymore once I've added those :). > > 09c2cb87 didn't motivate the addition of --allow-unrelated-histories > > to pull; only citing the reason from e379fdf3 (merge: refuse to create > > too cool a merge by default, 2016-03-18) gave for *not* including it. > > I like having both exposed in pull because while the fetch-and-merge > > approach might be a more popular way to judge "how well they fit > > together", you can also do that after an optimistic pull. And in > > cases where an optimistic pull is likely to succeed, suggesting it is > > easier to explain to Git newbies than a FETCH_HEAD merge. > > I find this paragraph totally unrelated to what the patch does. > Save it for the patch you add to pass --allow-unrelated-histories > given to pull down to underlying merge, perhaps? 09c2cb87 is your commit in master (v2.9.0-rc0~88^2) that is doing just that. I haven't gone through the list history to figure out why it ended up getting landed with its current commit message; “Prepare a patch to make it a reality, just in case it is needed” sounds more like it was “here's the code in case folks want it, I'll reroll the motivation if they do”. This paragraph was aiming to motivate both the --signoff pass-through I'm adding here and (retroactively) the --allow-unrelated-histories pass-through you added there. I'll add more context in v2 to try to make that more clear. > > + cat >expected <<-EOF && > > + Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e > > "s/>.*/>/") > > + EOF > > echo "Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>" >expect Much nicer, thanks. I'll add a patch to v2 to make the same change to t7614. > > + git init src && > > + ( > > + cd src && > > + test_commit one > > + ) && > > I suspect somebody will suggest "test_commit -C" ;-) Sounds good. I'll add a patch to v2 to make the same change to the existing t5521 --allow-unrelated-histories test. > > + git clone src dst && > > + ( > > + cd src && > > + test_commit two > > + ) && > >
[PATCH] pull: pass --signoff/--no-signoff to "git merge"
Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
add a --signoff flag, 2017-07-04).
The order of options in merge-options.txt isn't clear to me, but I've
put --signoff between --log and --stat as somewhat alphabetized and
having an "add to the commit message" function like --log.
The tests aren't as extensive as t7614-merge-signoff.sh, but they
exercises both the --signoff and --no-signoff options. There may be a
more efficient way to set them up (like t7614-merge-signoff.sh's
test_setup), but with all the pull options packed into a single test
script it seemed easiest to just copy/paste the duplicate setup code.
09c2cb87 didn't motivate the addition of --allow-unrelated-histories
to pull; only citing the reason from e379fdf3 (merge: refuse to create
too cool a merge by default, 2016-03-18) gave for *not* including it.
I like having both exposed in pull because while the fetch-and-merge
approach might be a more popular way to judge "how well they fit
together", you can also do that after an optimistic pull. And in
cases where an optimistic pull is likely to succeed, suggesting it is
easier to explain to Git newbies than a FETCH_HEAD merge.
Signed-off-by: W. Trevor King
---
Documentation/git-merge.txt | 8
Documentation/merge-options.txt | 10 ++
builtin/pull.c | 8
t/t5521-pull-options.sh | 43 +
4 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ OPTIONS
---
include::merge-options.txt[]
---signoff::
- Add Signed-off-by line by the committer at the end of the commit
- log message. The meaning of a signoff depends on the project,
- but it typically certifies that committer has
- the rights to submit this work under the same license and
- agrees to a Developer Certificate of Origin
- (see http://developercertificate.org/ for more information).
-
-S[]::
--gpg-sign[=]::
GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
With --no-log do not list one-line descriptions from the
actual commits being merged.
+--signoff::
+--no-signoff::
+ Add Signed-off-by line by the committer at the end of the commit
+ log message. The meaning of a signoff depends on the project,
+ but it typically certifies that committer has
+ the rights to submit this work under the same license and
+ agrees to a Developer Certificate of Origin
+ (see http://developercertificate.org/ for more information).
++
+With --no-signoff do not add a Signed-off-by line.
--stat::
-n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..4469342f51 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -97,6 +97,7 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
static char *opt_gpg_sign;
static int opt_allow_unrelated_histories;
+static int opt_signoff;
/* Options passed to git-fetch */
static char *opt_all;
@@ -175,6 +176,9 @@ static struct option pull_options[] = {
OPT_SET_INT(0, "allow-unrelated-histories",
&opt_allow_unrelated_histories,
N_("allow merging unrelated histories"), 1),
+ OPT_BOOL(0, "signoff",
+ &opt_signoff,
+ N_("add Signed-off-by:")),
/* Options passed to git-fetch */
OPT_GROUP(N_("Options related to fetching")),
@@ -610,6 +614,10 @@ static int run_merge(void)
argv_array_push(&args, opt_gpg_sign);
if (opt_allow_unrelated_histories > 0)
argv_array_push(&args, "--allow-unrelated-histories");
+ if (opt_signoff > 0)
+ argv_array_push(&args, "--signoff");
+ else
+ argv_array_push(&args, "--no-signoff");
argv_array_push(&args, "FETCH_HEAD");
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..d95789ab8c 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,47 @@ test_expect_success 'git pull --allow-unrelated-histories'
'
)
'
+test_expect_success 'git pull --signoff add a sign-off line' &#x
Re: [git] Distributed code review discussion
On Sat, Jan 10, 2015 at 01:05:35PM -0500, Matus Faro wrote: > What I mean is a distributed code review system where a merge > request along with review comments would be stored within the git > repository and allowed to be pushed and pulled between repository > clones. This would allow users to retain the code review history > without relying on centralized or proprietary third party solutions. You can do this today with pull-requests and code review happening in email, since you just have to distribute the mail archives to have a record of pull-requests and code review discussions (e.g. notmuch posts an mbox of it's list archives [1]). For better usability, you have a few more options borrowing from the notmuch workflow: * Index messages in notmuch [2] (like notmuch does) and tag them so you know what branches are waiting on review [3] or author feedback [4]. * Use nmbug [5] to collaborate on tagging and distribute tags to interested parties. Things I've been thinking about doing “at some point”: * Use ssoma [6] or my ssoma-mda Python port [7] to store the list archives in Git using this format [8]. I'd like to teach notmuch to read messages directly from the ssoma archive, which would let you replace the mbox archive with something that's easier to collaborate on than an mbox archive (e.g. for removing duplicates or fixing typos). If all interested parties are using ssoma-style storage for the archives, you don't have to worry about “oops, I didn't mean to hit send” types of errors, since it's easy to patch the archive itself. * Provide a web-UI for browsing the archive and manipulating tags, so folks don't need to install Git / notmuch / ssoma to get involved in patch review. I'd still have them submit comments via email, but you could have the web-app send email for them if you have anti-email users ;). Cheers, Trevor [1]: http://notmuchmail.org/archives/notmuch.mbox [2]: http://notmuchmail.org/ [3]: http://nmbug.tethera.net/status/#Review [4]: http://nmbug.tethera.net/status/#Moreinfo [5]: http://notmuchmail.org/nmbug/ [6]: http://ssoma.public-inbox.org/README [7]: http://git.tremily.us/?p=ssoma-mda.git [8]: http://ssoma.public-inbox.org/ssoma_repository.txt -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Joining historical repository using grafts or replace
On Thu, Oct 30, 2014 at 06:39:56PM +0300, Dmitry Oksenchuk wrote: > We're in the middle of conversion of a large CVS repository (20 > years, 70K commits, 1K branches, 10K tags) to Git and considering > two separate Git repositories: "historical" with CVS history and > "working" created without history from heads of active branches (10 > active branches). This allows us to have small fast "working" > repository for developers who don't want to have full history > locally and ability to rewrite history in "historical" repository > (for example, to add parents to merge commits or to fix conversion > mistakes) without affecting commit hashes in "working" repository > (the hashes can be stored in bug tracker or in the code). A number of projects have done something like this (e.g. Linux). Modern Gits have good support for shallow repositories though, so I'd just make one full repository and leave it to developers to decide how deep they want their local copy to be. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] t1304: Set LOGNAME even if USER is unset or null
On Sun, Oct 19, 2014 at 03:49:36PM -0700, Junio C Hamano wrote:
> I'll queue this as-is, but it makes me wonder if we want to do this
> without if/then/fi, e.g.
>
> : ${LOGNAME:=${USER:-$(id -u -n)}
I'm fine with that too.
> Spelling everything out with if/then/fi is obviously at the other
> extreme, i.e.
And I'm fine with this ;).
> More importantly, what if none of the alternatives work? I
> personally feel it is OK to punt and declare test_done early,
> instead of giving false positive breakages like you saw without this
> patch.
I can put this into a v2 if you like. Which conditional syntax do you
prefer?
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
[PATCH] t1304: Set LOGNAME even if USER is unset or null
Avoid:
# ./t1304-default-acl.sh
ok 1 - checking for a working acl setup
ok 2 - Setup test repo
not ok 3 - Objects creation does not break ACLs with restrictive umask
#
# # SHA1 for empty blob
# check_perms_and_acl
.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
#
not ok 4 - git gc does not break ACLs with restrictive umask
#
# git gc &&
# check_perms_and_acl .git/objects/pack/*.pack
#
# failed 2 among 4 test(s)
1..4
on systems where USER isn't set. It's usually set by the login
process, but it isn't set when launching some Docker images. For
example:
$ docker run --rm debian env
HOME=/
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=b2dfdfe797ed
'id -u -n' has been in POSIX from Issue 2 through 2013 [1], so I don't
expect compatibility issues.
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/id.html
Signed-off-by: W. Trevor King
---
The patch is based on the current maint branch.
Previous LOGNAME discussion:
* Michael Gruber on 2011-05-06 suggesting a discussing a whoami
fallback [1] (but whoami isn't POSIX).
* René Scharfe on 2011-10-14 suggesting USER as a fallback for
LOGNAME [2].
* Matthieu Moy on 2012-09-17 suggesting dropping $LOGNAME in
favor of numerical user IDs 'id -u' for a system with multiple
usernames sharing the same user ID [3].
Obviously, you can work around the problem with:
# USER=$(id -u -n) ./t1304-default-acl.sh
so the question is really "Are empty-USER systems worth supporting out
of the box?".
Cheers,
Trevor
[1]: http://thread.gmane.org/gmane.comp.version-control.git/172883/focus=172961
[2]: http://thread.gmane.org/gmane.comp.version-control.git/183586
[3]: http://thread.gmane.org/gmane.comp.version-control.git/205690/focus=205703
t/t1304-default-acl.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 79045ab..f5422f1 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -26,7 +26,7 @@ test_expect_success 'checking for a working acl setup' '
if test -z "$LOGNAME"
then
- LOGNAME=$USER
+ LOGNAME="${USER:-$(id -u -n)}"
fi
check_perms_and_acl () {
--
2.1.0.60.g85f0837
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs/git-mailinfo: Mention the manual separator (---)
On Tue, Sep 30, 2014 at 02:12:58PM -0700, Junio C Hamano wrote:
> If we are extending the documentation on "---", …
Ah, I see that the --- are actually mentioned already in the
DISCUSSION section of git-am(1) since 2499857b (git-am documentation:
describe what is taken from where, 2007-03-24). I expected the docs
to be either in git-mailinfo(1) (since the code added by f0658cf2
(restrict the patch filtering, 2007-03-12) is in mailinfo) or to match
a grep for '---'. Maybe we should drop this patch in favor of notes
in git-mailinfo(1) and git-format-patch(1) pointing folks at the
DISCUSSION section in git-am(1) and a more easily grepable “three
dashes ('---')" in gi-am(1)?
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [git] What's cooking in git.git (Sep 2014, #09; Tue, 30)
On Tue, Sep 30, 2014 at 01:23:18PM -0700, Junio C Hamano wrote: > Here are the topics that have been cooking. It looks like my boring git-mailinfo doc patch [1] fell through the cracks here ;). Or maybe it's just cooking a bit longer before getting queued? Cheers, Trevor [1]: Gmane: http://article.gmane.org/gmane.comp.version-control.git/257460 Subject: [PATCH] docs/git-mailinfo: Mention the manual separator (---) Message-ID: <28b04f1c17f2cc2fe252948bc0b7bb10df24b489.1411571629.git.wk...@tremily.us> Date: Wed, 24 Sep 2014 08:25:32 -0700 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] docs/git-mailinfo: Mention the manual separator (---)
And explain how it interacts with the scissors setting. Signed-off-by: W. Trevor King --- The three-dash limit comes from f0658cf2 (restrict the patch filtering, 2007-03-12), but I couldn't find any associated documentation. Since the effect is so similar to the scissors line, I thought about adding the information to the --scissors entry. The manual separator is really independent from the scissors though, so I settled on explaining both separators in the DESCRIPTION. This patch is against 'maint'. Documentation/git-mailinfo.txt | 23 +++ 1 file changed, 23 insertions(+) diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt index 164a3c6..6c6c527 100644 --- a/Documentation/git-mailinfo.txt +++ b/Documentation/git-mailinfo.txt @@ -21,6 +21,29 @@ written out to the standard output to be used by 'git am' to create a commit. It is usually not necessary to use this command directly. See linkgit:git-am[1] instead. +The commit message extracted from the e-mail depends on the scissors +setting (see '--[no-]scissors' in the OPTIONS section). Besides the +scissors option (which discards content before the scissors), you can +also use '---' as a separator (which discards content after the +separator). For example, without scissors you can have a body like +this: + + +Your commit message. +--- +Comments that aren't part of the commit message. + + +With scissors, you can have a body like this: + + +Comments that aren't part of the commit message. +--->8--- +Your commit message. +--- +More comments that aren't part of the commit message. + + OPTIONS --- -- 2.1.0.60.g85f0837 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pre-push.sample: Write error message to stderr
githooks(5) suggests: Information about why the push is rejected may be sent to the user by writing to standard error. So follow that advice in the sample. Signed-off-by: W. Trevor King --- templates/hooks--pre-push.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample index 1f3bceb..7cb911c 100755 --- a/templates/hooks--pre-push.sample +++ b/templates/hooks--pre-push.sample @@ -45,7 +45,7 @@ do commit=`git rev-list -n 1 --grep '^WIP' "$range"` if [ -n "$commit" ] then - echo "Found WIP commit in $local_ref, not pushing" + echo "Found WIP commit in $local_ref, not pushing" >&2 exit 1 fi fi -- 2.1.0.60.g85f0837 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git] Re: Relative submodule URLs, and forks that haven't forked the submodule
On Thu, Jun 12, 2014 at 06:05:10PM +0200, Charles Brossollet wrote:
> The two problems I'm pointing are:
>
> 1. After checkout of a branch that tracks /user/main repo, call git
>init submodule motors. Git registers it in .git/config with URL
>/user/sub, while it should be /central/sub according to
>documentation because origin's URL is at /central.
The logic for this is in resolve_relative_url, defined in
git-submodule.sh. The remote it uses is calculated using
get_default_remote, defined in git-parse-remote.sh:
get_default_remote () {
curr_branch=$(git symbolic-ref -q HEAD)
curr_branch="${curr_branch#refs/heads/}"
origin=$(git config --get "branch.$curr_branch.remote")
echo ${origin:-origin}
}
> 2. For an obscure reason, changing the url in .git/config to
> /central/sub and call git submodule update still make git want to
> clone from /user/sub, and fails. There seems to be no way to tell
> git the right URL for this submodule, while it should be possible
> according to the submodule documentation.
This is very surprising to me. With Git v1.9.1:
* Clone just the superproject, without it's sibling submodule projects:
$ git clone git://github.com/wking/pygrader.git pg-1
* Clone the isolated superproject, so we'll have broken relative URLs:
$ git clone pg-1 pg-2
* Initialize a submodule:
$ git submodule init dep/src/pyassuan
* Fix the broken, expanded-from-relative URL to point back to the
original:
$ git config submodule.dep/src/pyassuan.url
git://github.com/wking/pyassuan.git
* Initial, cloning update:
$ git submodule update
That all works as expected for me.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [git] Re: [RFC PATCH] clone: add clone.recursesubmodules config option
On Mon, Jun 09, 2014 at 03:17:07PM +0200, Jens Lehmann wrote: > And by the way: wouldn't it make more sense to tell the user /what/ > we do automatically? So maybe 'submodule.autoupdate' is a better > name for the new switch? Or autocheckout? No need to preserve submodule-specific jargon when we have a perfectly acceptable word for this in the core interface ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Submodules with feature branches
On Thu, Jun 05, 2014 at 12:00:33PM -0700, W. Trevor King wrote: > On Thu, Jun 05, 2014 at 01:31:39PM -0500, Robert Dailey wrote: > > Instead of just creating my branch and starting to make commits, I > > now have to setup my submodule branch first. Also pull requests > > won't show the changes to the third party libraries unless I do a > > second pull request for the third party repo. > > That I agree with ;). However, if you're treating the third-party > library as a separate repo, I think it makes sense that you need to > be making branches and pull requests in the submodule independently > from your branches and pull requests in the superproject. To make this more concrete, I think you'll rarely have tight one-to-one binding between third-party library changes and your superproject. More likely, you'll have some high-level feature branch in the superproject (“accept comments via email”) and an unrelated number of prerequisite feature branches for your libraries (“add support for MIME documents,” “parse RFC 2822 dates,” …). You only have synchronized branches when you mess with the API tying components together (updating the submodule API and updating the superproject to use it). With good library design, that type of API migration should happen more and more rarely as the library stabilizes. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Submodules with feature branches
On Thu, Jun 05, 2014 at 01:31:39PM -0500, Robert Dailey wrote: > On Thu, Jun 5, 2014 at 11:23 AM, W. Trevor King wrote: > > 3rd party libraries sound loosely-coupled to me ;). In one of my more > > mature projects I did a similar thing, and just used relative URLs [1] > > and sibling mirrors/forks [2,3,4]. > > > > Cheers, > > Trevor > > > > [1]: https://github.com/wking/pygrader/blob/master/.gitmodules > > [2]: https://github.com/wking/pgp-mime > > [3]: https://github.com/wking/pyassuan > > [4]: https://github.com/wking/jinja2 > > I guess I'm still confused on how relative URLs help here. If you want to add a feature to pygrader that needs tweaks to pgp-mime, you can put your public repositories somewhere as siblings, and: $ git clone --recursive git://you.net/pygrader.git will work fine (drawing from git://you.net/pgp-mime.git, etc.). > Won't the capping commits (A and C in your first email) still be > needed? Or is there a way I can modify the local > "../third-party.git" submodule repo instead? Can you explain? Anyone reviewing your changes locally will need a way to get your submodule commits as well as your superproject commits. In both cases, they can use the usual: $ git add remote you git://you.net/….git $ git fetch or other tweaks like GitHub's refs/pull/*/head namespace [1]. Even a shared central repository, if that's how your team rolls. > Unfortunately, the reason why I feel third party in a submodule > creates tight coupling is because: > > * You can't make changes to third party libs for your feature branch > without breaking the trunk You can in a branch. Maybe I'm missing something here. In any case, edits to third party libs are best upstreamed ;). > * Merge conflicts are insane to resolve and involve two clones if > trunk maintainers modify third party binaries and you do as well. You resolve the merge conflicts in the submodule, and then amend the superproject merge commit to point to the resolved submodule commit. That is one --amend away from what you're doing without submodules. > * Feature branching requires those capping / meta commits to simply > setup your branch to be a feature branch. With relative URLs (or shared centralized repository, or a refs/pull/*/head namespace) it's easy to share the commits themselves. Unless you're using 'git submodule update --remote …', you don't need to care where the gitlinked commits live, you just need to get them into the submodule repository somehow. That seems fairly orthogonal to feature branching to me. > Instead of just creating my branch and starting to make commits, I > now have to setup my submodule branch first. Also pull requests > won't show the changes to the third party libraries unless I do a > second pull request for the third party repo. That I agree with ;). However, if you're treating the third-party library as a separate repo, I think it makes sense that you need to be making branches and pull requests in the submodule independently from your branches and pull requests in the superproject. If you feel that the minimal (branch + PR for changes) overhead of managing the projects independently is too high, you're probably better off with the single repository or subtree approach. Cheers, Trevor [1]: https://help.github.com/articles/checking-out-pull-requests-locally -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: [RFC PATCH] clone: add clone.recursesubmodules config option
On Thu, Jun 05, 2014 at 11:18:28AM -0700, Junio C Hamano wrote: > Jens Lehmann writes: > > We had two settings in mind,... > > So what if clone would just do an "git submodule init" for now when > > "submodule.autoinit" is set but "submodule.autoupdate" isn't [?] > > ... and a single "submodule.auto" setting would be what users really want? > > I do not offhand think of a sensible scenario where you want to init > a submodule once but do not want to update it when the superproject > changes. Even if the user uses the mode to detach the submodule > HEAD, i.e. the branches in submodules do not matter and the whole > tree is described by the superproject's commit and gitlinks recorded > in it, the user would want the new objects necessary for the updated > superproject, which means a submodule that is init'ed (whether it is > via "git submodule init" or the submodule.autoinit variable) must be > updated. I agreed that once we have the ability to do so, autoupdating any initialized submodules should be automatic and non-optional. However, making it optional during a transition period while the ability gets fleshed out would make sense too (so checkout-mode folks can opt in before we clobber the local-branch folks ;). Ceers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] [PATCH 2/5] implement submodule config cache for lookup of submodule names
On Thu, Jun 05, 2014 at 08:07:50AM +0200, Heiko Voigt wrote: > +The caller can look up information about submodules by using the > +`submodule_from_path()` or `submodule_from_name()` functions. That's for an already-known submodule. Do we need a way to list submodules (e.g. for 'submodule foreach' style operations) or is the preferred way to do that just walking the tree looking for gitlinks? The cases where .gitmodules would lead you astray (e.g. via sloppy commits after removing a submodule) are: * Listing a submodule that wasn't in the tree anymore. Easy to check for and ignore. * Not listing a submodule that is in the tree. You'd need to walk the tree to check for this, but it's a pretty broken situation already, so I'd be fine just ignoring the orphaned gitlink. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Submodules with feature branches
On Thu, Jun 05, 2014 at 10:57:17AM -0500, Robert Dailey wrote: > I was planning on creating a submodule for our third party libs and > store them extracted in there. 3rd party libraries sound loosely-coupled to me ;). In one of my more mature projects I did a similar thing, and just used relative URLs [1] and sibling mirrors/forks [2,3,4]. Cheers, Trevor [1]: https://github.com/wking/pygrader/blob/master/.gitmodules [2]: https://github.com/wking/pgp-mime [3]: https://github.com/wking/pyassuan [4]: https://github.com/wking/jinja2 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Submodules with feature branches
On Thu, Jun 05, 2014 at 09:03:25AM -0500, Robert Dailey wrote: > When I work on a feature, I normally create a feature branch. If I > happen to make changes to the submodule that only work with the > changes introduced in my feature branch, that seems to complicate > things. For the purposes of the feature branch, do I need to create > a corresponding feature branch in the submodule and temporarily > update the submodule URL to point to it? When I merge my feature > branch, I'd have to swap it back? So you have: On the trunk host: On your public host: Locally: superproject superproject superproject submodulesubmodule `-- submodule In that case, a corresponding feature branch to the submodule, and an update to submodule..url (and possibly submodule..branch) would be the way I'd go (at A in the figure below). Once the trunk maintainers were happy with things, they could merge the submodule branch into trunk's submodule (at B in the figure below), and you could add a capping commit to your superproject branch that reverted the gitmodule changes (at C in the figure below): -o---o---o---o---o trunk's superproject/master \ / A---o---o---o---Cyour superproject/feature -o---o---B trunk's submodule/master \ / o---o---oyour submodule/feature An alternative is to use relative URLs in the trunk: superproject$ cat .gitmodules [submodule "bpl-subset"] path = submod url = ../submodule which makes it easier for folks who mirror/fork both the superproject and submodule (no need to change submodule..url). However, it makes it harder for folks who just mirror/fork the superproject (and don't need to tweak the submodule), because they have to mirror/fork the submodule as well to support the relative URL (or edit submodule..url, which turns attempted mirrors into forks). Personally, I prefer relative URLs [1,2], but both external projects I've approached on this front have ended up with absolute URLs [3,4] ;). This is less of an issue for loosely-coupled submodules, since you'll can motivate your submodule changes to the submodule maintainers independent of the superproject (i.e. you can just say things like “I'm extending the API so I can iterate over widgets. This lets you do things like frobbling whatsits in superproject” without having to present the associated superproject code). Once you land the submodule changes upstream, your superproject branch will work without the need to tweak the URL (for absolute URLs) or publish a sibling mirror (for relative URLs). Cheers, Trevor [1]: https://github.com/inducer/pycuda/pull/21 [2]: http://thread.gmane.org/gmane.comp.python.ipython.devel/10287/focus=10299 [3]: https://github.com/wking/pycuda/commit/5218bd449d6aae0bce3a3d1bf54a91377445e2f9 [4]: https://github.com/minrk/ipython/commit/4fe230e96e357b3612b6fadaeec9d8de71d6fca9 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] Documentation: mention config sources for @{upstream}
The earlier documentation made vague references to "is set to build
on". Flesh that out with references to the config settings, so folks
can use git-config(1) to get more detail on what @{upstream} means.
For example, @{upstream} does not care about remote.pushdefault or
branch..pushremote.
Signed-off-by: W. Trevor King
---
Documentation/revisions.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 5a286d0..0796118 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -94,7 +94,9 @@ some output processing may assume ref names in UTF-8.
'@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}'::
The suffix '@\{upstream\}' to a branchname (short form '@\{u\}')
refers to the branch that the branch specified by branchname is set to build
on
- top of. A missing branchname defaults to the current one.
+ top of (configured with `branch..remote` and
+ `branch..merge`). A missing branchname defaults to the
+ current one.
'{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::
A suffix '{caret}' to a revision parameter means the first parent of
--
1.9.1.353.gc66d89d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Conforming to pep8
On Fri, May 09, 2014 at 02:44:02AM -0500, William Giokas wrote: > Maybe a time to use something like:: > > from mercurial import foo \ > bar \ > baz \ > ... > > Would make that import into quite a few lines, but would help organize > things and let you easily organize things in the future. From PEP 8 [1]: The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation. So I prefer something like: from mercurial import ( bar, baz, foo, ) The indentation for the closing parenthesis is optional [2]. You can of course do things like: from mercurial import ( bar, baz, foo, ) but I prefer the complete specification of “single, alphebetized entry per line”. I'm happy to send patches if that style is ok. Cheers, Trevor [1]: http://legacy.python.org/dev/peps/pep-0008/#maximum-line-length [2]: http://legacy.python.org/dev/peps/pep-0008/#indentation -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: pull.prompt or other way to slow/disable 'git pull'
On Sat, May 03, 2014 at 04:50:52AM -0500, Felipe Contreras wrote:
> Either way it would be impossible for Git to figre out what you want
> to do.
That's my point. The details of my particular workflow are
unimportant.
> Anyway I don't see how is this possibly relevant to the topic at
> hand.
I'm trying to motivate a way to slow/disable 'git pull', which I see
as orthogonal to your push to change the default configuration. I
thought describing my workflow in more detail would help clarify why…
> W. Trevor King wrote:
> > On Fri, May 02, 2014 at 05:20:11PM -0500, Felipe Contreras wrote:
> > > W. Trevor King wrote:
> > > > > > The 'git pull' (with 'none' mode) explainer just helps retrain folks
> > > > > > that are already using the current 'git pull' incorrectly.
> > > > >
> > > > > If you are going to train them to use a configuration, it should be:
> > > > >
> > > > > % git config --global pull.ff false
> > > >
> > > > I don't want all pulls to be --no-ff, only pulls from topic branches.
… this global pull.ff config was not a solution.
> > > Either way, since I think these two are different modes:
> > >
> > > 1) git pull
> > > 2) git pull origin topic
> > >
> > > Maybe it would actually make sense to have a configuration specific to
> > > 2): pull.topicmode.
> >
> > I think it makes more sense to just use merge/rebase explicitly,
>
> Fine, if you want the user to be explicit, he can be explicit with
> `git pull --no-ff origin topic`. Problem solved.
That's certainly explicit, but some folks are in the habit of just
running 'git pull' (regardless of which branch they happen to be on)
without thinking “Where am I, what am I integrating, and how should I
integrate it?”. As I claimed earlier:
On Thu, May 01, 2014 at 06:10:04PM -0700, W. Trevor King wrote [1]:
> Folks who are setting any ff options don't need any of these
> training wheels. My proposed --prompt behavior is for folks who
> think “I often run this command without thinking it through all the
> way. I'm also not used to reading Git's output and using 'reset
> --hard' with the reflog to reverse changes. Instead of trusting me
> to only say what I mean or leaving me to recover from mistakes,
> please tell me what's about to change and let me opt out if I've
> changed my mind.”
In the messages following that, you seemed to agree that such folks
existed [2], and suggested I use pull.mode=fetch-only [3] or
pull.ff=false [4] or pull.topicargs='--merge --no-ff' [5]. Now we
agree (I think? Based on your “it would be impossible for Git…”
quoted above) that you can have a sane workflow for which no
pull-strategy default will always do the right thing. We just
disagree (I think) on what to do about it. I'm suggesting
pull.prompt, pull.mode=none, or some other way to slow/disable 'git
pull' while folks retrain themselves. You're suggesting (I think?
Based on your 'git pull --no-ff origin topic' quoted above) that folks
just skip right to remembering which ff options to use in which
situations. Do you feel folks won't need a way to slow/disable 'git
pull' while they build the ff options and their project's recommended
workflow into their own practice? Or do you agree that they will need
some kind of helper for the transition, and just feel that git.prompt
is the wrong helper?
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/247917
[2]: http://article.gmane.org/gmane.comp.version-control.git/247919
On Thu, May 01, 2014 at 08:14:29PM -0500, Felipe Contreras wrote:
> W. Trevor King wrote:
> > Folks who are setting any ff options don't need any of these
> > training wheels.
>
> Indeed.
[3]: http://article.gmane.org/gmane.comp.version-control.git/247957
On Fri, May 02, 2014 at 02:13:25PM -0500, Felipe Contreras wrote:
> W. Trevor King wrote:
> > On Fri, May 02, 2014 at 01:55:36PM -0500, Felipe Contreras wrote:
> > > W. Trevor King wrote:
> > > > But once such folks are identified, you just have to
> > > > convince them (once) to set the pull.prompt config.
> > > > That's a lot easier than convincing them (for every pull)
> > > > to set the appropriate ff flag.
> > >
> > > It wouldn't matter if by the default non-fast-forward
> > > merges are rejected.
> >
> > It would matter if you didn't want them making
> > non-fast-forward
Re: pull.prompt or other way to slow/disable 'git pull'
On Fri, May 02, 2014 at 05:20:11PM -0500, Felipe Contreras wrote:
> W. Trevor King wrote:
> > > > The 'git pull' (with 'none' mode) explainer just helps retrain folks
> > > > that are already using the current 'git pull' incorrectly.
> > >
> > > If you are going to train them to use a configuration, it should be:
> > >
> > > % git config --global pull.ff false
> >
> > I don't want all pulls to be --no-ff, only pulls from topic branches.
>
> Pulling some branch to a topic branch, or pulling a topic branch to
> another branch?
The latter. Here's a more detailed list:
1. HEAD: an integration branch (master, maint, …)
target: @{upstream}, branch.*.pushremote, and other mirrors
my preferred integration mode: ff-only merge the target
2. HEAD: an integration branch
target: a *different* branch (e.g. maint or feature-x, but not
origin/master or jdoe/master, if HEAD is master)
my preferred integration mode: no-ff merge the target into HEAD.
3. HEAD: a topic branch (e.g. feature-x)
target: a collaborating topic branch (jdoe/feature-x)
my preferred integration mode: ff-only merge the target
4. HEAD: a topic branch (e.g. feature-x)
target: a related topic branch (e.g. jdoe/feature-y) or integration
branch updates used by my feature-x
my preferred integration mode: rebase feature-x onto the target
Cases 1 and 2 can usually be distinguished by comparing the
checked-out branch with the branch portion of the remote-tracking
reference), but for folks developing in master, jdoe/master may be a
feature branch (case 2) not a mirror of the maintenance branch (case
1).
Cases 1 and 3 are the same idea, with any feature branch running long
enough to get collaborators being indistinguishable from an
integration branch except that the latter will eventually be merged
(or dropped) and deleted.
In the event of non-trivial merge conflicts in case 2, I sometimes
rebase the target onto HEAD and no-ff merge the resulting target'. On
the other hand, sometimes rebasing is not an option. For example, if
I want to merge the target into both master and maint, but master
contains a conflicting commit A:
-o---o---A---o---B master
|\
| o---o---C maint
\
o---D target
Rebasing would drag A into maint at F:
-o---o---A---o---B---E master
\ \ /
\ o---D'--- target'
\ \
o---o---C---F maint
And I don't want both the pre- and post-rebase versions in my history
at G:
-o---o---A---o---B---E---G master
|\ \ / /
| \ o---D'--- / target'
| \ /
| o---o---C---F maint
\ /
o---D target
So I'd just deal with a complicated merge at E:
-o---o---A---o---B---E---G master
|\ / /
| o---D / target
\ \ /
o---o---C---F-- maint
Case 4 has similar caveats, since you don't want to rebase feature-x
on top of jdoe/feature-y if there are already other branches based on
the current feature-x that can't (or won't) be rebased.
> Either way, since I think these two are different modes:
>
> 1) git pull
> 2) git pull origin topic
>
> Maybe it would actually make sense to have a configuration specific to
> 2): pull.topicmode.
I think it makes more sense to just use merge/rebase explicitly, and
not try and bundle all of this complication into something that *also*
fetches. Unfortunately, there's currently no finger-breaker to help
compulsive pull users break the habit or keep novices from starting.
Adding more elaborate handling to pull just pushes back the point
where you reach something that is pretty much impossible to resolve
automatically (like my case 2 caveat). When that happens, it would be
nice to have a workflow independent way to calm the pull-happy user
(e.g. pull.mode=none, or pull.prompt=true) while they learn to
explicitly use fetch/{merge|rebase} or more careful pulls.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
pull.prompt or other way to slow/disable 'git pull' (was: Pull is Evil)
On Fri, May 02, 2014 at 04:18:57PM -0500, Felipe Contreras wrote: > W. Trevor King wrote: > > On Fri, May 02, 2014 at 03:34:34PM -0500, Felipe Contreras wrote: > > > W. Trevor King wrote: > > > > On Fri, May 02, 2014 at 02:13:25PM -0500, Felipe Contreras wrote: > > > > > It would matter almost exactly zero. > > > > > > > > Some folks have explicit merge policies, and deciding how much > > > > that matters is probably best left up to the projects themselves > > > > and not decided in Git code. > > > > > > Let's make some fake numbers to see around how much this would matter. > > > > The point isn't that this is a huge flaw, the point is that we should > > be able to configure Git to match sane workflows. > > The point is that we are tainting a discussion about how to improve the > defaults for the vast majority of users I've renamed this sub-thread (which started around $gmane/247835) to avoid potential confusion/dilution. > > The goal is to train them to do: > > > > > % git config --global pull.mode none > > > % git fetch > > > % git merge --no-ff Sticking to my 'no-ff' topic branch example, this should have been: git merge --no-ff remote branch I want folks to use --ff-only when pulling their default upstream. > > The 'git pull' (with 'none' mode) explainer just helps retrain folks > > that are already using the current 'git pull' incorrectly. > > If you are going to train them to use a configuration, it should be: > > % git config --global pull.ff false I don't want all pulls to be --no-ff, only pulls from topic branches. I think adding a prompt or making the integration a two-step fetch/merge are both ways to jog a user into consciously evaluating their actions. I don't see how a changing the default single-step pull strategy (whatever it is) will. I also don't look forward to explaining an adaptive strategy that tries to get my workflow right without command-line ff options to folks on their first day using Git. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Pull is Evil
On Fri, May 02, 2014 at 03:34:34PM -0500, Felipe Contreras wrote:
> W. Trevor King wrote:
> > On Fri, May 02, 2014 at 02:13:25PM -0500, Felipe Contreras wrote:
> > > It would matter almost exactly zero.
> >
> > Some folks have explicit merge policies, and deciding how much
> > that matters is probably best left up to the projects themselves
> > and not decided in Git code.
>
> Let's make some fake numbers to see around how much this would matter.
The point isn't that this is a huge flaw, the point is that we should
be able to configure Git to match sane workflows. Saying “that's
unlikely to happen” doesn't solve the problem that some newcomers have
trouble matching their project's desired workflow.
> So no, for all intents and purposes it doesn't matter. I would rather
> concentrate on the issue more than 90% of the users face.
You don't have to concentrate on it, because I'm willing to write up
the patch, I'm just trying to find a consensus spec before writing the
patch. If you don't have strong feelings about a pull.prompt
proposal, I won't mind ;). I just don't want to write it up and
*then* hear “that's a terrible idea, you should have just done $x.”.
> > > And just as they can do pull.promot = true, they can do pull.mode =
> > > fetch-only.
> >
> > Why would you run a fetch-only pull instead of running 'git fetch'? I
> > think it would make more sense to have 'pull.mode = none' with which
> > 'git pull …' turns into a no-op suggesting an explicit
> > fetch/{merge|rebase}. Having something like that available would
> > help with the training issue that pull.prompt was addressing.
>
> I fail to see how training them to do this:
>
> % git config --global pull.mode none
> % git pull
> % git fetch
> % git merge --no-ff
>
> Is preferable than training them to do:
>
> % git pull --no-ff
The goal is to train them to do:
> % git config --global pull.mode none
> % git fetch
> % git merge --no-ff
The 'git pull' (with 'none' mode) explainer just helps retrain folks
that are already using the current 'git pull' incorrectly.
The benefit is that the repeated pair of commands (fetch/merge) takes
longer to type, which gives them longer to realize that they should
think about what they're doing and abort. That's all a pull.prompt
would be doing anyway.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: Pull is Evil
On Fri, May 02, 2014 at 02:13:25PM -0500, Felipe Contreras wrote:
> W. Trevor King wrote [1]:
> > On Fri, May 02, 2014 at 01:55:36PM -0500, Felipe Contreras wrote:
> > > W. Trevor King wrote:
> > > > On Thu, May 01, 2014 at 08:14:29PM -0500, Felipe Contreras wrote:
> > > > > W. Trevor King wrote:
> > > > > > My proposed --prompt behavior is for folks who think “I often run
> > > > > > this command without thinking it through all the way. I'm also
> > > > > > not used to reading Git's output and using 'reset --hard' with the
> > > > > > reflog to reverse changes. Instead of trusting me to only say
> > > > > > what I mean or leaving me to recover from mistakes, please tell me
> > > > > > what's about to change and let me opt out if I've changed my
> > > > > > mind.”
> > > > >
> > > > > Unfortunately those folks by definition wouldn't know about the
> > > > > --prompt option.
> > > >
> > > > But once such folks are identified, you just have to convince them
> > > > (once) to set the pull.prompt config. That's a lot easier than
> > > > convincing them (for every pull) to set the appropriate ff flag.
> > >
> > > It wouldn't matter if by the default non-fast-forward merges are
> > > rejected.
> >
> > It would matter if you [only wanted] them making non-fast-forward
> > merges (e.g. for explicitly-merged topic branches).
>
> It would matter almost exactly zero.
Some folks have explicit merge policies, and deciding how much that
matters is probably best left up to the projects themselves and not
decided in Git code. I like having a place to explain why a feature
is useful and has been included in projects I maintain.
> And just as they can do pull.promot = true, they can do pull.mode =
> fetch-only.
Why would you run a fetch-only pull instead of running 'git fetch'? I
think it would make more sense to have 'pull.mode = none' with which
'git pull …' turns into a no-op suggesting an explicit
fetch/{merge|rebase}. Having something like that available would
help with the training issue that pull.prompt was addressing.
Cheers,
Trevor
[1]: With David Kastrup's "only wanted" typo fix.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: Pull is Evil
On Fri, May 02, 2014 at 01:55:36PM -0500, Felipe Contreras wrote: > W. Trevor King wrote: > > On Thu, May 01, 2014 at 08:14:29PM -0500, Felipe Contreras wrote: > > > W. Trevor King wrote: > > > > My proposed --prompt behavior is for folks who think “I often run > > > > this command without thinking it through all the way. I'm also > > > > not used to reading Git's output and using 'reset --hard' with the > > > > reflog to reverse changes. Instead of trusting me to only say > > > > what I mean or leaving me to recover from mistakes, please tell me > > > > what's about to change and let me opt out if I've changed my > > > > mind.” > > > > > > Unfortunately those folks by definition wouldn't know about the > > > --prompt option. > > > > But once such folks are identified, you just have to convince them > > (once) to set the pull.prompt config. That's a lot easier than > > convincing them (for every pull) to set the appropriate ff flag. > > It wouldn't matter if by the default non-fast-forward merges are > rejected. It would matter if you didn't want them making non-fast-forward merges (e.g. for explicitly-merged topic branches). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v6 5/7] pull: add merge-ff-only option
On Thu, May 01, 2014 at 07:00:06PM -0500, Felipe Contreras wrote: > It is very typical for Git newcomers to inadvertently create merges and > worst; inadvertently pushing them. This is one of the reasons many > experienced users prefer to avoid 'git pull', and recommend newcomers to > avoid it as well. > > To avoid these problems and keep 'git pull' useful, it has been > suggested that 'git pull' barfs by default if the merge is > non-fast-forward, which unfortunately would break backwards > compatibility. > > This patch leaves everything in place to enable this new mode, but it > only gets enabled if the user specifically configures it; pull.mode = > merge-ff-only. The subject and commit message also need “merge-ff-only” → “ff-only”. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Pull is Evil
On Thu, May 01, 2014 at 08:14:29PM -0500, Felipe Contreras wrote: > W. Trevor King wrote: > > My proposed --prompt behavior is for folks who think “I often run > > this command without thinking it through all the way. I'm also > > not used to reading Git's output and using 'reset --hard' with the > > reflog to reverse changes. Instead of trusting me to only say > > what I mean or leaving me to recover from mistakes, please tell me > > what's about to change and let me opt out if I've changed my > > mind.” > > Unfortunately those folks by definition wouldn't know about the > --prompt option. But once such folks are identified, you just have to convince them (once) to set the pull.prompt config. That's a lot easier than convincing them (for every pull) to set the appropriate ff flag. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v6 1/7] pull: rename pull.rebase to pull.mode
On Thu, May 01, 2014 at 07:00:02PM -0500, Felipe Contreras wrote: > Also 'branch..rebase' to 'branch..pullmode'. Perhaps this has already been hashed out in a previous version of this series, but we may want to use pull.update and branch..update to match the existing submodule..update. Both settings are selecting the default integration style between HEAD and some other reference (pull's remote branch, the gitlinked commit, or the submodule's --remote branch). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Pull is Evil
On Thu, May 01, 2014 at 07:37:04PM -0500, Felipe Contreras wrote: > W. Trevor King wrote: > > On Thu, May 01, 2014 at 06:25:16PM -0500, Felipe Contreras wrote: > > > W. Trevor King wrote: > > > > Fast-forward $current_branch by $count commits to $repository > > > > $refpec? > > > > > > Why would anyone say 'no' to this one? > > > > Because the want explicit merges when they bring in topic > > branches? > > If that was the case the user wouls have run `git merge > --no-ff`. Only expereinced users would answer 'no'. Folks who are setting any ff options don't need any of these training wheels. My proposed --prompt behavior is for folks who think “I often run this command without thinking it through all the way. I'm also not used to reading Git's output and using 'reset --hard' with the reflog to reverse changes. Instead of trusting me to only say what I mean or leaving me to recover from mistakes, please tell me what's about to change and let me opt out if I've changed my mind.” > > > > and have a chance to bail out if you saw: > > > > > > > > Merge 1003 commits from git://example.net/main.git master into > > > > my-feature? > > > > > > > > because you forgot which branch you were on. > > > > > > Yes, that might be nice. But we still need to change the defaults. > > > > So I should submit an orthogonal patch with -i/--interative/--prompt? > > I'm not entirely sure what would be the ideal behavior. > > For example, I'm thinking that by default when the a fast-forward is > possible, just do it, … But just because a ff is possible doesn't mean it's what the user/project wants. It may be the most likely guess, but why guess when they've explicitly asked for a prompt? > when it's not, ask if the user wants to do a merge or a rebase, if > the user just press 'enter' a merge is attempted. I'll just mimic however mergetool currently handles prompt accept/decline. > In addition a summary of the commits ahead behind would be helpful. Good idea. > If the user wants to cancel the operation, he can just do CTRL+C. I'll just mimic mergetool. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Pull is Evil
On Thu, May 01, 2014 at 06:25:16PM -0500, Felipe Contreras wrote: > W. Trevor King wrote: > > On Thu, May 01, 2014 at 12:48:46PM -0700, W. Trevor King wrote: > > > My interest in all of the proposed git-pull-training-wheel patches is > > > that they give users a way to set a finger-breaking configuration that > > > makes pull a no-op (or slows it down, like 'rm -i …'). Then folks who > > > compulsively run 'git pull' (e.g. because SVN habits die slowly) can > > > set an option that gives them something to think about before going > > > ahead and running the pull anyway. > > > > Actually, what do we think about an -i/--interactive flag (with an > > associated pull.interactive boolean config to setup global/per-repo > > defaults)? Then after the fetch, you'd get one of the following: > > > > Merge $count commits from $repository $refspec into $current_branch? > > Rebase $count commits from $current_branch onto $repository $refpec? > > Not much interactivity in those options. Maybe --prompt would make more > sense. I think matching rm, mv, cp, etc. is good, but I'd be ok with --prompt. > > Fast-forward $current_branch by $count commits to $repository $refpec? > > Why would anyone say 'no' to this one? Because the want explicit merges when they bring in topic branches? > > and have a chance to bail out if you saw: > > > > Merge 1003 commits from git://example.net/main.git master into my-feature? > > > > because you forgot which branch you were on. > > Yes, that might be nice. But we still need to change the defaults. So I should submit an orthogonal patch with -i/--interative/--prompt? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Pull is Evil
On Thu, May 01, 2014 at 06:34:06PM -0500, Felipe Contreras wrote: > Nobody ever complained about somebody doing a fast-forward by mistake. Unless they fast-forward merged a feature branch into master, but the project prefers explicitly-merged feature branches with a cover-letter explaination in the merge commit [1]. On the one hand, folks integrating feature branches are likely more experienced Git users. On the other hand, I know several project maintainers who integrate feature branches that are pull-happy. I agree that accidental ff-merges are likely to be less troublesome than accidental non-ff merge/rebases, but I don't think changing the default to ff-only is a perfect fix. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/247807 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Pull is Evil
On Thu, May 01, 2014 at 12:48:46PM -0700, W. Trevor King wrote: > My interest in all of the proposed git-pull-training-wheel patches is > that they give users a way to set a finger-breaking configuration that > makes pull a no-op (or slows it down, like 'rm -i …'). Then folks who > compulsively run 'git pull' (e.g. because SVN habits die slowly) can > set an option that gives them something to think about before going > ahead and running the pull anyway. Actually, what do we think about an -i/--interactive flag (with an associated pull.interactive boolean config to setup global/per-repo defaults)? Then after the fetch, you'd get one of the following: Merge $count commits from $repository $refspec into $current_branch? Rebase $count commits from $current_branch onto $repository $refpec? Fast-forward $current_branch by $count commits to $repository $refpec? and have a chance to bail out if you saw: Merge 1003 commits from git://example.net/main.git master into my-feature? because you forgot which branch you were on. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Pull is Evil
On Thu, May 01, 2014 at 02:04:33PM -0400, Marc Branchaud wrote: > On 14-05-01 01:56 PM, W. Trevor King wrote: > > On Thu, May 01, 2014 at 11:20:44AM -0400, Marc Branchaud wrote: > >> On 14-05-01 05:46 AM, brian m. carlson wrote: > >>> git checkout maintenance-branch > >>> # Update our maintenance branch to the latest from the main repo. > >>> git pull --ff-only > >>> git pull --no-ff developer-remote topic-branch > >>> git push main-repo HEAD > >> > >> … > >> What's more, it seems to me that the only real advantage "git > >> pull" provides here is a less typing compared to the non-pull > >> equivalent: > >> > >> git fetch main-repo > >> git checkout main-repo/maintenance-branch > >> git fetch developer-remote > >> git merge --no-ff developer-remote/topic-branch > >> git push main-repo HEAD > > > > You're missing Brian's fast-forward merge here. It should be: > > > > git checkout maintenance-branch > > git fetch main-repo > > git merge --ff-only main-repo/maintenance-branch > > git fetch developer-remote > > … > > I think you're mistaken -- I checked out > "main-repo/maintenance-branch" directly, so there's no need to > fast-forward a local branch. I find a local branch useful to mark the amount of the upstream branch that I've reviewed. The reflog helps a bit, but I may go several fetches between reviews. For newbies, I recommend avoiding detached HEADs, where possible, so they don't have to rely on the reflog if they accidentally commit and then checkout something else (ignoring Git's warning). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Pull is Evil
On Thu, May 01, 2014 at 02:16:50PM -0500, Felipe Contreras wrote: > The only problem would be when it's not desirable, however, that's a > problem of the user's ignorance, and the failure of the project's > policity to communicate clearly to him that he should be running > `git merge --no-ff`. There's absolutely nothing we can do to help him. I think “user ignorange” is the *only* problem with git pull. Once you understand the ff flags, you can set them however you like, and pull will do what you tell it to. > The only thing we could do is not allow fast-forward merges either, in > which case `git pull` becomes a no-op that can't possibly do anything > ever. My interest in all of the proposed git-pull-training-wheel patches is that they give users a way to set a finger-breaking configuration that makes pull a no-op (or slows it down, like 'rm -i …'). Then folks who compulsively run 'git pull' (e.g. because SVN habits die slowly) can set an option that gives them something to think about before going ahead and running the pull anyway. The space in 'git pull' makes a shell-side: $ alias 'git pull'='echo "try fetch/merge!"' solution unfeasible, and clobbering /usr/libexec/git-core/git-pull seems a bit extreme. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Pull is Evil
On Thu, May 01, 2014 at 11:20:44AM -0400, Marc Branchaud wrote: > On 14-05-01 05:46 AM, brian m. carlson wrote: > > git checkout maintenance-branch > > # Update our maintenance branch to the latest from the main repo. > > git pull --ff-only > > git pull --no-ff developer-remote topic-branch > > git push main-repo HEAD > > … > What's more, it seems to me that the only real advantage "git pull" provides > here is a less typing compared to the non-pull equivalent: > > git fetch main-repo > git checkout main-repo/maintenance-branch > git fetch developer-remote > git merge --no-ff developer-remote/topic-branch > git push main-repo HEAD You're missing Brian's fast-forward merge here. It should be: git checkout maintenance-branch git fetch main-repo git merge --ff-only main-repo/maintenance-branch git fetch developer-remote … > Sure, the non-pull approach makes use of Scary Branch Stuff (remotes > and namespaces and detached HEADs -- oh my!). No need for detached heads with Brian's local maintenance-branch. If you're teaching and just need folks merging the remote's HEAD, you can avoid namespaces and remotes entirely: git fetch git://example.net/main-repo.git git merge --ff-only FETCH_HEAD although I doubt “the remote's HEAD” will be easier to explain than the namespaced, remote-tracking branches it replaces. It's certainly not worth the hassle of un-training FETCH_HEAD-merges later on ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] tag: add -i and --introduced modifier for --contains
On Mon, Apr 21, 2014 at 05:38:34PM -0700, Luis R. Rodriguez wrote:
> [0] mcgrof@ergon ~/linux (git::master)$ git log c5905afb..v3.5| grep
> ^commit | wc -l
> 24878
> [1] mcgrof@ergon ~/linux (git::master)$ git log c5905afb..v3.4| grep
> ^commit | wc -l
> 13106
> [2] mcgrof@ergon ~/linux (git::master)$ git log c5905afb..v3.3| grep
> ^commit | wc -l
> 1360
From gitrevisions(7), r1..r2 is “commits that are reachable from r2
excluding those that are reachable from r1”. Using Peff's example:
On Thu, Apr 17, 2014 at 06:16:20PM -0400, Jeff King wrote:
> ---A---B---C-D---E---F (maint, v3.4)
> \ \ /
> \ ---G-H---I (master, v4.0)
>\ / /
> --J---
>
> The fix is J, and it got merged up to maint at D, and to master at H.
> v4.0 does not contain v3.4. What's the best description of J?
J..v3.4 is going to include B, C, D, E and F. However, the “distance”
used by ‘git describe’ uses the shortest path between the commits
(J-D-E-F), which doesn't care about development between A and D.
> The results for command [2] above however a bit surprising, I'd take a
> look but I should go back to look at other stuff, figured I'd at least
> bring it up now as it seems relevant.
Here's a simplified graph with d1-* tags for the v3.5-rc1~120^3~76^2
description and d2-* tags for the v3.4~479^2~9^2 description [1]:
* f8f5701 (tag: v3.5-rc1) Linux 3.5-rc1
* 912afc3 (tag: d1-F) Merge tag 'dm-3.5-changes-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/agk/linux-dm
* 56edab3 (tag: d1-E) Merge branches 'perf-urgent-for-linus' and
'perf-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
|\
| * ab0cce5 (tag: d1-D) Revert "sched, perf: Use a single callback into the
scheduler"
| * 26252ea (tag: d1-C-1, tag: d1-C) perf evlist: Show event attribute details
| * a385ec4 (tag: d1-C-64) Merge tag 'v3.4-rc2' into perf/core
| |\
| * \ 659c36f (tag: d1-C-65) Merge tag 'perf-core-for-mingo' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
| |\ \
| | * | 5a7ed29 (tag: d1-C-65-2) perf record: Use sw counter only if hw pmu
is not detected
* | |/ 76e10d1 (tag: v3.4) Linux 3.4
| |/|
|/| |
* |/ dd775ae (tag: v3.4-rc1) Linux 3.4-rc1
|/|
* | c5bc437 Merge tag 'perf-core-for-mingo' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent
|\|
| * 9521d83 (tag: d1-C-66) Merge tag 'perf-core-for-mingo' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
* | 9c2b957 (tag: d2-E) Merge branch 'perf-core-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
|\ \
| |/
| * bea95c1 (tag: d2-D, tag: d1-C-67) Merge branch 'perf/hw-branch-sampling'
into perf/core
| * f9b4eeb (tag: d2-C, tag: d1-C-68) perf/x86: Prettify pmu config literals
| * a706d4f (tag: d2-B, tag: d1-C-76, tag: d1-B) Merge branch
'perf/jump-labels' into perf/core
| * c5905af (tag: A) static keys: Introduce 'struct static_key',
static_key_true()/false() and static_key_slow_[inc|dec]()
* | c16fa4f (tag: v3.3) Linux 3.3
|/
* dcd6c92 (tag: v3.3-rc1) Linux 3.3-rc1
This shows the v3.4-rc1 bypass from 9521d83 (d1-C-66) to 659c36f
(d1-C-65) which sets up the v3.5-rc1~120^3~76 description. It also
shows the c5905afb..v3.3 commits on the branch from c5905af's fork
(between v3.3-rc1 and v3.3) and v3.3.
Cheers,
Trevor
[1]: The simplified graph is from:
$ git tag A c5905afb
$ git tag d1-B v3.5-rc1~120^3~76
$ git tag d1-C v3.5-rc1~120^3~1
$ git tag d1-D v3.5-rc1~120^3
$ git tag d1-E v3.5-rc1~120
$ git tag d1-F v3.5-rc1~1
$ for x in $(seq 76); do git tag d1-C-$x v3.5-rc1~120^3~$x; done
$ git tag d1-C-65-2 d1-C-65^2
$ git tag d2-B v3.4~479^2~9
$ git tag d2-C v3.4~479^2~1
$ git tag d2-D v3.4~479^2
$ git tag d2-E v3.4~479
$ git tag -d sound-fixes sound-3.4 v3.3-rc{2,3,4,5,6,7} v3.4-rc{2,3,4,5,6,7}
$ git log --graph --topo-order --oneline --decorate --simplify-by-decoration
v3.5-rc1
…simplified graph…
$ git tag -d A d1-{B,C,D,E,F} d2-{B,C,D,E} d1-C-65-2
$ for x in $(seq 76); do git tag -d d1-C-$x; done
With some additional tweaks to cull the d1-C-* bits we don't care
about and clear up the 659c36f (d1-C-65) merge.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [git] [RFC/PATCH 2/4] Submodules: Add the lib-submodule-update.sh test library
On Thu, Apr 17, 2014 at 11:08:06PM +0200, Jens Lehmann wrote: > Am 17.04.2014 18:41, schrieb W. Trevor King: > > On Tue, Mar 25, 2014 at 06:05:05PM +0100, Jens Lehmann wrote: > >> *) When a submodule is replaced with a tracked file of the same name > >>the submodule work tree including any local modifications (and > >>even the whole history if it uses a .git directory instead of a > >>gitfile!) is simply removed. > >> … > >> I think the first bug really needs to be fixed, as that behavior is > >> extremely nasty. We should always protect work tree modifications > >> (unless forced) and *never* remove a .git directory (even when > >> forced). > > > > I think this should be covered by the usual “don't allow checkouts > > from dirty workdirs unless the dirty-ing changes are easily applied to > > the target tree”. > > Nope, the target tree will be removed completely and everything in > it is silently nuked. It should be allowed with '-f', but only if > the submodule contains a gitfile, and never if it contains a .git > directory (which is just what we do for rm too). I think it's not covered *now* because of a flaw in our “are dirty-ing changes easily applied to the target tree” detection logic, and the solution should involve updating that logic to hit on this case. > b) recursive checkout is the place to consistently care about > submodule modifications (the submodule script doesn't do that and it > is impossible to change that without causing trouble to a lot of > users. I agree that the submodule script is not the place for this, and the core checkout code is. I'd like checkouts to always be recursive, and see --[no-]recurse-submodules as a finger-breaking stop-gap until we can complete that transition for checkout, bisect, merge, reset, and other work-tree altering commands. I think this is one reason why some folks prefer the stiffer joints you get from a subtree approach. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] What's cooking in git.git (Apr 2014, #03; Fri, 11)
On Fri, Apr 11, 2014 at 03:22:58PM -0700, Junio C Hamano wrote: > * jl/submodule-recursive-checkout (2013-12-26) 5 commits > - Teach checkout to recursively checkout submodules > - submodule: teach unpack_trees() to update submodules > - submodule: teach unpack_trees() to repopulate submodules > - submodule: teach unpack_trees() to remove submodule contents > - submodule: prepare for recursive checkout of submodules > > Expecting a reroll. I think this was rerolled with Jens' v2 [1]: * jl/submodule-recursive-checkout (2014-02-03) 9 commits - submodule: prepare for recursive checkout of submodules - Teach reset the --[no-]recurse-submodules option - Teach checkout the --[no-]recurse-submodules option - Teach merge the --[no-]recurse-submodules option - Teach bisect--helper the --[no-]recurse-submodules option - Teach bisect the --[no-]recurse-submodules option - submodule: teach unpack_trees() to remove submodule contents - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to update submodules Cheers, Trevor [1]: http://thread.gmane.org/gmane.comp.version-control.git/241455 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] [RFC/PATCH 2/4] Submodules: Add the lib-submodule-update.sh test library
On Tue, Mar 25, 2014 at 06:05:05PM +0100, Jens Lehmann wrote: > *) When a submodule is replaced with a tracked file of the same name >the submodule work tree including any local modifications (and >even the whole history if it uses a .git directory instead of a >gitfile!) is simply removed. > … > I think the first bug really needs to be fixed, as that behavior is > extremely nasty. We should always protect work tree modifications > (unless forced) and *never* remove a .git directory (even when > forced). I think this should be covered by the usual “don't allow checkouts from dirty workdirs unless the dirty-ing changes are easily applied to the target tree”. Are we waiting to land this series (or a successor) before starting on a fix for this issue? There have been a number of submodule series in flight recently, and I'm having trouble keeping track of them all ;). > *) Forced work tree updates happily manipulate files in the >directory of a submodule that has just been removed in the >superproject (but is of course still present in the work tree due >to the way submodules are currently handled). This becomes >dangerous when files in the submodule directory are overwritten >by files from the new superproject commit, as any modifications >to the submodule files will be lost) and is expected to also >destroy history in the - admittedly unlikely case - the new >commit adds a file named ".git" to the submodule directory. > … > I'm not so sure about the second one. Even though I believe the > current behavior is not correct (switching commits should never mess > around in a submodule directory) This should also be covered by the usual “don't allow checkouts from dirty workdirs unless the dirty-ing changes are easily applied to the target tree”. We don't implement this yet, but I'd like to force users to move any about-to-be-clobbered state from their submodule into .git/modules// (via commits or stashes) before allowing them to begin the checkout. Once we've ensured that the state is preserved out-of-tree, then clobber away ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC] t7410: 210 tests for various 'git submodule update' scenarios
On Thu, Apr 17, 2014 at 01:42:42PM +0200, Johan Herland wrote:
> >> +# T2: Test with submodule..url != submodule's remote.origin.url.
> >> Does
> >> +# "submodule update --remote" sync with submodule..url, or with
> >> the
> >> +# submodule's origin? (or with the submodule's current branch's
> >> upstream)?
> >
> > All fetches should currently use the submodule's remote.origin.url.
> > submodule..url is only used for the initial clone (*.*.*.1), and
> > never referenced again. This would change using my tightly-bound
> > submodule proposal [1], where a difference between
> > submodule..url and the submodule's @{upstream} URL would be
> > trigger a dirty-tree condition (for folks with tight-bind syncing
> > enabled) from which you couldn't update before resolving the
> > difference.
>
> Ok. As stated above, I am worried about the amount of duplicated
> state between the superproject's submodule config (which itself is
> split between .gitmodules and .git/config) and the submodule's own
> config. And from the above paragraph, I suspect two more dimensions
> need to be added to the test matrix:
>
> - submodule's remote.origin.url ==/!= submodule..url
>
> - "tightly-bound submodule" is enabled/disabled
Tight-binding hasn't been implemented yet, or even accumulated much
support from other folks ;). However, the idea is to unify the state
between the superproject's .gitmodules and .git/config and the
submodule's .git/config (or ../.git/modules//config, or
whatever). Then folks with tightly-bound syncing enabled have only
one state space to maintain (and get auto-updates for each
superproject checkout), and folks who opt-out of tightly-bound syncing
are presumably embracing the complexity of our current system, with
it's two, confusingly-aligned configuration spaces.
I'm happy to force syncing (i.e. no opting-out allowed) [1], but I
imagine there are folks who would resist ;). Maybe a deprecation
period to help ease the transition? This is all assuming that I get
more folks to buy into the tight-syncing ;).
The end-goal of my tightly-bound approach is to remove 'submodule
update' altogether and end up with a simpler interface [2]:
On Sat, Jan 11, 2014 at 05:08:47PM -0800, W. Trevor King wrote:
> * git submodule [--quiet] add [-b ] [-f|--force] [--name ]
> [--reference ] [--] []
> * git submodule [--quiet] init [--] [...]
> * git submodule [--quiet] deinit [-f|--force] [--] ...
> * git submodule [--quiet] foreach [--recursive]
All of this 'submodule update' integration confusion would be resolved
by the developer who updated the gitlink, and superproject checkouts
would just swap the local submodule branch/HEAD without having to
worry about clobbering uncommitted state.
On Thu, Apr 17, 2014 at 01:42:42PM +0200, Johan Herland wrote:
> We should instead seek ways to minimize the duplication of state.
The tightly-bound-submodules I'm proposing try to use the submodule's
config settings (plus submodule..local-branch) as the familiar
language, while your proposal uses Git commands as the familiar
language. I think both would work, but config settings are easier to
parse automatically, which helps with automatically syncing between
the superproject and submodule configs. Syncing, in turn, helps
bridge the gap between the easily shared superproject/.gitmodules and
superproject/.git/modules//config (enabling familiar-to-use Git
commands in the submodule).
> - submodule..create: …
Syncing submodule state back up into this is going to be a manual
operation. For example, changing the submodule's remote.origin.url is
going to require hand-tweaking to update this setting.
> - submodule..update: …
> …
> - 'git reset --hard $GITLINK'
> Equivalent to checkout-mode (without --remote).
>
> - 'git fetch && git reset --hard origin/HEAD'
> Equivalent to checkout-mode with --remote.
Folks who sometimes use --remote updates will still need non-remote
updates. For example, if Alice and Bob are both developers on the
same superproject:
alice$ git submodule update --recursive --remote # integrate upstream changes
alice$ git commit -m 'Bumped submodules to upstream tips'
alice$ git push
bob$ git pull
bob$ git submodule update --recursive # integrate Alice's gitlink changes
so it should be easy to toggle back and forth between the two
integration targets. However:
git fetch && git reset --hard origin/HEAD
is easy to run using 'git submodule foreach', or after changing into
the submodule directory, so I'm not particularly concerned here.
With tight-binding and superproje
Re: [RFC] t7410: 210 tests for various 'git submodule update' scenarios
On Wed, Apr 16, 2014 at 02:54:48AM +0200, Johan Herland wrote:
> This is a work-in-progress to flesh out (and promote discussion about)
> the expected behaviors for all possible scenarios in which
> 'git submodule update' might be run.
This is lovely :).
> +# - current state of submodule:
> +# ?.?.?.1 - not yet cloned
> +# ?.?.?.2 - cloned, detached, HEAD == gitlink
> +# ?.?.?.3 - cloned, detached, HEAD != gitlink
> +# ?.?.?.4 - cloned, on branch foo (exists upstream), HEAD == gitlink
> +# ?.?.?.5 - cloned, on branch foo (exists upstream), HEAD != gitlink
> +# ?.?.?.6 - cloned, on branch bar (MISSING upstream), HEAD == gitlink
> +# ?.?.?.7 - cloned, on branch bar (MISSING upstream), HEAD != gitlink
The remote branches should only matter for the initial clone and
--remote updates. Also, only the configured submodule..branch
(your first axis) should be checked; the locally checked-out submodule
branch shouldn't matter.
> +# T2: Test with submodule..url != submodule's remote.origin.url. Does
> +# "submodule update --remote" sync with submodule..url, or with the
> +# submodule's origin? (or with the submodule's current branch's
> upstream)?
All fetches should currently use the submodule's remote.origin.url.
submodule..url is only used for the initial clone (*.*.*.1), and
never referenced again. This would change using my tightly-bound
submodule proposal [1], where a difference between
submodule..url and the submodule's @{upstream} URL would be
trigger a dirty-tree condition (for folks with tight-bind syncing
enabled) from which you couldn't update before resolving the
difference.
> +# D1: When submodule is already at right commit, checkout-mode currently does
> +# nothing. Should it instead detach, even when no update is needed?
> +# Affects: 1.2.1.4, 1.2.1.6, 2.2.1.4, 2.2.1.6, 3.2.1.4, 3.2.1.6
“Checkout updates always leave a detached HEAD” seems easier to
explain, so I'm leaning that way.
> +# D2: Should all/some of 1.3.*/1.4.* abort/error because we don't know what
> to
> +# merge/rebase with (because .branch is unset)? Or is the current default
> +# to origin/HEAD OK?
> +# Affects: 1.3.*, 1.4.*
Maybe you mean 1.3.*, 1.4.*, and 1.5.* (merge, rebase, and !command)?
In all of these cases, we're integrating the current HEAD with the
gitlinked (*.*.1.*) or remote-tracking branch (*.*.2.*). Since
submodule..branch defaults to master (and may be changed to HEAD
after a long transition period? [2,3]), I don't think we should
abort/error in those cases.
> +# D3: When submodule is already at right commit, merge/rebase-mode currently
> +# does nothing. Should it do something else (e.g. not leave submodule
> +# detached, or checked out on the "wrong" branch (i.e. != .branch))?
> +# (This discussion point is related to D1, D5 and D6)
“Non-checkout updates always leave you on a branch” seems easier to
explain, but I think we'd want to distinguish between the local branch
and the remote submodule..branch [1]. Lacking that distinction,
I'd prefer to leave the checked-out branch unchanged.
> +# D4: When 'submodule update' performs a clone to populate a submodule, it
> +# currently also creates a default branch (named after origin/HEAD) in
> +# that submodule, EVEN WHEN THAT BRANCH WILL NEVER BE USED (e.g. because
> +# we're in checkout-mode and submodule will always be detached). Is this
> +# good, or should the clone performed by 'submodule update' skip the
> +# automatic local branch creation?
> +# Affects: 1.2.*.1, 1.3.*.1, 1.4.*.1, 1.5.*.1,
> +# 2.2.*.1, 2.3.*.1, 2.4.*.1, 2.5.*.1,
> +# 3.2.1.1, 3.3.1.1, 3.4.1.1, 3.5.1.1
“Checkout updates always leave a detached HEAD” seems easier to
explain, so I'm leaning that way.
> +# D5: When in merge/rebase-mode, and 'submodule update' actually ends up
> doing
> +# a merge/rebase, that will happen on the current branch (or detached
> HEAD)
> +# and NOT on the configured (or default) .branch. Is this OK? Should we
> +# abort (or at least warn) instead? (In general, .branch seems only to
> +# affect the submodule's HEAD when the submodule is first cloned.)
> +# (This discussion point is related to D3 and D6)
> +# Affects: 1.3.1.3, 1.3.1.5, 1.3.1.7, 1.3.2.>=2,
> +# 1.4.1.3, 1.4.1.5, 1.4.1.7, 1.4.2.>=2,
> +# 2.3.1.3, 2.3.1.5, 2.3.1.7, 2.3.2.2, 2.3.2.4, 2.3.2.6,
> +# 2.4.1.3, 2.4.1.5, 2.4.1.7, 2.4.2.2, 2.4.2.4, 2.4.2.6
> +# 3.3.1.3, 3.3.1.5, 3.3.1.7
> +# 3.4.1.3, 3.4.1.5, 3.4.1.7
With the --remote option that added submodule..branch (which
eventually landed with v8 of that series [4]), I initially imagined it
as the name of the local branch [5], but transitioned to imagining it
as the name of the remote-tracking branch in v5 of that series [6].
There were no major logical changes between v5 and v8. With the v8
version that landed in Git v1.8.2, submodule..br
Re: [RFC] submodule: change submodule..branch default from master to HEAD
On Mon, Mar 31, 2014 at 09:35:07PM +0200, Jens Lehmann wrote: > Am 28.03.2014 04:36, schrieb W. Trevor King: > > The main drawback to this approach is that we're changing a default, > > but I agree with John's: > > > > On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote: > >> I expect in most cases where "origin/master" happens to be the > >> Right Answer, using the submodule's upstream's HEAD will yield > >> the same result. > > > > so the default-change may not be particularly intrusive. > > I'd prefer a solution that doesn't change any defaults for the > checkout use case (again). Maybe it is a better route to revert > this series, then add tests describing the current behavior for > checkout submodules as a next step before adding the branch mode > for rebase and merge users again? The change in defaults should only adversely effect users who: * don't configure submodule..branch, * use --remote updates, * expect them to pull the master branch of the submodule's upstream, and * have an upstream whose HEAD doesn't point at master. Since 23d25e4 (submodule: explicit local branch creation in module_clone, 2014-01-26), we adversely effects users (regardless of update strategy) who: * don't configure submodule..branch, and * update-clone from a submodule upstream that doesn't have a master branch. But with this patch we'll fix that. Before 23d25e4, we adversly affected users who: * used non-checkout update strategies, and * wanted an attached HEAD after update-clones. All of these were easy to work around, with either: $ git config submodule.$name.branch $branch or: $ cd $submod $ git checkout $branch I'm fine reverting 23d25e4 if we want to kick it around some more, but I have trouble imagining a future UI that works for both: * users that want --remote to pull master even though upstream's HEAD points elsewhere, and * users that want --remote to pull HEAD because upstream doesn't have a master branch, if neither of those users are willing to configure an explicit submodule..branch. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC] submodule: change submodule..branch default from master to HEAD
On Fri, Mar 28, 2014 at 05:57:50PM +0100, Jens Lehmann wrote: > Am 28.03.2014 04:58, schrieb W. Trevor King: > > On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote: > >> No the remote branch is in the upstream subproject. I suppose I meant > >> “the submodule's remote-tracking branch following the upstream > >> subproject's HEAD which we just fetched so it's fairly current” ;). > > > > Hmm, maybe we should change the existing “upstream submodule” to > > “upstream subproject” for consistency? > > For me it's still an "upstream submodule" ... We have a few existing “[upstream] subproject” references though. I prefer subproject, because the submodule's upstream repository is likely a bare repo and not a submodule itself. It's also possible (likely?) that the upstream repository is a stand-alone project, and not designed to always be a submodule. However, “upstream submodule” and “submodule's upstream” are both clear enough, and if they're the consensus phrasing, I'd rather standardize on them than jump back and forth between phrasings in the docs. I can write up a patch that shifts us to consistently use one form, once we decide what that should be (although I'm happy to let someone else write the patch too ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] Documentation/submodule: Fix submodule. -> . typos
On Fri, Mar 28, 2014 at 05:55:18PM +0100, Jens Lehmann wrote: > I just noticed that the two patches Junio added to pu have a > reworded commit message I'm perfectly happy with. The revised wording works for me too. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC] submodule: change submodule..branch default from master to HEAD
On Thu, Mar 27, 2014 at 08:52:55PM -0700, W. Trevor King wrote: > On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote: > > On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King wrote: > > > submodule..branch:: > > > A remote branch name for tracking updates in the upstream > > > submodule. > > > - If the option is not specified, it defaults to 'master'. See the > > > - `--remote` documentation in linkgit:git-submodule[1] for details. > > > + If the option is not specified, it defaults to the subproject's > > > > Did you mean s/subproject/submodule/ ? > > > > > + HEAD. See the `--remote` documentation in > > > linkgit:git-submodule[1] > > > + for details. > > No the remote branch is in the upstream subproject. I suppose I meant > “the submodule's remote-tracking branch following the upstream > subproject's HEAD which we just fetched so it's fairly current” ;). Hmm, maybe we should change the existing “upstream submodule” to “upstream subproject” for consistency? Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC] submodule: change submodule..branch default from master to HEAD
On Thu, Mar 27, 2014 at 11:43:47PM -0400, Eric Sunshine wrote: > On Thu, Mar 27, 2014 at 11:36 PM, W. Trevor King wrote: > > submodule..branch:: > > A remote branch name for tracking updates in the upstream submodule. > > - If the option is not specified, it defaults to 'master'. See the > > - `--remote` documentation in linkgit:git-submodule[1] for details. > > + If the option is not specified, it defaults to the subproject's > > Did you mean s/subproject/submodule/ ? > > > + HEAD. See the `--remote` documentation in linkgit:git-submodule[1] > > + for details. No the remote branch is in the upstream subproject. I suppose I meant “the submodule's remote-tracking branch following the upstream subproject's HEAD which we just fetched so it's fairly current” ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[RFC] submodule: change submodule..branch default from master to HEAD
gitmodule(5) mentioned 'master' as the default remote branch, but
folks using checkout-style updates are unlikely to care which upstream
branch their commit comes from (they only care that the clone fetches
that commit). If they haven't set submodule..branch, it makes
more sense to mirror 'git clone' and use the subproject's HEAD than to
default to 'master' (which may not even exist).
After the initial clone, subsequent updates may be local or remote.
Local updates (integrating gitlink changes) have no need to fetch a
specific remote branch, and get along just fine without
submodule..branch. Remote updates do need a remote branch, but
HEAD works as well here as it did for the initial clone.
Reported-by: Johan Herland
Signed-off-by: W. Trevor King
---
This still needs tests, but it gets through the following fine:
rm -rf superproject subproject &&
mkdir subproject &&
(cd subproject &&
git init &&
echo 'Subproject' > README &&
git add README &&
git commit -m 'Subproject commit' &&
git branch -m master next
) &&
mkdir superproject &&
(cd superproject &&
git init &&
git submodule add ../subproject submod &&
git commit -am 'Add submod'
)
(cd subproject &&
echo 'work work work' >> README &&
git commit -am 'Subproject commit 2'
) &&
(cd superproject &&
git submodule update --remote &&
git commit -am 'Add submod'
)
The main drawback to this approach is that we're changing a default,
but I agree with John's:
On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote:
> I expect in most cases where "origin/master" happens to be the Right
> Answer, using the submodule's upstream's HEAD will yield the same
> result.
so the default-change may not be particularly intrusive.
Cheers,
Trevor
Documentation/git-submodule.txt | 2 +-
Documentation/gitmodules.txt| 5 +++--
git-submodule.sh| 11 ---
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 46c1eeb..c485a17 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -284,7 +284,7 @@ OPTIONS
the superproject's recorded SHA-1 to update the submodule, use the
status of the submodule's remote-tracking branch. The remote used
is branch's remote (`branch..remote`), defaulting to `origin`.
- The remote branch used defaults to `master`, but the branch name may
+ The remote branch used defaults to `HEAD`, but the branch name may
be overridden by setting the `submodule..branch` option in
either `.gitmodules` or `.git/config` (with `.git/config` taking
precedence).
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f539e3f..1aecce9 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -53,8 +53,9 @@ submodule..update::
submodule..branch::
A remote branch name for tracking updates in the upstream submodule.
- If the option is not specified, it defaults to 'master'. See the
- `--remote` documentation in linkgit:git-submodule[1] for details.
+ If the option is not specified, it defaults to the subproject's
+ HEAD. See the `--remote` documentation in linkgit:git-submodule[1]
+ for details.
+
This branch name is also used for the local branch created by
non-checkout cloning updates. See the `update` documentation in
diff --git a/git-submodule.sh b/git-submodule.sh
index 6135cfa..5f08e6c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -819,8 +819,8 @@ cmd_update()
name=$(module_name "$sm_path") || exit
url=$(git config submodule."$name".url)
config_branch=$(get_submodule_config "$name" branch)
- branch="${config_branch:-master}"
- local_branch="$branch"
+ branch="${config_branch:-HEAD}"
+ local_branch="$config_branch"
if ! test -z "$update"
then
update_module=$update
@@ -860,7 +860,12 @@ Maybe you want to use 'update --init'?")"
if ! test -d "$sm_path"/.git -o -f "$sm_path"/.git
then
- start_point="origin/${branch}"
+ if test -n "$config_branch"
+ then
+ start_point="origin/$branch"
+ else
+ start_point=""
+ fi
module_clone "$sm_path" "$name" "$url" "$reference"
"$depth" "$start_point" "$local_branch" || exit
cloned_modules="$cloned_modules;$name"
subsha1=
--
1.9.1.352.gd393d14.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Possible regression in master? (submodules without a "master" branch)
On Fri, Mar 28, 2014 at 12:21:23AM +0100, Johan Herland wrote: > On Thu, Mar 27, 2014 at 9:27 PM, Heiko Voigt wrote: > > On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote: > >> There is this bit for "update" in git-submodule.txt: > >> > >> For updates that clone missing submodules, checkout-mode > >> updates will create submodules with detached HEADs; all other > >> modes will create submodules with a local branch named after > >> submodule..branch. > >> > >> … > >> So the proposed change is to make the part before semicolon true? > >> If we are not newly cloning (because we already have it), if the > >> submodule..branch is not set *OR* refers to a branch that > >> does not even exist, shouldn't we either (1) abort as an error, > >> or (2) do the same and detach? > > > > I would expect "(1) abort as an error" since the user is not > > getting what he would expect. Branch-attachment is mostly a function of submodule..update, not a function of submodule..branch. We could certainly interpret a missing submodule..branch as: * Use the subproject's HEAD for the initial clone (clear start_point in cmd_update if submodule."$name".branch is not set). * Don't change the branch name on subsequent local updates (what we already do). * Do $something if the user tries a --remote update. I just don't know what that $something should be. > FWIW, here is the behaviour I would expect from "git submodule > update": > > - In checkout-mode, if submodule..branch is not set, we > should _always_ detach. Whether or not the submodule is already > cloned does not matter. Agreed, checkout-mode should *always* detach the submodule's HEAD. > - In rebase/merge-mode, if submodule..branch is not set, we > should _always_ abort with an error. Why? Can't we mimic clone and use the remote's HEAD (for --remote updates)? That seems more intuitive to me. For local updates, we're just integrating the gitlinked commit with the submodule's HEAD, and you don't need submodule..branch for that at all. > - If submodule..branch is set - but the branch it refers to > does not exist - we should _always_ abort with an error. The current > checkout/rebase/merge-mode does not matter. Sounds good to me, and should match the current functionality. > In other words, submodule..branch is _necessary_ in > rebase/merge mode, but _optional_ in checkout-mode (its absence > indicating that we should detach). The main trigger for “we should detach” is the update mode (checkout-mode detaches, all others integrate with the submodule's HEAD (without changing submodule branches). You only need submodule..branch for determining which *remote* commit you're trying to integrate (or clone from). HEAD, master, and “die screaming” all sound like reasonable defaults in that case. Deciding between them is a policy/UI decision, not a technical decision. > >> > gitmodules(5) is pretty clear that 'submodule..branch' > >> > defaults to master (and not upstream's HEAD), do we want to > >> > adjust this at the same time? > >> > >> That may be likely. If the value set to a configuration variable > >> causes an established behaviour of a program change a lot, > >> silently defaulting that variable to something many people are > >> expected to have (e.g. 'master') would likely to cause a > >> usability regression. > > > > IMO this branch configuration should completely ignored in the > > default, non --remote, usage. Since we simply checkout a specific > > SHA1 in this case, that should be possible. > > Yes. Checkout-mode with no submodule..branch configured should > always detach. Except for the initial clone (where it's easy to fix), submodule..branch *is* ignored in non --remote updates. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 11:55:21PM +0100, Jens Lehmann wrote: > Me thinks that when a superproject doesn't have 'branch' configured > and does set 'update' to something other than 'checkout' for a > submodule it should better make sure 'master' is a valid branch in > there. Everything else sounds like a misconfiguration on the > superproject's part that warrants an error. submodule..branch should only matter for --remote updates (and the initial clone, which is a special case of remote update). So having an alternative update mode and no submodule..branch *is* a valid configuration. It says: * I want to integrate local submodule commits with superproject gitlink changes using the submodule..update strategy. * I never use --remote updates, so I haven't bothered to setup submodule..branch. I can imagine folks using a workflow like that. And I think erroring out if they *do* try a --remote update shouldn't be too surprising for them. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH] Documentation/submodule: Fix submodule. -> . typos
On Fri, Mar 28, 2014 at 12:15:00AM +0100, Jens Lehmann wrote: > Am 27.03.2014 22:06, schrieb W. Trevor King: > > The transition from submodule..* to submodule..* happened > > in 73b0898d (Teach "git submodule add" the --name option, 2012-09-30), > > which landed in v1.8.1-rc0 on 2012-12-03. > > Nope, the distinction between path and name is way older (AFAIK it > is there from day one). That was just the point in time where you > could choose a different name without editing .gitmodules. And the > fact that the name is initialized with the path confused a lot of > people. Before 73b0898d, cmd_add used: git config -f .gitmodules submodule."$sm_path".path "$sm_path" and similar, so I used submodule..branch in my initial documentation of this patch (v5 of that series) [1]. By the final v8 (which rebased onto the then-current master with 73b0898d), the surrounding calls were [2]: git config -f .gitmodules submodule."$sm_name".path "$sm_path" but I missed the update to in my rebasing. I suppose I could have used instead of in my initial v5 patch, but I was one of the folks confused by the old name == path behavior ;). > > This patch is against master, because 23d25e48 hasn't landed in maint > > yet. If you want, I can split this into two patches, one against > > maint fixing the b9289227 typo and another against master fixing the > > 23d25e48 typo. > > This fixes the only two usages of 'submodule..*' in the > Documentation I can see in current master. Right. However, this patch won't apply to the maint branch (where 23d25e48 hasn't landed). I'm just saying that we may want to split this patch in half and push the fix for b9289227 in a maintenance release. On the other hand, we've survived since 2012 with the current docs, so *not* splitting this patch apart works for me too. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/210763 [2]: http://article.gmane.org/gmane.comp.version-control.git/211832 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] Documentation/submodule: Fix submodule. -> . typos
The transition from submodule..* to submodule..* happened in 73b0898d (Teach "git submodule add" the --name option, 2012-09-30), which landed in v1.8.1-rc0 on 2012-12-03. The first submodule..branch reference landed a short time later in b9289227 (submodule add: If --branch is given, record it in .gitmodules, 2012-12-19), and I was probably just not aware of 73b0898d. The second submodule..branch reference landed in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26), and is just a copy paste error. This commit updates both references to the current submodule..branch. Reported-by: Junio C Hamano Signed-off-by: W. Trevor King --- This patch is against master, because 23d25e48 hasn't landed in maint yet. If you want, I can split this into two patches, one against maint fixing the b9289227 typo and another against master fixing the 23d25e48 typo. Documentation/git-submodule.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 46c1eeb..77588b0 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -162,7 +162,7 @@ update:: + For updates that clone missing submodules, checkout-mode updates will create submodules with detached HEADs; all other modes will create -submodules with a local branch named after `submodule..branch`. +submodules with a local branch named after `submodule..branch`. + For updates that do not clone missing submodules, the submodule's HEAD is only touched when the remote reference does not match the @@ -247,7 +247,7 @@ OPTIONS -b:: --branch:: Branch of repository to add as submodule. - The name of the branch is recorded as `submodule..branch` in + The name of the branch is recorded as `submodule..branch` in `.gitmodules` for `update --remote`. -f:: -- 1.9.1.352.gd393d14.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
submodule..branch vs. submodule..branch (was: Possible regression in master? (submodules without a "master" branch).
I'm breaking this off into a sub-thread, so it doesn't distract from the main issue. On Thu, Mar 27, 2014 at 12:39:03PM -0700, Junio C Hamano wrote: > There is this bit for "update" in git-submodule.txt: > > For updates that clone missing submodules, checkout-mode updates > will create submodules with detached HEADs; all other modes will > create submodules with a local branch named after > submodule..branch. > > [side note] Isn't that a typo of submodule..branch? Good catch. The transition from submodule..* to submodule..* happened in 73b0898d (Teach "git submodule add" the --name option, 2012-09-30), which landed in v1.8.1-rc0 on 2012-12-03. The first submodule..branch reference landed a short time later in b9289227 (submodule add: If --branch is given, record it in .gitmodules, 2012-12-19), and I was probably just not aware of 73b0898d. The second submodule..branch reference landed in 23d25e48 (submodule: explicit local branch creation in module_clone, 2014-01-26), and is just a copy paste error. Both should be updated to submodule..branch. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 06:31:27PM +0100, Jens Lehmann wrote:
> Am 27.03.2014 18:16, schrieb Junio C Hamano:
> > Johan Herland writes:
> >
> >> I just found a failure to checkout a project with submodules where
> >> there is no explicit submodule branch configuration, and the
> >> submodules happen to not have a "master" branch:
> >>
> >> git clone git://gitorious.org/qt/qt5.git qt5
> >> cd qt5
> >> git submodule init qtbase
> >> git submodule update
> >>
> >> In current master, the last command fails with the following output:
> >
> > ... and with a bug-free system, what does it do instead? Just clone
> > 'qtbase' and make a detached-head checkout at the commit recorded in
> > the superproject's tree, or something else?
>
> After reverting 23d25e48f5ead73 on current master it clones 'qtbase'
> nicely with a detached HEAD.
Fixing this for initial update clone is pretty easy, we just need to
unset start_point before calling module_clone if
submodule..branch is not set. However, that's just going to
push remote branch ambiguity problems back to the --remote update
functionality. What should happen when submodule..branch is not
set and you run a --remote update, which has used:
git rev-parse "${remote_name}/${branch}"
since the submodule..branch setting was introduced in 06b1abb
(submodule update: add --remote for submodule's upstream changes,
2012-12-19)?
gitmodules(5) is pretty clear that 'submodule..branch' defaults
to master (and not upstream's HEAD), do we want to adjust this at the
same time? For folks using non-checkout updates, should the preferred
local branch name still be master, or should it match upstream's HEAD?
If upstream's HEAD changes, should we update the local branch name to
stay in sync? If we don't rename the local branch, do we keep
integrating remote changes from upstream's original branch or keep
integrating HEAD?
I think this would all be simpler if we just made the
superproject-branch-to-submodule-local-branch binding explicit and
pushed this submodule-to-upstream-subproject binding down into the
submodule itself [1]. Then the usual single-project commands would
handle the tricky remote-tracking cases (with explicit
branch..merge entries, etc.), and a dumb syncing mechanism would
pull those clever choices back up into the superproject for
distribution.
> > If an existing set-up that was working in a sensible way is broken
> > by a change that assumes something that should not be assumed,
> > then that is a serious regression, I would have to say.
>
> Yes, especially as it promised to not change this use case.
Sorry. A side effect of relying too much on our existing
documentation and not enough on testing actual use cases. I can work
up some non-master submodule tests to go with the fix.
Cheers,
Trevor
[1]: http://thread.gmane.org/gmane.comp.version-control.git/239955/focus=240336
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 08:52:08AM -0700, W. Trevor King wrote: > Working around that to default to the upstream submodule's HEAD is > possible (you can just use --branch HEAD) Actually, this is probably not a good idea. The initial submodule addition works: $ git submodule add -b HEAD /tmp/submod.git submod Cloning into 'submod'... done. But subsequent log calls (from the superproject) do not: $ git log fatal: bad default revision 'HEAD' $ echo $? 128 and status calls (from the superproject) also have trouble: $ git status warning: refname 'HEAD' is ambiguous warning: refname 'HEAD' is ambiguous. On branch master … So it's better to just specify your preferred upstream branch directly (e.g. --branch next). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Possible regression in master? (submodules without a "master" branch)
On Thu, Mar 27, 2014 at 03:21:49PM +0100, Johan Herland wrote: > I just found a failure to checkout a project with submodules where > there is no explicit submodule branch configuration, and the > submodules happen to not have a "master" branch: The docs say [1]: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. which is what we do now. Working around that to default to the upstream submodule's HEAD is possible (you can just use --branch HEAD), but I think it's easier to just explicitly specify your preferred branch. Cheers, Trevor [1]: submodule..branch in gitmodules(5) http://git-scm.com/docs/gitmodules.html -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [git] Re: [WIP/PATCH 4/9] Teach merge the --[no-]recurse-submodules option
On Fri, Feb 07, 2014 at 02:00:23PM -0800, Junio C Hamano wrote: > Jens Lehmann writes: > > I think the user needs to sort things out, just like she has to do > > when a file has a merge conflict. But unfortunately we cannot use > > conflict markers here, so I'd propose the following: > > > > * When merge proposes a merge resolution (which it does today by > > telling the user "Found a possible merge resolution for the > > submodule ... [use] git update-index --cacheinfo 16 ...") > > that commit should be checked out in the submodule but not > > staged. Then the user can simply add and commit. > > … > … > > For the former, "add and commit" at the top-level makes perfect > sense, … This still works if the merge issue is in a grandchild submodule, but it's going to be a bit tedious if the user has to add-and-commit at each level from the troublesome sub-sub-…-module on up to the top-level superproject. I can't think of a cleaner solution though. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents
On Mon, Feb 03, 2014 at 08:52:49PM +0100, Jens Lehmann wrote: > Implement the functionality needed to enable work tree manipulating > commands to that a deleted submodule should not only affect the index > (leaving all the files of the submodule in the work tree) but also to > remove the work tree of the superproject (including any untracked > files). > > That will only work properly when the submodule uses a gitfile instead of > a .git directory and no untracked files are present. Otherwise the removal > will fail with a warning (which is just what happened until now). I'm having trouble parsing this one. How about: Add a depopulate_submodule helper which removes the submodule working directory without touching the index. This will only work properly when the submodule uses a gitfile… Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules
On Mon, Feb 03, 2014 at 08:54:17PM +0100, Jens Lehmann wrote: > Implement the functionality needed to enable work tree manipulating > commands so that an changed submodule does not only affect the index but > it also updates the work tree of any initialized submodule according to > the SHA-1 recorded in the superproject. How about: …so that *a* changed submodule ** updates the index and work tree of any initialized submodule according to the SHA-1 recorded in the superproject. Before this commit it updated neither; users had to run 'submodule update' to propagate gitlink updates into the submodule. I'm pretty sure that's accurate anyway ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [WIP/PATCH 6/9] Teach bisect the --[no-]recurse-submodules option
On Mon, Feb 03, 2014 at 08:51:57PM +0100, Jens Lehmann wrote: > submodule update' eacht time obsolete, which was tedious and error prone. ^ each I'm just reading the commit messages this pass ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 1/4] submodule: Make 'checkout' update_module explicit
On Sun, Jan 26, 2014 at 08:32:04PM -0500, Eric Sunshine wrote: > On Sun, Jan 26, 2014 at 3:45 PM, W. Trevor King wrote: > > + update_module="checkout" > > Here, you (unnecessarily) quote 'checkout'... > > > - update_module= ;; > > + update_module=checkout ;; > > But here you use bare (unquoted) 'checkout'. Bare is probably more > idiomatic. Whatever you want ;). Queued for v6. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH v5 2/4] submodule: Document module_clone arguments in comments
Signed-off-by: W. Trevor King --- git-submodule.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-submodule.sh b/git-submodule.sh index 5e8776c..68dcbe1 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -241,6 +241,12 @@ module_name() # # Clone a submodule # +# $1 = submodule path +# $2 = submodule name +# $3 = URL to clone +# $4 = reference repository to reuse (empty for independent) +# $5 = depth argument for shallow clones (empty for deep) +# # Prior to calling, cmd_update checks that a possibly existing # path is not a git repository. # Likewise, cmd_add checks that path does not exist at all, -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/4] Documentation: Describe 'submodule update --remote' use case
On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote [1]: > I think copying some motivation from the log message of 06b1abb5 > (submodule update: add --remote for submodule's upstream changes, > 2012-12-19) would help the readers here. A naïve expectation from a > casual reader of the above would be "The superproject's tree ought > to point at the same commit as the tip of the branch used in the > submodule (modulo mirroring delays and somesuch), if the repository > of the superproject and submodules are maintained properly", which > would lead to "when would any sane person need to use --remote in > the first place???". There have been other interpretation issues with the --remote option as well. With this commit, I try to make it clear that there is no implicit floating going on; --remote lets you explicitly integrate the upstream branch in your current HEAD (just like running 'git pull' in the submodule). The only distinction with the current 'git pull' is the config location/setting used for the upstream branch, which is hopefully clear now. With syncing between the out-of-tree submodule config and the in-tree superproject .gitmodules [2], you wouldn't have to chose between (or manually sync) "easily distributable .gitmodules settings" and "native submodule pull", but this patch is my take on the current situation. [1]: http://article.gmane.org/gmane.comp.version-control.git/240529 [2]: http://article.gmane.org/gmane.comp.version-control.git/240336 Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2e1c7a2..21cb59a 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -299,6 +299,16 @@ In order to ensure a current tracking branch state, `update --remote` fetches the submodule's remote repository before calculating the SHA-1. If you don't want to fetch, you should use `submodule update --remote --no-fetch`. ++ +Use this option to integrate changes from the upstream subproject with +your submodule's current HEAD. Alternatively, you can run `git pull` +from the submodule, which is equivalent except for the remote branch +name: `update --remote` uses the default upstream repository and +`submodule..branch`, while `git pull` uses the submodule's +`branch..merge`. Prefer `submodule..branch` if you want +to distribute the default upstream branch with the superproject and +`branch..merge` if you want a more native feel while working in +the submodule itself. -N:: --no-fetch:: -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/4] submodule: Explicit local branch creation in module_clone
The previous code only checked out branches in cmd_add. This commit moves the branch-checkout logic into module_clone, where it can be shared by cmd_add and cmd_update. I also update the initial checkout command to use 'reset' to preserve branches setup during module_clone. With this change, folks cloning submodules for the first time via: $ git submodule update ... will get a local branch instead of a detached HEAD, unless they are using the default checkout-mode updates. This is a change from the previous situation where cmd_update always used checkout-mode logic (regardless of the requested update mode) for updates that triggered an initial clone, which always resulted in a detached HEAD. This commit does not change the logic for updates after the initial clone, which will continue to create detached HEADs for checkout-mode updates, and integrate remote work with the local HEAD (detached or not) in other modes. The motivation for the change is that developers doing local work inside the submodule are likely to select a non-checkout-mode for updates so their local work is integrated with upstream work. Developers who are not doing local submodule work stick with checkout-mode updates so any apparently local work is blown away during updates. For example, if upstream rolls back the remote branch or gitlinked commit to an earlier version, the checkout-mode developer wants their old submodule checkout to be rolled back as well, instead of getting a no-op merge/rebase with the rolled-back reference. By using the update mode to distinguish submodule developers from black-box submodule consumers, we can setup local branches for the developers who will want local branches, and stick with detached HEADs for the developers that don't care. Testing === In t7406, just-cloned checkouts now update to the gitlinked hash with 'reset', to preserve the local branch for situations where we're not on a detached HEAD. I also added explicit tests to t7406 for HEAD attachement after cloning updates, showing that it depends on their update mode: * Checkout-mode updates get detached HEADs * Everyone else gets a local branch, matching the configured submodule..branch and defaulting to master. The 'initial-setup' tag makes it easy to reset the superproject to a known state, as several earlier tests commit to submodules and commit the changed gitlinks to the superproject, but don't push the new submodule commits to the upstream subprojects. This makes it impossible to checkout the current super master, because it references submodule commits that don't exist in the upstream subprojects. For a specific example, see the tests that currently generate the 'two_new_submodule_commits' commits. Documentation = I updated the docs to describe the 'submodule update' modes in detail. The old documentation did not distinguish between cloning and non-cloning updates and lacked clarity on which operations would lead to detached HEADs, and which would not. The new documentation addresses these issues while updating the docs to reflect the changes introduced by this commit's explicit local branch creation in module_clone. I also add '--checkout' to the usage summary and group the update-mode options into a single set. Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 36 ++--- Documentation/gitmodules.txt| 4 +++ git-submodule.sh| 58 + t/t7406-submodule-update.sh | 39 ++- 4 files changed, 110 insertions(+), 27 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..2e1c7a2 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -15,8 +15,8 @@ SYNOPSIS 'git submodule' [--quiet] init [--] [...] 'git submodule' [--quiet] deinit [-f|--force] [--] ... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference ] [--depth ] - [--merge] [--recursive] [--] [...] + [-f|--force] [--rebase|--merge|--checkout] [--reference ] + [--depth ] [--recursive] [--] [...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ] [commit] [--] [...] 'git submodule' [--quiet] foreach [--recursive] @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying -
[PATCH v5 1/4] submodule: Make 'checkout' update_module explicit
This avoids the current awkwardness of having either '' or 'checkout' for checkout-mode updates, which makes testing for checkout-mode updates (or non-checkout-mode updates) easier. Signed-off-by: W. Trevor King --- git-submodule.sh | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 5247f78..5e8776c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule."$name".update) - case "$update_module" in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")" - ;; - esac + if test -z "$update_module" + then + update_module="checkout" + fi fi displaypath=$(relative_path "$prefix$sm_path") @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?")" case ";$cloned_modules;" in *";$name;"*) # then there is no local change to integrate - update_module= ;; + update_module=checkout ;; esac must_die_on_failure= case "$update_module" in + checkout) + command="git checkout $subforce -q" + die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")" + say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")" + ;; rebase) command="git rebase" die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$displaypath'")" @@ -906,10 +904,7 @@ Maybe you want to use 'update --init'?")" must_die_on_failure=yes ;; *) - command="git checkout $subforce -q" - die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")" - say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")" - ;; + die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")" esac if (clear_local_git_env; cd "$sm_path" && $command "$sha1") -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/4] submodule: Local branch creation in module_clone
Changes since v4: In git-submodule.sh: * Explicitly set an empty $local_branch in cmd_add if $branch is empty [1]. * Restore die-early checking for invalid $update_module [2]. This check is now outside the load-from-config branch, ensuring we have a valid update_module, regardless of how it was set. In Documentation/git-submodule.txt: * Fix “but be” → “but can be” [3]. * Fix “checkout” → “--checkout” [4]. * New text on why you'd use --remote [5] (new commit #4). In Documentation/git-submodule.txt and Documentation/gitmodules.txt: * Use backticks (instead of single quotes) for command line options [6]. I also squashed the implementation, testing fixes, new tests, and documentation for the new local_branch stuff (v4's #3, #4, #5, and #6) into a single commit (v5's #3) [7]. [1]: http://article.gmane.org/gmane.comp.version-control.git/240524 [2]: http://article.gmane.org/gmane.comp.version-control.git/240522 [3]: http://article.gmane.org/gmane.comp.version-control.git/240543 [4]: http://article.gmane.org/gmane.comp.version-control.git/240531 [5]: http://article.gmane.org/gmane.comp.version-control.git/240529 [6]: http://article.gmane.org/gmane.comp.version-control.git/240536 [7]: http://article.gmane.org/gmane.comp.version-control.git/240530 W. Trevor King (4): submodule: Make 'checkout' update_module explicit submodule: Document module_clone arguments in comments submodule: Explicit local branch creation in module_clone Documentation: Describe 'submodule update --remote' use case Documentation/git-submodule.txt | 46 - Documentation/gitmodules.txt| 4 ++ git-submodule.sh| 89 ++--- t/t7406-submodule-update.sh | 39 +- 4 files changed, 136 insertions(+), 42 deletions(-) -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: submodules
On Thu, Jan 23, 2014 at 03:38:49AM -0500, shawn wilson wrote: > My issue is in trying to update the submodules, I'm getting: > % git submodule update --init > gits/kt (master ⚡) > swlap1 > fatal: reference is not a tree: 98f1e9f99fca32ab3de901219eb2f600d38ab3a5 > Unable to checkout '98f1e9f99fca32ab3de901219eb2f600d38ab3a5' in > submodule path 'repo_a' > > Ok, so a bit of googling and I found this: > http://stackoverflow.com/questions/13425298/git-submodule-update-fatal-error-reference-is-not-a-tree > Ok, so update the repo that contains the submodules every time you > push from a sub repo? How / where do I create a hook to do this? You've got it switched. You *did* push the superproject, but forgot to push the new submodule commits that it references. An easy way to avoid this problem is to always push the superproject using: $ git push --recurse-submodules=on-demand … If you no longer have access to the submodule repositories that weren't pushed, you'll have to roll back the superproject so it references submodule commits that were pushed. The easiest way to do this is probably to just cd into the submodule directory and checkout an appropriate commit (e.g. origin/master): $ cd repo_a $ git fetch origin $ git checkout origin/master That will put you on a detached HEAD, so you might want to replace the checkout with: $ git checkout -B master origin/master or some such to get a local branch. Then cd back into the superproject and commit the submodule (referencing the current submodule commit): $ cd .. $ git commit -m 'repo_a: Roll back to last public commit' repo_a When you push that commit, don't forget to push everything ;) $ git push --recurse-submodules=on-demand Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 01:55:37PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: > >> "W. Trevor King" writes: > >>> +is only touched when the remote reference does not match the > >>> +submodule's HEAD (for none-mode updates, the submodule is never > >>> +touched). The remote reference is usually the gitlinked commit from > >>> +the superproject's tree, but with '--remote' it is the upstream > >>> +subproject's 'submodule..branch'. This remote reference is > >>> +integrated with the submodule's HEAD using the specified update mode. > >> … > >> A naïve expectation from a casual reader of the above would be > >> "The superproject's tree ought to point at the same commit as the > >> tip of the branch used in the submodule (modulo mirroring delays > >> and somesuch), > > > > What is the branch used in the submodule? The remote subproject's > > current submodule..branch? The local submodule's > > submodule..branch (or localBranch) branch? The submodule's > > current HEAD? > > They are good questions that such casual readers would have, and > giving answers to them in this part of the documentation would be a > good way to give them a clear picture of how the command is designed > to be used. How about: Note that the update command only interacts with the submodule's HEAD. It doesn't care what this head points to. If the submodule has a branch checked out, HEAD will reference that branch. If the submodule's HEAD is detached, it will reference a commit. After following any references, the commit referenced by the submodule's HEAD may resolve to the commit gitlinked by the superproject, or it may not (if you have made local submodule changes, or checked out a different superproject branch). The update command does not adjust your submodule's HEAD to point at the gitlinked commit before performing any integration. It just takes your submodle's HEAD, whatever it points to, and integrates the remote reference. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 10:18:06PM -, Philip Oakley wrote: > From: "Junio C Hamano" > > "W. Trevor King" writes: > >> + repository. The update mode defaults to 'checkout', but be > > nit: s/but be/but can be/ ? Thanks. Queuing for v5. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 09:02:22PM +, John Keeping wrote: > On Thu, Jan 16, 2014 at 12:55:21PM -0800, W. Trevor King wrote: > > On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: > > > Not '--checkout'? > > > > Oops, will fix in v5. > > Shouldn't this also be `--checkout` (backticks), according to > CodingGuidelines. This applies to all this options in this patch I > think. I can change that too. The existing content is inconsistent between backticks and single quotes, but I see no mention of single quotes in CodingGuidelines. Thanks for the reference. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
On Thu, Jan 16, 2014 at 11:43:44AM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > @@ -817,11 +831,15 @@ cmd_update() > > > > displaypath=$(relative_path "$prefix$sm_path") > > > > - if test "$update_module" = "none" > > - then > > + case "$update_module" in > > + none) > > echo "Skipping submodule '$displaypath'" > > continue > > - fi > > + ;; > > + checkout) > > + local_branch="" > > + ;; > > + esac > > I wonder if there is a way to avoid detaching (and you may need to > update the coddpath that resets the submodule to the commit > specified by the superproject tree) when it is safe to do so. > > … > > Perhaps that kind of "'git submodule update' is parallel to 'git > pull' in the project without submodules" is better done with other > update modes like --rebase or --merge. If so, how should we explain > what 'submodule update --checkout' is to the end users? Is it > supposed to be like "git fetch && git checkout origin/master"? It sounds like you're looking for: submodule..update = !git merge --ff-only That's fine for folks who want that sort of advancement, but I think there will also be blind updaters who just want the gitlinked commit, and don't care if that blows away local work, because they never work locally in the submodule. They'll still prefer the current checkout-mode with it's clobbering. I think the best way to explain this to users is to have 'git checkout' (with an optional '--recurse-submdules' trial period) checkout the gitlinked commit automatically. Then there is never local submodule work that is not committed or stashed in the superproject (or stashed on some out-of-the-way branch in the submodule). Currently we have: 1. Checkout a superproject branch and currently gitlinked submodule. 2. Do local work on the submodule. 3. Alter the superproject and its gitlinks. 4. 'git submodule update' to integrate your work from 2 with the changes from 3 and checkout the appropriate submodule commit. I think it would make more sense to: 1. Checkout a superproject branch and currently gitlinked submodule. 2. Do local work on the submodule. 3. Commit your new gitlink to the superproject (or stash it, or put it on a temporary submodule branch and reset the submodule HEAD to the old value). 4. Alter the superproject and its gitlinks, using the existing logic to integrate conflicts. Automatically checkout the appropriate submodule commit (as the appropriate submodule branch). That shifts “dealing with local submodule changes” from an integration-time issue (I just called submodule update, what changes are local?) to a pre-checkout-time issue (I've got a dirty submodule (it's HEAD is not the gitlinked commit, all of these changes are local). I think that's a lot easier to wrap your head around. This series is a stop-gap to avoid detached HEADs after cloning, non-checkout updates, while we hash out the real solution ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
On Thu, Jan 16, 2014 at 12:21:04PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > @@ -155,13 +155,31 @@ it contains local modifications. > > > > update:: > > Update the registered submodules, i.e. clone missing submodules and > > - checkout the commit specified in the index of the containing repository. > > - This will make the submodules HEAD be detached unless `--rebase` or > > - `--merge` is specified or the key `submodule.$name.update` is set to > > - `rebase`, `merge` or `none`. `none` can be overridden by specifying > > - `--checkout`. Setting the key `submodule.$name.update` to `!command` > > - will cause `command` to be run. `command` can be any arbitrary shell > > - command that takes a single argument, namely the sha1 to update to. > > + checkout the commit specified in the index of the containing > > + repository. The update mode defaults to 'checkout', but be > > + configured with the 'submodule..update' setting or the > > + '--rebase', '--merge', or 'checkout' options. > > Not '--checkout'? Oops, will fix in v5. > > ++ > > +For updates that clone missing submodules, checkout-mode updates will > > +create submodules with detached HEADs; all other modes will create > > +submodules with a local branch named after 'submodule..branch'. > > ++ > > +For updates that do not clone missing submodules, the submodule's HEAD > > That is, updates that update submodules that are already checked out? It's submodules for which $sm_path/.git does not exist. > > +is only touched when the remote reference does not match the > > +submodule's HEAD (for none-mode updates, the submodule is never > > +touched). The remote reference is usually the gitlinked commit from > > +the superproject's tree, but with '--remote' it is the upstream > > +subproject's 'submodule..branch'. This remote reference is > > +integrated with the submodule's HEAD using the specified update mode. > > I think copying some motivation from the log message of 06b1abb5 > (submodule update: add --remote for submodule's upstream changes, > 2012-12-19) would help the readers here. I think a brief reference to --remote is best here, mentioning that it has something to do with the upstream subproject. More detail on when you should use --remote should probably go under the docs for --remote ;). > A naïve expectation from a casual reader of the above would be "The > superproject's tree ought to point at the same commit as the tip of > the branch used in the submodule (modulo mirroring delays and > somesuch), What is the branch used in the submodule? The remote subproject's current submodule..branch? The local submodule's submodule..branch (or localBranch) branch? The submodule's current HEAD? > if the repository of the superproject and submodules are maintained > properly", which would lead to "when would any sane person need to > use --remote in the first place???". --remote is not for sane people (who will probably be pulling from withing the submodule itself). --remote is for folks who track too many submodules to care about them as individuals, who just want to blindly update to whatever the upstream subproject maintainer has in submodule..branch. For example, if you are a distribution with a hundred submodules tracking all the projects you package, and want to bump them all to a their current trunk tip in one fell swoop. > If I am reading 06b1abb5 correctly, the primary motivation behind > "--remote" seems to be that it is exactly to help the person who > wants to update superproject to satisify the "... are maintained > properly" part by fetching the latest in each of the submodules in > his superproject in preparation to 'git add .' them. I still do not > think "--remote" was a better way than the "foreach", but that is a > separate topic. I agree now ;), the problems with: $ git submodule foreach 'git pull' are: 1. You may not be on the “right” local branch to begin with, and 2. You may not have out-of-tree submodule configs setting up the “right” upstream for that branch. 06b1abb did not address the former, and added a new .gitmodules-level submodule..branch to help with the latter. I now think all of this would be easier if we had automatic submodule local-branch checkouts (fixing problem 1) and synchronized out-of-tree submodule configs (fixing problem 2). Fixing problem 1 is all you need if you aren't interested in sharing your out-of-tree configs. I think 06b1abb was inspired by “we already have a
Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
On Thu, Jan 16, 2014 at 09:07:22PM +0100, Francesco Pretto wrote: > 2014/1/16 W. Trevor King : > > Avoiding useless clones is probably more important than avoiding > > duplicate "Invalid update mode" messages. > > No, it's not duplicate code. I meant “duplicating the "Invalid update mode" error message”. I missed the die-early distinction, but I understand now. I think its non-DRY to have an early case statement to validate the update_module, and a later case statement to use it. Still, keeping those separate statements in sync shouldn't be too hard ;). > Please keep both as Junio said. That's what I said I'd do in the email you're quoting ;). Are you ok with the die-early validation checking all update_module settings instead of just checking the loaded-from config branch? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
On Thu, Jan 16, 2014 at 11:22:52AM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > To preserve the local branch, for situations where we're not on a > > detached HEAD. > > > > Signed-off-by: W. Trevor King > > --- > > This should be a part of some other change that actually changes how > this "git submodule update" checks out the submodule, no? Sure, we can squash both this test fix and the subsequent new test patch into patch #3 in v5. I was just splitting them out because backwards compatibility was a concern, and separate patches makes it easy for me to explain why the results changed here without getting lost in patch #3's implementation details. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
On Thu, Jan 16, 2014 at 11:18:00AM -0800, Junio C Hamano wrote:
> "W. Trevor King" writes:
> > @@ -312,7 +317,16 @@ module_clone()
> > echo "gitdir: $rel/$a" >"$sm_path/.git"
> >
> > rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
> > - (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config
> > core.worktree "$rel/$b")
> > + (
> > + clear_local_git_env
> > + cd "$sm_path" &&
> > + GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
> > + # ash fails to wordsplit ${local_branch:+-B "$local_branch"...}
>
> Interesting...
That's copied out of the old cmd_add code ;). I don't have ash
intalled to actually confirm the comment.
> > + case "$local_branch" in
> > + '') git checkout -f -q ${start_point:+"$start_point"} ;;
> > + ?*) git checkout -f -q -B "$local_branch"
> > ${start_point:+"$start_point"} ;;
> > + esac
>
> I am wondering if it makes more sense if you did this instead:
>
> git checkout -f -q ${start_point:+"$start_point"} &&
> if test -n "$local_branch"
> then
> git checkout -B "$local_branch" HEAD
> fi
>
> The optional "re-attaching to the local_branch" done with the second
> "checkout" would be a non-destructive no-op to the working tree and
> to the index, but it does distinguish between creating the branch
> anew and resetting the existing branch in its output (note that
> there is no "-q" to squelch it). By doing it this way, when we
> later teach "git branch -f" and "git checkout -B" to report more
> about what commits have been lost by such a resetting, you will get
> the safety for free if you made the switching with "-B" run without
> "-q" here.
This is immediately post-non-checkout-clone. There are no local
branches to clobber yet.
> > @@ -475,16 +489,14 @@ Use -f if you really want to add it." >&2
> > echo "$(eval_gettext "Reactivating local git
> > directory for submodule '\$sm_name'.")"
> > fi
> > fi
> > - module_clone "$sm_path" "$sm_name" "$realrepo" "$reference"
> > "$depth" || exit
> > - (
> > - clear_local_git_env
> > - cd "$sm_path" &&
> > - # ash fails to wordsplit ${branch:+-b "$branch"...}
> > - case "$branch" in
> > - '') git checkout -f -q ;;
> > - ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
> > - esac
> > - ) || die "$(eval_gettext "Unable to checkout submodule
> > '\$sm_path'")"
> > + if test -n "$branch"
> > + then
> > + start_point="origin/$branch"
> > + local_branch="$branch"
> > + else
> > + start_point=""
> > + fi
>
> I'd feel safer if the "else" clause explicitly cleared $local_branch
> by assigning an empty string to it; it would make it a lot clearer
> that "when $branch is an empty string here, we do not want to
> trigger the new codepath to run checkout with "-B $local_branch" in
> module_clone" is what you mean.
Ok. Will add in v5.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
On Thu, Jan 16, 2014 at 10:46:36AM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > @@ -803,17 +803,10 @@ cmd_update() > > update_module=$update > > else > > update_module=$(git config submodule."$name".update) > > - case "$update_module" in > > - '') > > - ;; # Unset update mode > > - checkout | rebase | merge | none) > > - ;; # Known update modes > > - !*) > > - ;; # Custom update command > > - *) > > - die "$(eval_gettext "Invalid update mode > > '$update_module' for submodule '$name'")" > > - ;; > > - esac > > + if test -z "$update_module" > > + then > > + update_module="checkout" > > + fi > > fi > > Is this a good change? > > It removes the code to prevent a broken configuration value from > slipping through. The code used to stop early to give the user a > chance to fix it before actually letting "submodule update" to go > into the time consuming part, e.g. a call to module_clone, but that > code is now lost. Avoiding useless clones is probably more important than avoiding duplicate "Invalid update mode" messages. I'll reinstate the case statement in v5, but I think it should live outside of the “load from config” block, in case someone adds the ability to set arbitrary update modes from the command line (`--update merge`, `--update '!command'`, etc.). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH v4 4/6] t7406: Just-cloned checkouts update to the gitlinked hash with 'reset'
To preserve the local branch, for situations where we're not on a detached HEAD. Signed-off-by: W. Trevor King --- t/t7406-submodule-update.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 0825a92..5aa9591 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -703,7 +703,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re git clone super_update_r super_update_r2 && (cd super_update_r2 && git submodule update --init --recursive >actual && -test_i18ngrep "Submodule path .submodule/subsubmodule.: checked out" actual && +test_i18ngrep "Submodule path .submodule/subsubmodule.: .git reset --hard -q" actual && (cd submodule/subsubmodule && git log > ../../expected ) && -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/6] Documentation: Describe 'submodule update' modes in detail
The old documentation did not distinguish between cloning and non-cloning updates and lacked clarity on which operations would lead to detached HEADs, and which would not. The new documentation addresses these issues while updating the docs to reflect the changes introduced by this branch's explicit local branch creation in module_clone. I also add '--checkout' to the usage summary and group the update-mode options into a single set. Signed-off-by: W. Trevor King --- Documentation/git-submodule.txt | 36 +++- Documentation/gitmodules.txt| 4 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index bfef8a0..02500b4 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -15,8 +15,8 @@ SYNOPSIS 'git submodule' [--quiet] init [--] [...] 'git submodule' [--quiet] deinit [-f|--force] [--] ... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] - [-f|--force] [--rebase] [--reference ] [--depth ] - [--merge] [--recursive] [--] [...] + [-f|--force] [--rebase|--merge|--checkout] [--reference ] + [--depth ] [--recursive] [--] [...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ] [commit] [--] [...] 'git submodule' [--quiet] foreach [--recursive] @@ -155,13 +155,31 @@ it contains local modifications. update:: Update the registered submodules, i.e. clone missing submodules and - checkout the commit specified in the index of the containing repository. - This will make the submodules HEAD be detached unless `--rebase` or - `--merge` is specified or the key `submodule.$name.update` is set to - `rebase`, `merge` or `none`. `none` can be overridden by specifying - `--checkout`. Setting the key `submodule.$name.update` to `!command` - will cause `command` to be run. `command` can be any arbitrary shell - command that takes a single argument, namely the sha1 to update to. + checkout the commit specified in the index of the containing + repository. The update mode defaults to 'checkout', but be + configured with the 'submodule..update' setting or the + '--rebase', '--merge', or 'checkout' options. ++ +For updates that clone missing submodules, checkout-mode updates will +create submodules with detached HEADs; all other modes will create +submodules with a local branch named after 'submodule..branch'. ++ +For updates that do not clone missing submodules, the submodule's HEAD +is only touched when the remote reference does not match the +submodule's HEAD (for none-mode updates, the submodule is never +touched). The remote reference is usually the gitlinked commit from +the superproject's tree, but with '--remote' it is the upstream +subproject's 'submodule..branch'. This remote reference is +integrated with the submodule's HEAD using the specified update mode. +For checkout-mode updates, that will result in a detached HEAD. For +rebase- and merge-mode updates, the commit referenced by the +submodule's HEAD may change, but the symbolic reference will remain +unchanged (i.e. checked-out branches will still be checked-out +branches, and detached HEADs will still be detached HEADs). If none +of the builtin modes fit your needs, set 'submodule..update' to +'!command' to configure a custom integration command. 'command' can +be any arbitrary shell command that takes a single argument, namely +the sha1 to update to. + If the submodule is not yet initialized, and you just want to use the setting as stored in .gitmodules, you can automatically initialize the diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index f7be93f..36e5447 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -53,6 +53,10 @@ submodule..branch:: A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to 'master'. See the `--remote` documentation in linkgit:git-submodule[1] for details. ++ +This branch name is also used for the local branch created by +non-checkout cloning updates. See the 'update' documentation in +linkgit:git-submodule[1] for details. submodule..fetchRecurseSubmodules:: This option can be used to control recursive fetching of this -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/6] submodule: Explicit local branch creation in module_clone
The previous code only checked out branches in cmd_add. This commit
moves the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. I also update the initial checkout
command to use 'reset' to preserve branches setup during module_clone.
With this change, folks cloning submodules for the first time via:
$ git submodule update ...
will get a local branch instead of a detached HEAD, unless they are
using the default checkout-mode updates. This is a change from the
previous situation where cmd_update always used checkout-mode logic
(regardless of the requested update mode) for updates that triggered
an initial clone, which always resulted in a detached HEAD.
This commit does not change the logic for updates after the initial
clone, which will continue to create detached HEADs for checkout-mode
updates, and integrate remote work with the local HEAD (detached or
not) in other modes.
The motivation for the change is that developers doing local work
inside the submodule are likely to select a non-checkout-mode for
updates so their local work is integrated with upstream work.
Developers who are not doing local submodule work stick with
checkout-mode updates so any apparently local work is blown away
during updates. For example, if upstream rolls back the remote branch
or gitlinked commit to an earlier version, the checkout-mode developer
wants their old submodule checkout to be rolled back as well, instead
of getting a no-op merge/rebase with the rolled-back reference.
By using the update mode to distinguish submodule developers from
black-box submodule consumers, we can setup local branches for the
developers who will want local branches, and stick with detached HEADs
for the developers that don't care.
Signed-off-by: W. Trevor King
---
git-submodule.sh | 53 -
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 68dcbe1..4a09951 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -246,6 +246,9 @@ module_name()
# $3 = URL to clone
# $4 = reference repository to reuse (empty for independent)
# $5 = depth argument for shallow clones (empty for deep)
+# $6 = (remote-tracking) starting point for the local branch (empty for HEAD)
+# $7 = local branch to create (empty for a detached HEAD, unless $6 is
+# also empty, in which case the local branch is left unchanged)
#
# Prior to calling, cmd_update checks that a possibly existing
# path is not a git repository.
@@ -259,6 +262,8 @@ module_clone()
url=$3
reference="$4"
depth="$5"
+ start_point="$6"
+ local_branch="$7"
quiet=
if test -n "$GIT_QUIET"
then
@@ -312,7 +317,16 @@ module_clone()
echo "gitdir: $rel/$a" >"$sm_path/.git"
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config
core.worktree "$rel/$b")
+ (
+ clear_local_git_env
+ cd "$sm_path" &&
+ GIT_WORK_TREE=. git config core.worktree "$rel/$b" &&
+ # ash fails to wordsplit ${local_branch:+-B "$local_branch"...}
+ case "$local_branch" in
+ '') git checkout -f -q ${start_point:+"$start_point"} ;;
+ ?*) git checkout -f -q -B "$local_branch"
${start_point:+"$start_point"} ;;
+ esac
+ ) || die "$(eval_gettext "Unable to setup cloned submodule
'\$sm_path'")"
}
isnumber()
@@ -475,16 +489,14 @@ Use -f if you really want to add it." >&2
echo "$(eval_gettext "Reactivating local git
directory for submodule '\$sm_name'.")"
fi
fi
- module_clone "$sm_path" "$sm_name" "$realrepo" "$reference"
"$depth" || exit
- (
- clear_local_git_env
- cd "$sm_path" &&
- # ash fails to wordsplit ${branch:+-b "$branch"...}
- case "$branch" in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
- esac
- ) || die "$(eval_gettext "Unable to checkout submodule
'\$sm_path'")"
+ if test -n "$branch"
+ then
+ start_point="origin/$branch"
+ local_branch="$branch"
+
[PATCH v4 5/6] t7406: Add explicit tests for head attachement after cloning updates
Test that cloning updates checkout the appropriate local branch for their update-mode: * Checkout-mode updates get detached HEADs * Everyone else gets a local branch, matching the configured submodule..branch and defaulting to master. The 'initial-setup' tag makes it easy to reset the superproject to a known state, as several earlier tests commit to submodules and commit the changed gitlinks to the superproject, but don't push the new submodule commits to the upstream subprojects. This makes it impossible to checkout the current super master, because it references submodule commits that don't exist in the upstream subprojects. For a specific example, see the tests that currently generate the 'two_new_submodule_commits' commits. Signed-off-by: W. Trevor King --- t/t7406-submodule-update.sh | 37 + 1 file changed, 37 insertions(+) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 5aa9591..f056c01 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -63,6 +63,9 @@ test_expect_success 'setup a submodule tree' ' git submodule add ../none none && test_tick && git commit -m "none" + ) && + (cd super && +git tag initial-setup ) ' @@ -764,4 +767,38 @@ test_expect_success 'submodule update clone shallow submodule' ' ) ) ' + +test_expect_success 'submodule update --checkout clones detached HEAD' ' + git clone super super4 && + echo "detached HEAD" >expected && + (cd super4 && +git reset --hard initial-setup && +git submodule init submodule && +git submodule update >> /tmp/log 2>&1 && +(cd submodule && + git symbolic-ref HEAD > ../../actual || + echo "detached HEAD" > ../../actual +) + ) && + test_cmp actual expected && + rm -rf super4 +' + +test_expect_success 'submodule update --merge clones attached HEAD' ' + git clone super super4 && + echo "refs/heads/master" >expected && + (cd super4 && +git reset --hard initial-setup && +git submodule init submodule && +git config submodule.submodule.update merge && +git submodule update --merge && +(cd submodule && + git symbolic-ref HEAD > ../../actual || + echo "detached HEAD" > ../../actual +) + ) && + test_cmp actual expected && + rm -rf super4 +' + test_done -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/6] submodule: Document module_clone arguments in comments
Signed-off-by: W. Trevor King --- git-submodule.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/git-submodule.sh b/git-submodule.sh index 5e8776c..68dcbe1 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -241,6 +241,12 @@ module_name() # # Clone a submodule # +# $1 = submodule path +# $2 = submodule name +# $3 = URL to clone +# $4 = reference repository to reuse (empty for independent) +# $5 = depth argument for shallow clones (empty for deep) +# # Prior to calling, cmd_update checks that a possibly existing # path is not a git repository. # Likewise, cmd_add checks that path does not exist at all, -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/6] submodule: Make 'checkout' update_module explicit
This avoids the current awkwardness of having either '' or 'checkout' for checkout-mode updates, which makes testing for checkout-mode updates (or non-checkout-mode updates) easier. Signed-off-by: W. Trevor King --- git-submodule.sh | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 5247f78..5e8776c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -803,17 +803,10 @@ cmd_update() update_module=$update else update_module=$(git config submodule."$name".update) - case "$update_module" in - '') - ;; # Unset update mode - checkout | rebase | merge | none) - ;; # Known update modes - !*) - ;; # Custom update command - *) - die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")" - ;; - esac + if test -z "$update_module" + then + update_module="checkout" + fi fi displaypath=$(relative_path "$prefix$sm_path") @@ -882,11 +875,16 @@ Maybe you want to use 'update --init'?")" case ";$cloned_modules;" in *";$name;"*) # then there is no local change to integrate - update_module= ;; + update_module=checkout ;; esac must_die_on_failure= case "$update_module" in + checkout) + command="git checkout $subforce -q" + die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")" + say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")" + ;; rebase) command="git rebase" die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$displaypath'")" @@ -906,10 +904,7 @@ Maybe you want to use 'update --init'?")" must_die_on_failure=yes ;; *) - command="git checkout $subforce -q" - die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")" - say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")" - ;; + die "$(eval_gettext "Invalid update mode '$update_module' for submodule '$name'")" esac if (clear_local_git_env; cd "$sm_path" && $command "$sha1") -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/6] submodule: Local branch creation in module_clone
Here is the next iteration of cloning-update local branch setup. This version is based on Francesco's fp/submodule-checkout-mode [1], and moves back towards the weakly-bound v2 approach and away from the tightly-bound v3 approach. The first patch in this series extends that commit to consolidate '' and 'checkout' update_module values. I wouldn't mind if that gets squashed into Francesco's patch. The meat of the series is in the third patch, which changes the v2 implementation by triggering attached HEADs based on the update-mode instead of on the existence of a non-empty submodule..branch [2]. There has been pushback from both Heiko [3] and Jens [4] on my mapping checkout updaters to “developers not interested in local submodule development”, but I still think it's the right interpretation. I'm not clear on Heiko or Jens' current possitions, maybe I've won them over ;). The other patches in this series are all new in v4. This still does a double checkout (once in module_clone to create a local branch, and again in cmd_update to point that branch at the correct commit), which Heiko was not excited about [5]. I'm also not sure if defaulting to $remote_name/$branch in cmd_update is appropriate. cmd_add defaults to using the remote's HEAD, and that makes more sense to me than defaulting to the master branch. However, changing this logic is probably food for another series. I still think that this series is only useful as a temporary stop-gap [6] until we get something like my v3 [7], so the appropriate submodule branch is automatically checked out when you change superproject branches. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/240036 [2]: http://article.gmane.org/gmane.comp.version-control.git/239973 [3]: http://article.gmane.org/gmane.comp.version-control.git/239978 [4]: http://article.gmane.org/gmane.comp.version-control.git/240368 [5]: http://article.gmane.org/gmane.comp.version-control.git/239968 [6]: http://article.gmane.org/gmane.comp.version-control.git/240232 [7]: http://article.gmane.org/gmane.comp.version-control.git/240248 W. Trevor King (6): submodule: Make 'checkout' update_module explicit submodule: Document module_clone arguments in comments submodule: Explicit local branch creation in module_clone t7406: Just-cloned checkouts update to the gitlinked hash with 'reset' t7406: Add explicit tests for head attachement after cloning updates Documentation: Describe 'submodule update' modes in detail Documentation/git-submodule.txt | 36 +- Documentation/gitmodules.txt| 4 ++ git-submodule.sh| 84 + t/t7406-submodule-update.sh | 39 ++- 4 files changed, 121 insertions(+), 42 deletions(-) -- 1.8.5.2.8.g0f6c0d1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v3 3/4] submodule: Teach 'add' about a configurable local-branch
On Wed, Jan 15, 2014 at 01:18:12AM +0100, Francesco Pretto wrote: > I've matured this opinion about "local-branch" some days ago, but I > couldn't join the discussion because I was extremely busy. Hope it's > is still current (and correct). I think the discussion is still open, but actions are postponed until 'checkout --recurse-submodules' lands [1]. > The "local-branch" feature means to my brain the following: I, > maintainer, decide for you, developer, what name should be the > branch you are checking out. The goal is to have “I, your faithful Git command, check out for you, oh wise developer, the superproject branch you requested, along with the local submodule branch associated with that super project branch.” ;) Maybe that would be more clear if the localBranch settings are purely local (stored only in out-of-tree configs), and not contained in the superproject's .gitmodules file? As this series stands, that would just drop step 3 from the lookup chain [2]. That step is below all the local out-of-tree locations though, and I see no non-psychological reason to keep folks from sharing reasonable default names for local branches. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/240420 [2]: http://article.gmane.org/gmane.comp.version-control.git/240251 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 11:19:07PM +0100, Heiko Voigt wrote:
> On Tue, Jan 14, 2014 at 01:42:09PM -0800, W. Trevor King wrote:
> > The “gitlinked commits must be in the subproject's master” rule
> > protects you from blowing stuff away here. You could use rebase- or
> > merge-style integration as well, assuming the maintainer didn't have
> > dirty local work in their submodule.
>
> No we can't. Developers are not allowed to merge in some submodules.
> The most central ones have maintainers and only they are allowed to
> merge into the stable branch. So we need to track exact commits on the
> stable branch, no local merge (except the fast-forward case of course)
> allowed. Thats why the developer does an exact checkout here.
Because both sides of the merge/rebase are required (by your rule) to
be in the subproject's master, you're guaranteed to have a
fast-forward merge/rebase.
> > > We have a tool in our git gui configuration that does
> > >
> > > git submodule foreach 'git fetch && git checkout origin/master'
> >
> > I agree that with 'submodule update' seems superfluous. With proper
> > out-of-tree submodule configs specifying remote URLs and upstream
> > branches,
> >
> > git submodule foreach 'git fetch && git checkout @{upstream}'
> >
> > (or merge/rebase/…) should cover this case more generically and with
> > less mental overhead.
> >
> > > I hope that draws a clear picture of how we use submodules.
> >
> > It's certainly clearer, thanks :). I'm not sure where checkout-mode
> > is specifically important, though. In your step-2, it doesn't restore
> > your original branch. In your “update the superproject's master”
> > step, you aren't even using 'submodule update' :p.
>
> Ah sorry I though that was clear. The "others" are using submodule update ;-)
>
> I mean someone who gets stuff from a stable superproject branch by
> either rebasing their development branch or updating their local copy of
> a stable branch (e.g. master) by using
>
> git checkout master
> git pull --ff --ff-only
> git submodule update --init --recursive
>
> This also prevents the pure "updaters" from creating unnecessary merges.
Right. Folks who don't do local development in their submodules can
get away with checkout-mode updates and their detached HEADs.
Assuming the upstream superproject only advances the gitlinked commits
using fast-forwards, they could equally well use any of the other
update modes, but there is no need for them to make that assumption.
*This* is the use case that I think the current recursive-checkout
facilitates [1]. I don't think it helps folks who are actually doing
submodule development on branches from within their superproject.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/239695
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 10:46:08PM +0100, Heiko Voigt wrote: > I would like to step back a bit and get back to the original problem > at hand: Francescos original use case of an attached head for direct > commits on a stable branch in a submodule. How about we finish > discussing the exact solution of that first. AFAIK that is already > solved with the following: > > * Trevor's first patch[2] to create a branch on initial clone of a submodule v1 broke a bunch of tests. Are you ok with v2 [1]? v2 still needs a clearer commit message, a test, and a possible transition to triggering on non-checkout submodule..update instead of non-empty submodule..branch [2]. > That should be all (and IIRC Francesco agreed) needed for that use-case. That was my understanding [3] ;). > Lets not implement more than currently is needed. We can revisit the > ideas once some other real use-case manifests. I have most of a real use case already. I have a repository with submodules in one branch (master) and a subtree version in another (assembled) [4]. The *tree* is the same in each case, so I have to 'git rm -rf .' to clear the submodules out of master before I can checkout assembled. $ git checkout assembled error: The following untracked working tree files would be overwritten by checkout: modular/README.md modular/shell/README.md … $ git rm -rf . $ git checkout assembled That leaves some extra stuff removed: $ git status On branch assembled Changes to be committed: (use "git reset HEAD ..." to unstage) deleted:.gitignore deleted:.mailmap deleted:CONTRIBUTING.md deleted:LICENSE.md deleted:instructor.md so I need to check that out by hand: $ git reset --hard HEAD Now I can work in the assembled branch. Going back to master is a bit less tedious: $ git checkout master $ git submodule update --recursive Luckily for me, I don't have a third superproject branch where the submodules are on a different, so the submodule's HEADs are preserved. As I understand it, the new recursive checkout functionality [5] would checkout my submodules with detached HEADs. The fact that they are only accidentally preserved now is not comforting ;). > Also we (Jens and I) would first like to proceed with the recursive > checkout / fetch (for which the plan is clear) as the next > complicated step. > > Once that is done and people gain some experience with it we can > still extend further. This is quite reasonable. Given the need for backwards compatibility, I just wanted to make sure my ideal UI was clear before we went forward. There's no need to break fingers twice ;), but if tight binding with localBranch is too big a chunk to bite off now, I'm happy to kick that can down the road. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/239973 [3]: http://article.gmane.org/gmane.comp.version-control.git/240139 [4]: (gitweb) http://git.tremily.us/?p=swc-boot-camp.git [5]: http://article.gmane.org/gmane.comp.version-control.git/239695 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 09:58:30PM +0100, Heiko Voigt wrote:
> A typical workflow where a feature in a project needs some extension or
> change in a submodule goes like this:
>
> 1. The developer does his changes locally implementing everything
>needed. To commit he creates a local branch in the submodule and in
>the superproject (most of the times from the current HEAD that is
>checked out).
>
> 2. For convenience I usually commit the resulting commit sha1 of the
>submodule in the commit that needs the change. That way when I switch
>to a different branch and back I can simply say: git submodule update
>and get the correct code everywhere.
This checkout functionality is exactly what my
submodule..localBranch is designed to automate [1]. I think
that should be different from integrating local and external changes,
which is what 'git submodule update' is about. For example, after you
run 'git submodule update' here, you'll have your original commit
checked out, but you'll be on a detached HEAD instead of your original
branch. If you want to further develop the submodule feature branch,
you currently have to cd into the submodule and check the branch out
by hand.
> How about the use-case I sketched above? Is that what you are searching
> for? In that use-case we have to update to the new master after a
> submodule change was merged. That could be achieved by
>
> git submodule update --remote
>
> with the wanted stable branch configured. But in practise something
> along the lines of
>
> (cd && git checkout origin/)
>
> is usually used and simple enough.
The “gitlinked commits must be in the subproject's master” rule
protects you from blowing stuff away here. You could use rebase- or
merge-style integration as well, assuming the maintainer didn't have
dirty local work in their submodule.
> We have a tool in our git gui configuration that does
>
> git submodule foreach 'git fetch && git checkout origin/master'
I agree that with 'submodule update' seems superfluous. With proper
out-of-tree submodule configs specifying remote URLs and upstream
branches,
git submodule foreach 'git fetch && git checkout @{upstream}'
(or merge/rebase/…) should cover this case more generically and with
less mental overhead.
> I hope that draws a clear picture of how we use submodules.
It's certainly clearer, thanks :). I'm not sure where checkout-mode
is specifically important, though. In your step-2, it doesn't restore
your original branch. In your “update the superproject's master”
step, you aren't even using 'submodule update' :p.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/240336
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 14, 2014 at 11:24:45AM +0100, Heiko Voigt wrote: > On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote: > > Users who are worried about loosing local updates should not be > > using a checkout-style updates. If they are using a > > checkout-style update, and they ask for an update, they're > > specifically requesting that we blow away their local work and > > checkout/reset to the new sha1. Solving update conflicts is the > > whole point of the non-checkout update modes. > > But checkout is different from reset --hard in that it does not blow > away local uncommitted changes. Please see below. Ah, good point. We should definately die if there are local uncommitted changes. > > On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: > > > Am 09.01.2014 20:55, schrieb W. Trevor King: > > > > Maybe you meant "for checkout I can easily overwrite the local > > > > changes with the upstream branch", which is what I understand > > > > checkout to do. > > > > > > But which I find really unfriendly and would not like to see in > > > a new feature. We should protect the user from loosing any local > > > changes, not simply throw them away. Recursive update makes sure > > > it won't overwrite any local modification before it checks out > > > anything and will abort before doing so (unless forced of > > > course). > > > > If you want to get rid of checkout-mode updates, I'm fine with > > that. However, I don't think it supports use-cases like Heiko's > > (implied) “I don't care what's happening upstream, I never touch > > that submodule, just checkout what the superproject maintainer > > says should be checked out for this branch. Even if they have > > been rebasing or whatever” [3]. > > I never stated that in that post. Sorry for misunderstanding. I think I'm just unclear on your workflow? > The recursive checkout Jens is working on is simply implementing the > current checkout-mode to the places where the superproject checks > out its files. That way submodules get checked out when the > superproject is checked out. If the submodule does not match the > sha1 registered in the superproject it either stays there (if the > checkout would not need to update the submodule) or (if checkout > would need to update) git will not do the checkout and bail out with > "you have local modifications to ... . Sounds good to me as far as it goes. I think it misses the “what should we do if the gitlinked hashes are different” case, because the checkout will always leave you with a detached HEAD. > > > or be asked to resolve the conflict manually when "checkout" is > > > configured and the branches diverged. > > > > I still think that checkout-mode updates should be destructive. > > See my paraphrased-version of Heiko's use case above. How are > > they going to resolve this manually? Merge or rebase? Why > > weren't they using that update mode in the first place? > > I strongly disagree. They should only be destructive in the sense > that another commit get checked out but never loose local > modifications. I think the key I'm missing is an example workflow where a developer wants to make local submodule changes, but also wants to use checkout-mode updates instead of merge/rebase updates. Can you sketch one out for me? Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Tight submodule bindings
On Mon, Jan 13, 2014 at 02:13:46PM -0800, Junio C Hamano wrote: > "W. Trevor King" writes: > > > Additional metadata, the initial checkout, and syncing down > > --- > > > > However, folks who do local submodule development will care about > > which submodule commit is responsible for that tree, because > > that's going to be the base of their local development. They also > > care about additional out-of-tree information, including the > > branch that commit is on. > > Well, please step back a bit. > > They do not have to care about what local branch they use to build > follow-up work based on that commit. They do if they want to checkout the banch out again later, before pushing it somewhere public. > In fact, they would want to be able to develop more than one > histories on top, which means more than one branches they can name > themselves. Agreed, bug for each superproject branch they will still have a single submodule branch that should be checked out by default when they checkout that superproject branch. > The only thing they care about is where the result of their > development _goes_, that is the URL and the branch of the remote > they are pushing back to. Maybe they're just doing local development? I think the remote branch(es) you pull from and push to are important, but not the only thing you might care about. > I have a feeling that this is not specific for submodules---if you > did this: > > git init here > cd here > git fetch $there master > git reset --hard FETCH_HEAD > > and are given the resulting working tree to start hacking on, you > would not know where the history came from, or where your result > wants to go. > > So "the branch that commit is on" is a wrong thing to focus on. > "The branch the history built on top of the commit wants to go" may > be closer and these two are different. That makes sense. I don't think the former (as distinct from the latter) is of any interest to anybody. I don't care what the branch name was when the past history was developed. I don't even really care about the new branch name. I do care that checking out a superproject branch gives me the same branch (with pull/push configs, etc.) that I had the last time I was on that superproject branch. > > For already-initialized submodules, there are existing places > > in the submodule config to store this configuration: > > > > 1. HEAD for the checked-out branch, > > 2. branch..remote → remote..url for the upstream > >subproject URL, > > 4. branch..rebase (or pull.rebase) to prefer rebase over merge > >for integration, > > 5. … > > What happened to 3 ;-)? I can't count? :p > In any case, "local-branch" is wrong from two aspects: > > 1. (obvious) It does not follow our naming convention not to use > dashed-names for configuration variables. I'll use localBranch in my mockup ;). Although skimming through config.txt shows a number of alllowercase settings as well as camelCase. > 2. You do not care about the names you use locally. The only thing > you care about is where people meet at the central repository, > i.e. where your result is pushed to. I also care about local-checkout consistency, as described above. > > Syncing up > > -- > > > > In the previous section I explained how data should flow from > > .gitmodules into out-of-tree configs. > > s/should/you think should/, I think, but another way may be not to > copy and read from there, which may be a lot simpler. Then upon > switching branches of top-level superproject (which would update > .gitmodules to the version on the new branch), you may get different > settings automatically. That only works for superproject-level commands that know about the .gitmodules file. If you cd into the submodule and work there directly, your actions will be using the submodule's out-of-tree config. I think most of the time folks will want those out-of-tree configs to match the settings in the superproject's .gitmodules, hence the submodule..sync defaulting to true. > > ... Since you *will* want to share the upstream URL, I proposed > > using an explicit submodule..active setting to store the “do > > I care” information [2], instead of overloading > > submodule..url (I'd auto-sync the .gitmodule's > > submodule..url with the subproject's remote.origin.url > > unless the user opted out of .gitmodules syncing). > > It may not be a good idea to blindly update to whatever happens to > be in .gitmodules, especially once sub
Re: Tight submodule bindings
On Mon, Jan 13, 2014 at 08:37:37PM +0100, Jens Lehmann wrote: > Am 12.01.2014 02:08, schrieb W. Trevor King: > > For folks who treat the submodule as a black box (and do no local > > development), switchable trees are all they care about. They can > > easily checkout (or not, with deinit), the submodule tree at a > > gitlinked hash, and everything is nice and reproducible. The fact > > that 'submod' is stored as a commit object and not a tree, is just > > a convenient marker for optional > > init/deinit/remote-update-integration functionality. > > But there are users (like me) who do not treat submodules as > black boxes and nonetheless do development in them with update > set to checkout (after creating a feature branch of course ;-). I'm still not clear on how this works for you ;). Can you sketch out an example shell history showing how you use checkout updates to do this? > > When you checkout a submodule for the first time, Git should take > > the default information from .gitmodules and file it away in the > > submodule's appropriate out-of-tree config locations. > > I disagree, that only makes sense for the URL setting (and this > currently only happens with the update setting, which I intend to > change). Everything else should be taken from .gitmodules unless > the user wants to override it. The only setting I'm not so sure > about is the local branch setting, as that might have to propagate > into the submodule. I think copying into the submodule's out-of-tree config is the way to go, because users won't always be driving the submodule from the superproject. If the settings are in the submodule's out-of-tree config, everything will be consistent betwee stuff run from the superproject and stuff run from submodule itself. It also allows us to use familiar configuration commands inside the submodule, and have those automatically mapped back into the .gitmodules file for us. > > In fact, I think life is easier for everyone if this is the > > default, and we add a new option (submodule..sync = false) > > that says “don't overwrite optional settings in my submodule's > > out-of-tree config on checkout” for for folks who want to opt out. > > Don't worry, this is not going to clobber people, because we'll be > > syncing the other way too. > > Yet another flag to make peoples life easier? I don't think so ;-) I'm fine if there is no opt-out, and the syncing is mandatory, but I imagine that folks who want a local (unshared, not in .gitmodules) URL would complain. > > Purely local metadata > > - > > > > Some metadata does not make sense in the superproject tree. For > > example, whether a submodule is interesting enough to checkout > > (init/deinit) or whether you want to auto-sync optional metadata > > .gitmodules defaults. This metadata should live in the > > superproject's out-of-tree config, and should not be stored in the > > in-tree .gitmodules. > > Not always. It makes a lot of sense to let upstream mark a > submodule as "too big and you won't need it anyway" in the > .gitmodules file. Good. Then there's no need for this special class of settings. > > Since you *will* want to share the upstream URL, I proposed using > > an explicit submodule..active setting to store the “do I > > care” information [2], instead of overloading submodule..url > > (I'd auto-sync the .gitmodule's submodule..url with the > > subproject's remote.origin.url unless the user opted out of > > .gitmodules syncing). > > That is wrong as it would break horribly when you check out an old > commit with a now dead submodule URL and that gets automatically > synced. If you've already checked out the submodule with a current URL, you should already have the old commit locally, and Git will use it without trying to re-fetch from the broken old URL. > > Subsequent checkouts > > > > > > Now that we have strict linking between the submodule state (both > > in-tree and out-of-tree configs) and the superproject tree (gitlink > > and .gitmodules), changing between superproject branches is really > > easy: > > > > 1. Make sure the working tree is not dirty. If it is, ask the user to > >either add-and-commit or stash, and then die to let them do so. > > This condition is too hard, relax that to "a trivial merge can > switch from current state to target state" and make it behave just > like branch switching in the superproject. After all submodules > should behave as much as possible like content of the superproject. Sounds
Tight submodule bindings (was: Preferred local submodule branches)
On Wed, Jan 08, 2014 at 10:17:51PM -0800, W. Trevor King wrote: > In another branch of the submodule thread Francesco kicked off, I > mentioned that we could store the preferred local submodule branch on > a per-superbranch level if we used the > .git/modules//config for local overrides [1]. Here's > a patch series that greatly extends my v2 "submodule: Respect > requested branch on all clones" series [2] to also support automatic, > recursive submodule checkouts, as I outlined here [3]. > > [1]: http://article.gmane.org/gmane.comp.version-control.git/240240 > [2]: http://article.gmane.org/gmane.comp.version-control.git/239967 > [3]: http://article.gmane.org/gmane.comp.version-control.git/240192 While mulling over better ways to explain my local-branch idea, I've come up with a more tightly bound model that may help break the silence that has greeted the “Preferred local submodule branches” series ;). That series doesn't have strong options on update mechanics, which leads to wishy-washy exchanges where nobody has a clear mental picture: On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: > Am 09.01.2014 20:55, schrieb W. Trevor King: > > On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: > >> Am 09.01.2014 18:32, schrieb W. Trevor King: > >>>> when superproject branches are merged (with and without conflicts), > >>> > >>> I don't think this currently does anything to the submodule itself, > >>> and that makes sense to me (use 'submodule update' or my 'submodule > >>> checkout' if you want such effects). We should keep the current logic > >>> for updating the gitlinked $sha. In the case that the > >>> .gitmodule-configured local-branches disagree, we should give the > >>> usual conflict warning (and <<<===>>> markup) and let the user resolve > >>> the conflict in the usual way. > >> > >> For me it makes lots of sense that in recursive checkout mode the > >> merged submodules are already checked out (if possible) right after > >> a superproject merge, making another "submodule update" unnecessary > >> (the whole point of recursive update is to make "submodule update" > >> obsolete, except for "--remote"). > > > > If you force the user to have the configured local-branch checked out > > before a non-checkout operations with checkout side-effects (as we > > currently do for other kinds of dirty trees), I think you'll avoid > > most (all?) of the branch-clobbering problems. > > I'm thinking that a local branch works in two directions: It should > make it easy to follow an upstream branch and also make changes to it > (and publish those) if necessary. But neither local nor upstream > changes take precedence, so the user should either use "merge" or > "rebase" as update strategy or be asked to resolve the conflict > manually when "checkout" is configured and the branches diverged. > Does that make sense? The current series is only weakly bound (you can explicitly call git submodule checkout' to change to the preferred local submodule branch), and the current Git is extremely weakly bound (you have to cd into the submodule and change branches by hand). The following extrapolates the “Preferred local submodule branches” series to a tightly-bound ideal. Gitlinked commit hash - The submodule model revolves around links to commits (“gitlinks”): $ git ls-tree HEAD 100644 blob 189fc359d3dc1ed5019b9834b93f0dfb49c5851f.gitmodules 16 commit fbfa124c29362f180026bf0074630e8bd0ff4550 submod These are effectively switchable trees. The tree referenced by commit fbfa124 is 492781c: $ (cd submod/ && git cat-file commit fbfa124) tree 492781c581d4dec380a61ef5ec69a104de448a74 … If you init the submodule, subsequent checkouts will check out that tree, just like 'git checkout' would do if you'd had a superproject tree like: $ git ls-tree HEAD 100644 blob 189fc359d3dc1ed5019b9834b93f0dfb49c5851f.gitmodules 04 tree 492781c581d4dec380a61ef5ec69a104de448a74submod For folks who treat the submodule as a black box (and do no local development), switchable trees are all they care about. They can easily checkout (or not, with deinit), the submodule tree at a gitlinked hash, and everything is nice and reproducible. The fact that 'submod' is stored as a commit object and not a tree, is just a convenient marker for optional init/deinit/remote-update-integration functionality. Additional metadata, the initial checkout, and syncing down --- Howev
Re: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote: > Am 09.01.2014 20:55, schrieb W. Trevor King: > > On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: > >> Am 09.01.2014 18:32, schrieb W. Trevor King: > >>> However, the local-branch setting needs to be both > >>> per-submodule and per-superproject-branch, so .git/config doesn't work > >>> very well. I think it's better to use something like my > >>> .git/modules//config implementation [1] to set this > >>> override. > >> > >> Yes, the local branch should be set in the submodule's .git/config > >> to make operations done inside the submodule work seamlessly. > > > > Once you're inside the submodule my local-branch setting shouldn't > > matter, because it just connects superproject branches with submodule > > branches. The submodule's config is just a convenient out-of-tree > > place to store per-submodule overrides. > > Now I get it, you want to be able to override a submodule branch for > every superproject branch. I'm not sure I'd add that in the first > iteration though, as it seems to add quite some complexity and I'm > not convinced yet users really need it (but I won't object when we > find real world use cases for that). Not much complexity in the code, it's all in the first patch of my v3 series [1]. Adding a new override location doesn't seem that complicated to me, but I haven't been very successful at getting this idea across, so maybe it's weirder than I think ;). Clearer explanations welcome ;). > >> And it isn't a "per-superproject-branch override" but a > >> "per-superproject-branch default" which can be overridden in > >> .git/config (except for 'update', but I intend to fix that). > > > > You're talking about .gitmodules vs. .git/config here, but for > > local-branch, I'm talking about a fallback chain like [1]: > > > > 1. superproject..local-branch in the submodule's > >config (superproject/.git/modules/≤submodule-name>/config). > > 2. submodule..local-branch in the superproject's > >config (.git/config). > > 3. submodule..local-branch in the superproject's > >.gitmodules file. > > 4. default to 'master' > > > > Only #1 is a new idea. > > Thanks for the explanation, now I understand what you're aiming at. For additional clarity, my whole v3 series is not super long [2]… ;) > >>> On the other hand, maybe an in-tree .gitmodules is good enough, > >>> and folks who want a local override can just edit .gitmodules in > >>> their local branch? I've never felt the need to override > >>> .gitmodules myself (for any setting), so feedback from someone > >>> who has would be useful. > >> > >> That way these changes would propagate to others working on the > >> same branch when pushing, which I believe is a feature. > > > > Sure. Unless they don't want to propagate them, at which point > > they use an out-of-tree override masking the .gitmodules value. > > The question is, would folks want local overrides for local-branch > > (like they do for submodule..update), or not? Since it's > > easy to do [1], I don't see the point of *not* supporting > > per-superproject-branch overrides. > > Unless actual use cases are shown I'd vote for YAGNI here. A new > config option means considerable maintenance burden, no matter how > easy it is to implement in the first place. Automatically checking out the preferred submodule branch for a given superproject branch already requires a new config option. The per-superproject-branch out-of-tree override just renames it (from submodule..local-branch to superproject..local-branch). So different names depending on superproject-level or submodule-level config, but still the same option. That doesn't sound like it's adding that much of a maintenance burden. On the other hand, I, personally, have no need for out-of-tree overrides for *any* submodule-related config, so I'm fine if we drop the submodule-level lookup location ;). > > I'm all for rolling my 'git submodule checkout' into 'git checkout > > --recurse-submodules' [2]. It was just faster to mock up in shell > > while we decide how it should work. > > Sure. As I said that's perfectly fine for testing this approach, > but we should do that right in "git checkout" and friends and not > add yet another submodule command. The current C code looked fairly fo
Re: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote: > Am 09.01.2014 18:32, schrieb W. Trevor King: > > However, the local-branch setting needs to be both > > per-submodule and per-superproject-branch, so .git/config doesn't work > > very well. I think it's better to use something like my > > .git/modules//config implementation [1] to set this > > override. > > Yes, the local branch should be set in the submodule's .git/config > to make operations done inside the submodule work seamlessly. Once you're inside the submodule my local-branch setting shouldn't matter, because it just connects superproject branches with submodule branches. The submodule's config is just a convenient out-of-tree place to store per-submodule overrides. > > This lack of per-superproject-branch overrides applies to all of the > > submodule..* settings, but you're unlikely to want an > > out-of-tree override for 'path' or a per-superproject-branch override > > for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. > > Unlikely it is not ;-) We do have people who set update=none in > the .git/config of the superproject for submodules they don't have > access to (and which is not necessary for their work). That is not a per-superproject-branch override. local-branch is the only per-submodule config I can think of where I can imagine a sane person actually wanting an out-of-tree per-superproject-branch override. > And it isn't a "per-superproject-branch override" but a > "per-superproject-branch default" which can be overridden in > .git/config (except for 'update', but I intend to fix that). You're talking about .gitmodules vs. .git/config here, but for local-branch, I'm talking about a fallback chain like [1]: 1. superproject..local-branch in the submodule's config (superproject/.git/modules/≤submodule-name>/config). 2. submodule..local-branch in the superproject's config (.git/config). 3. submodule..local-branch in the superproject's .gitmodules file. 4. default to 'master' Only #1 is a new idea. > > On the other hand, maybe an in-tree .gitmodules is good enough, > > and folks who want a local override can just edit .gitmodules in > > their local branch? I've never felt the need to override > > .gitmodules myself (for any setting), so feedback from someone who > > has would be useful. > > That way these changes would propagate to others working on the same > branch when pushing, which I believe is a feature. Sure. Unless they don't want to propagate them, at which point they use an out-of-tree override masking the .gitmodules value. The question is, would folks want local overrides for local-branch (like they do for submodule..update), or not? Since it's easy to do [1], I don't see the point of *not* supporting per-superproject-branch overrides. > >> It have the impression that attaching the head to the given > >> branch for merge and rebase might be the sensible thing to do, > >> but it would be great to hear from users of merge and rebase if > >> that would break anything for them in their current use cases for > >> these settings. > > > > Which local branch would you attach to before merging? I think > > 'git submodule update' should always use the current submodule > > state (attached branch or detached HEAD) [3], and we should have a > > separate call that explicitly checked out the desired submodule > > branch [4]. > > Like we currently do with "git submodule update --remote" (where you > have to have an explicit command telling git when to advance the > branch)? Having a separate call that does something *after* a git > command is exactly the problem I'm trying to fix with recursive > update, so I'm not terribly excited ;-) I'm all for rolling my 'git submodule checkout' into 'git checkout --recurse-submodules' [2]. It was just faster to mock up in shell while we decide how it should work. > >>> If it's not the first clone, you should take no action (and your > >>> original patch was ok about this). > >> > >> I'm not sure this is the right thing to do, after all you > >> configured git to follow that branch so I'd expect it to be > >> updated later too, no? Otherwise you might end up with an old > >> version of your branch while upstream is a zillion commits > >> ahead. > > > > Non-clone updates should not change the submodule's *local* branch > > *name*. They should change the commit that that branch refere
Re: [RFC v2] submodule: Respect requested branch on all clones
On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote: > Am 09.01.2014 02:09, schrieb Francesco Pretto: > > 2014/1/9 W. Trevor King : > >> > >> However, submodule..local-branch has nothing to do with remote > >> repositories or tracking branches. > > > > My bad: this means the feature is still not entirely clear to me. > > > >> > >> [branch "my-feature"] > >> remote = origin > >> merge = refs/heads/my-feature > >> [submodule "submod"] > >> local-branch = "my-feature" > >> > >> and I don't think Git's config supports such nesting. > >> > > > > Aesthetically, It doesn't look very nice. > > And I'm not sure we even need that. What's wrong with having the > branch setting in the .gitmodules file of the my-feature branch? > The only problem I can imagine is accidentally merging that into > a branch where that isn't set, but that could be solved by a merge > helper for the .gitmodules file. .gitmodules is fine so long as the config can live in the versioned tree. Many (all?) .gitmodules settings can be overridden in .git/config. However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules//config implementation [1] to set this override. This lack of per-superproject-branch overrides applies to all of the submodule..* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Maybe folks would want per-superproject-branch overrides to 'branch', although I'd prefer we reuse branch..merge in the submodule's config for that [2]. On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful. > >> I can resuscitate that if folks want, but Heiko felt that my initial > >> consolidation didn't go far enough [2]. If it turns out that we're ok > >> with the current level of consolidation, would you be ok with > >> "non-checkout submodule..update" as the trigger [3]? > > > > For me it was ok with what you did: > > - > > if "just_cloned" and "config_branch" > > then > > !git reset --hard -q" > > fi > > - > > > > So yes: at the first clone 'checkout' keeps attached HEAD, while > > 'merge' and 'rebase' attach to the branch. > > It have the impression that attaching the head to the given branch > for merge and rebase might be the sensible thing to do, but it > would be great to hear from users of merge and rebase if that > would break anything for them in their current use cases for these > settings. Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4]. > > If it's not the first clone, you should take no action (and your > > original patch was ok about this). > > I'm not sure this is the right thing to do, after all you > configured git to follow that branch so I'd expect it to be > updated later too, no? Otherwise you might end up with an old > version of your branch while upstream is a zillion commits > ahead. Non-clone updates should not change the submodule's *local* branch *name*. They should change the commit that that branch references, otherwise 'git submodule update' would be a no-op ;). > First I'd like to see a real consensus about what exactly should > happen when a branch is configured to be checked out (and if I > missed such a summary in this thread, please point me to it ;-). I don't think we have a consensus yet. A stand-alone outline of my current position is in my v3 RFC [5], but I don't have any buy-in yet ;). > And we should contrast that to the exact checkout and floating > branch use cases. With my v3 series, there are no more detached HEADs. Folks using checkout updates get a local master branch. I do not change any of the exact checkout (superproject gitlinked sha1) vs. floati
[RFC v3 2/4] submodule: Teach 'update' to preserve local branches
From: "W. Trevor King" There's no sense in setting up a local branch if we're just going to go back to a detached HEAD with every checkout-mode update. This commit replaces the checkout with a reset, updating whatever the locally checked out branch (or detached HEAD) happens to be. While it is tempting to checkout a new local-branch here (as we did after the clone), it's more consistent to follow the lead of the other update modes and just use the currently checked out branch. --- git-submodule.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 56fc3f1..c5ea7bd 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -930,9 +930,9 @@ Maybe you want to use 'update --init'?")" must_die_on_failure=yes ;; *) - command="git checkout $subforce -q" - die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")" - say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")" + command="git reset --hard -q" + die_msg="$(eval_gettext "Unable to reset branch to '\$sha1' in submodule path '\$displaypath'")" + say_msg="$(eval_gettext "Submodule path '\$displaypath': reset branch to '\$sha1'")" ;; esac -- 1.8.5.2.237.g01c62c6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
