* [PATCH v2 4/4] show-ref: Inline show_one
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
As the small function is now only called from a single place, there is
no point in keeping it as a separate function any longer.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 3cf344d47..ec44164d8 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -17,15 +17,6 @@ static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
static const char **pattern;
static const char *exclude_existing_arg;
-static void show_one(const char *refname, const struct object_id *oid)
-{
- const char *hex = find_unique_abbrev(oid->hash, abbrev);
- if (hash_only)
- printf("%s\n", hex);
- else
- printf("%s %s\n", hex, refname);
-}
-
static int show_ref(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
{
@@ -74,7 +65,11 @@ static int show_ref(const char *refname, const struct object_id *oid,
if (quiet)
return 0;
- show_one(refname, oid);
+ hex = find_unique_abbrev(oid->hash, abbrev);
+ if (hash_only)
+ printf("%s\n", hex);
+ else
+ printf("%s %s\n", hex, refname);
if (!deref_tags)
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 3/4] show-ref: Optimize show_ref a bit
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
The inner `if (verify)' check was not being used before the preceding
commit, as show_ref was never being called from a code path where
verify was non-zero.
Adding a `!verify' check to the outer if skips an unnecessary string
comparison when verify is non-zero, and show_ref is already called
with a reference exactly matching pattern.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index bcdc1a95e..3cf344d47 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -43,7 +43,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
if (!match)
return 0;
}
- if (pattern) {
+ if (pattern && !verify) {
int reflen = strlen(refname);
const char **p = pattern, *m;
while ((m = *p++) != NULL) {
@@ -54,9 +54,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
continue;
if (len == reflen)
goto match;
- /* "--verify" requires an exact match */
- if (verify)
- continue;
if (refname[reflen - len - 1] == '/')
goto match;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/4] show-ref: Allow --head to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
Previously, when --verify was specified, show-ref would use a separate
code path which ignored --head. Thus, "git show-ref HEAD" used with
"--verify" (because the user is not interested in seeing
refs/remotes/origin/HEAD), and used with "--head" (because the user
does not want HEAD to be filtered out), i.e. "git show-ref --head
--verify HEAD", did not work as expected.
Instead of insisting that the input begins with "refs/", allow "HEAD"
when "--head" is given in the codepath that handles "--verify", so
that all valid full refnames including HEAD are passed to the same
output machinery.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 3 ++-
t/t1403-show-ref.sh | 14 ++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e66900..945a483e3 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
while (*pattern) {
struct object_id oid;
- if (starts_with(*pattern, "refs/") &&
+ if ((starts_with(*pattern, "refs/") ||
+ (show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, &oid);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 7e10bcfe3..2fb5dc879 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -164,4 +164,18 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' '
test_cmp expect actual
'
+test_expect_success 'show-ref --verify --head' '
+ echo $(git rev-parse HEAD) HEAD >expect &&
+ git show-ref --verify --head HEAD >actual &&
+ test_cmp expect actual &&
+
+ >expect &&
+
+ git show-ref --verify --head -q HEAD >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify --head -q A >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/4] show-ref: Allow -d to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170121010821.25046-1-git@thecybershadow.net>
Use the same output machinery when --verify is absent or present,
which allows tag dereferencing (-d) to work with --verify. This is
useful when the user wishes to avoid the costly iteration of refs.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 3 +--
t/t1403-show-ref.sh | 9 +++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 945a483e3..bcdc1a95e 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if ((starts_with(*pattern, "refs/") ||
(show_head && !strcmp(*pattern, "HEAD"))) &&
!read_ref(*pattern, oid.hash)) {
- if (!quiet)
- show_one(*pattern, &oid);
+ show_ref(*pattern, &oid, 0, NULL);
}
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 2fb5dc879..5c540e67f 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -97,6 +97,9 @@ test_expect_success 'show-ref -d' '
git show-ref -d refs/tags/A refs/tags/C >actual &&
test_cmp expect actual &&
+ git show-ref --verify -d refs/tags/A refs/tags/C >actual &&
+ test_cmp expect actual &&
+
echo $(git rev-parse refs/heads/master) refs/heads/master >expect &&
git show-ref -d master >actual &&
test_cmp expect actual &&
@@ -116,6 +119,12 @@ test_expect_success 'show-ref -d' '
test_cmp expect actual &&
test_must_fail git show-ref -d --verify heads/master >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d A C >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d tags/A tags/C >actual &&
test_cmp expect actual
'
--
2.11.0
^ permalink raw reply related
* [PATCH v2] show-ref: Allow -d, --head to work with --verify
From: Vladimir Panteleev @ 2017-01-21 1:08 UTC (permalink / raw)
To: git
This is the second take for "[PATCH] show-ref: Allow --head to work
with --verify". Thanks to Junio for his extensive feedback.
^ permalink raw reply
* Does it make sense to show tips on blame?
From: Edmundo Carmona Antoranz @ 2017-01-21 5:23 UTC (permalink / raw)
To: Git List
Hi!
I'm having fun with difflame. I added support for an option called
'tips' which would display 1-line summary of a block of added lines
that belong to the same revision, in order to provide context while
reading difflame output without having to run an additional git show
command.
Output is like this (from README.txt, taken from difflame on git
itself, sorry if it's too wide):
diff --git a/fast-import.c b/fast-import.c
index f561ba833..64fe602f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)
char *fullpath, unsigned int fullpath_len,
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)
unsigned char fanout)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {
-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221) struct
tree_content *t = root->tree;
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2221) struct
tree_content *t;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2222) struct
tree_entry *e, leaf;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)
unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)
uintmax_t num_notes = 0;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)
unsigned char sha1[20];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226) char
realpath[60];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2228) if (!root->tree)
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2229)
load_tree(root);
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2230) t = root->tree;
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2231)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232) for (i
= 0; t && i < t->entry_count; i++) {
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)
e = t->entries[i];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)
tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
The tip that was used for revision 405d7f4af on both "blocks" of added
lines can be seen.
The question is if you think it makes sense to add this option to
git-blame itself.
Something like:
git blame --tips README.txt
Thanks in advance
^ permalink raw reply related
* [PATCH] rebase: pass --signoff option to git am
From: Giuseppe Bilotta @ 2017-01-21 10:49 UTC (permalink / raw)
To: git; +Cc: Giuseppe Bilotta
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Documentation/git-rebase.txt | 5 +++++
git-rebase.sh | 3 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e6883..e6f0b93337 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -385,6 +385,11 @@ have the long commit hash prepended to the format.
Recreate merge commits instead of flattening the history by replaying
commits a merge commit introduces. Merge conflict resolutions or manual
amendments to merge commits are not preserved.
+
+--signoff::
+ This flag is passed to 'git am' to sign off all the rebased
+ commits (see linkgit:git-am[1]).
+
+
This uses the `--interactive` machinery internally, but combining it
with the `--interactive` option explicitly is generally not a good
diff --git a/git-rebase.sh b/git-rebase.sh
index 48d7c5ded4..e468a061f9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -34,6 +34,7 @@
autosquash move commits that begin with squash!/fixup! under -i
committer-date-is-author-date! passed to 'git am'
ignore-date! passed to 'git am'
+signoff! passed to 'git am'
whitespace=! passed to 'git apply'
ignore-whitespace! passed to 'git apply'
C=! passed to 'git apply'
@@ -321,7 +322,7 @@ run_pre_rebase_hook ()
--ignore-whitespace)
git_am_opt="$git_am_opt $1"
;;
- --committer-date-is-author-date|--ignore-date)
+ --committer-date-is-author-date|--ignore-date|--signoff)
git_am_opt="$git_am_opt $1"
force_rebase=t
;;
--
2.11.0.585.g56041942c3.dirty
^ permalink raw reply related
* [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: George Vanburgh @ 2017-01-21 12:02 UTC (permalink / raw)
To: git
From: George Vanburgh <gvanburgh@bloomberg.net>
When running git-p4 on Windows, with multiple git-p4.mapUser entries in
git config - no user mappings are applied to the generated repository.
Reproduction Steps:
1. Add multiple git-p4.mapUser entries to git config on a Windows
machine
2. Attempt to clone a p4 repository
None of the user mappings will be applied.
This issue is caused by the fact that gitConfigList,
uses split(os.linesep) to convert the output of
git config --get-all into a list.
On Windows, os.linesep is equal to '\r\n' - however git.exe
returns configuration with a line seperator of '\n'. This leads to
the list returned by gitConfigList containing only one element - which
contains the full output of git config --get-all in string form. This
causes problems for the code introduced to getUserMapFromPerforceServer
in 10d08a1.
This issue should be caught by the test introduced in 10d08a1, and
would require running on Windows to reproduce. When running inside
MinGW/Cygwin, however, os.linesep correctly returns '\n', and everything
works as expected.
The simplest fix for this issue would be to convert the line split logic
inside gitConfigList to use splitlines(), which splits on any standard
line delimiter. However, this function was only introduced in Python
2.7, and would mean a bump in the minimum required version of Python
required to run git-p4. The alternative fix, implemented here, is to use
'\n' as a delimiter, which git.exe appears to output consistently on
Windows anyway.
Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index f427bf6..c134a58 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -656,7 +656,7 @@ def gitConfigInt(key):
def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
- _gitConfig[key] = s.strip().split(os.linesep)
+ _gitConfig[key] = s.strip().split("\n")
if _gitConfig[key] == ['']:
_gitConfig[key] = []
return _gitConfig[key]
--
https://github.com/git/git/pull/319
^ permalink raw reply related
* Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard
From: Duy Nguyen @ 2017-01-21 12:44 UTC (permalink / raw)
To: Jeff King, Jacob Keller; +Cc: Git mailing list
In-Reply-To: <20170120192539.7jts6xqzx46unn7y@sigill.intra.peff.net>
On Sat, Jan 21, 2017 at 2:25 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 20, 2017 at 11:16:15AM -0800, Jacob Keller wrote:
>
>> > I recently taught urxvt to recognize sha1s and grab them via keyboard
>> > hints, and I'm finding it quite useful. Here's what it looks like if
>> > you're interested:
>> >
>> > http://peff.net/git-hints.gif
>> >
>> > The hints technique is taken from pentadactyl (which I also use), but
>> > the urxvt port is mine. I'm happy to share the code.
You just gave me a reason to rebuild urxvt with perl support. It
solves my problem with SHA-1 nicely (and fixes another problem with
very large counter in my approach, when you scroll git-log further
down, because I can't know what's on screen or already scrolled past).
Do you think it should be in contrib for better discoverability (than
burying it in the mail archive)? A separate git repo is also ok, but I
guess that's just more burden.
>> I would be interested in the code for this.. I'm curious if I can
>> adapt it to my use of tmux.
Yeah. Let me know if it works out. Would be nice to have this run on
basically any terminal, as long as I run tmux on top.
--
Duy
^ permalink raw reply
* Re: [PATCH] log: new option decorate reflog of remote refs
From: Duy Nguyen @ 2017-01-21 12:48 UTC (permalink / raw)
To: Jacob Keller, Jeff King; +Cc: Git Mailing List
In-Reply-To: <CA+P7+xqgyUW-Wt2oUSCc86HG-MfL-itAu2yVrZ219LNwqQnwKw@mail.gmail.com>
On Sat, Jan 21, 2017 at 5:00 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Fri, Jan 20, 2017 at 6:30 AM, Jeff King <peff@peff.net> wrote:
>>> Imposing order between options could cause confusion, I think, if you
>>> remove --decorate-reflog leaving --remotes on by accident, now you get
>>> --remotes with a new meaning. We could go with something like
>>> --decodate-reflog=remote, but that clashes with the number of reflog
>>> entries and we may need a separator, like --decorate-reflog=remote,3.
>>> Or we could add something to --decorate= in addition to
>>> short|full|auto|no. Something like --decorate=full,reflog or
>>> --decorate=full,reflog=remote,entries=3 if I want 3 reflog entries.
>>
>> I agree that making option-order important is potentially confusing. But
>> it does already exist with --exclude. It's necessary to specify some
>> sets of refs (e.g., all of A, except for those that match B, and then
>> all of C, including those that match B).
>>
>> Having --decorate-reflog=remote would be similarly constrained. You
>> couldn't do "decorate all remotes except for these ones". For that
>> matter, I'm not sure how you would do "decorate just the refs from
>> origin".
>>
>> I'll grant that those are going to be a lot less common than just "all
>> the remotes" (or all the tags, or whatever). I'd just hate to see us
>> revisiting this in a year to generalize it, and being stuck with
>> historical baggage.
>>
>>> My hesitant to go that far is because I suspect decorating reflog
>>> won't be helpful for non-remotes. But I'm willing to make more changes
>>> if it opens door to master.
>>
>> Forgetting reflogs for a moment, I'd actually find it useful to just
>> decorate tags and local branches, but not remotes. But right now there
>> isn't any way to select which refs are worthy of decoration (reflog or
>> not).
>>
>> That's why I'm thinking so much about a general ref-selection system. I
>> agree the "--exclude=... --remotes" thing is complicated, but it's also
>> the ref-selection system we _already_ have, which to me is a slight
>> point in its favor.
>>
>> -Peff
>
> I agree that the interaction between --exclude and --remotes/etc is
> confusing, but I think it's reasonable enough because we already
> support it, so it makes sense to extend it with this. I also think its
> better to extend here than it is to hard-code it.
OK. Next question, how do we deal with the reflog count (i..e the
argument of --decorate-remote-reflog). Should it be shared for all ref
type, or can be specified differently for remote, local and tags? I'm
leaning towards the former. But I'll wait a bit for ideas before
rewriting the patch.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH] Disallow commands from within unpopulated submodules.
From: Duy Nguyen @ 2017-01-21 12:55 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Beller, Git Mailing List
In-Reply-To: <20170120191728.l3ne5tt5pwbmafjh@sigill.intra.peff.net>
On Sat, Jan 21, 2017 at 2:17 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Jan 19, 2017 at 11:30:23AM -0800, Stefan Beller wrote:
>
>> Now let's ask the same question for "git -C sub status ." (which is a
>> command that is only reading and not writing to the repository)
>>
>> 1) If the submodule is populated, the user clearly intended to know
>> more about the submodules status
>> 2) It is unclear if the user wanted to learn about the submodules state
>> (So ideally: "The submodule 'sub' is not initialized. To init ...")
>> or the status check should be applied to the superproject instead.
>>
>> Avoid the confusion in 2) as well and just error out for now. Later on
>> we may want to add another flag to git.c to allow commands to be run
>> inside unpopulated submodules and each command reacts appropriately.
>
> I like the general idea of catching commands in unpopulated submodules,
> but I'm somewhat uncomfortable with putting an unconditional check into
> git.c, for two reasons:
>
> 1. Reading the index can be expensive. You would not want "git
> rev-parse" to incur this cost.
>
> 2. How does this interact with commands which do interact with the
> index? Don't they expect to find the_index unpopulated?
>
> (I notice that it's effectively tied to RUN_SETUP, which is good.
> But that also means that many commands, like "diff", won't get the
> benefit. Not to mention non-builtins).
>
> I'd rather see it in the commands themselves. Especially given the
> "ideal" in your status example, which requires command-specific
> knowledge.
I agree. It's already bad enough for pathspec code to peek into the
index, adding a hidden dependency between parse_pathspec() and
read_cache(). And I still think parse_pathspec() is not the right
place to check submodule paths. Worktree should be checked as well, in
the case that the submodule is not yet registered in the index. The
right place to do that is per-command, with their consent so to speak,
because they may need to set things up (index, .git/config and stuff)
properly before explicitly doing this check.
--
Duy
^ permalink raw reply
* [RFC PATCH] Option to allow cherry-pick to skip empty commits
From: Giuseppe Bilotta @ 2017-01-21 13:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Giuseppe Bilotta
This allows cherry-picking a set of commits, some of which may be
redundant, without stopping to ask for the user intervention.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
Documentation/git-cherry-pick.txt | 4 ++++
builtin/revert.c | 1 +
sequencer.c | 45 +++++++++++++++++++++++++++++++--------
sequencer.h | 1 +
4 files changed, 42 insertions(+), 9 deletions(-)
I would like to add to cherry-pick the ability to skip patches. To this
end, I'm working on two options: a general `--skip-empty` option to
handle redundant and empty commits by simply skipping them (no user
intervention), and a `--skip` option as an alternative form to
`--continue` to skip the ongoing (conflicting or empty) commit.
The patch here presents my implementation of the `--skip-empty` option,
including documentation. Comments welcome.
However, it has been some time since I dwelved into git internals and
I'm having issues with the `--skip` option. My idea was to rely on the
existing implementation for `--continue`, and have it run the rollback
instead of the commit. However, I'm finding that after the rollback is
done (I'm using the existing rollback function in sequencer, which calls
`git reset --merge`, but I've also tried a `reset --hard`) the sequencer
does not continue because e.g. if the cherry-pick had stopped due to a
conflict, it still finds unmerged paths, and I can't find a way to tell
the sequencer to just re-read the index from scratch. Opinions welcome
(I can provide my WIP patch to make discussion easier).
On the same note, I've noticed that `git reset` clears CHERRY_PICK_HEAD.
I find this a little confusing, both for humans for the code itself
(since it doesn't really abort an ongoing cherry-pick, yet the sequencer
will think there is no running cherry-pick). This is particularly bad
because `git reset` is one of the strategies proposed to solve conflicts
in cherry-picking. So I was wondering if this was intentional or a side
effect of something else that I might look into cleaning up during this
patch series.
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d35d771fc8..ffced816d6 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -138,6 +138,10 @@ effect to your index in a row.
examine the commit. This option overrides that behavior and
creates an empty commit object. Implies `--allow-empty`.
+--skip-empty::
+ This option causes empty and redundant cherry-picked commits to
+ be skipped without requesting the user intervention.
+
--strategy=<strategy>::
Use the given merge strategy. Should only be used once.
See the MERGE STRATEGIES section in linkgit:git-merge[1]
diff --git a/builtin/revert.c b/builtin/revert.c
index 4ca5b51544..ffdd367f99 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+ OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip redundant, empty commits")),
OPT_END(),
};
options = parse_options_concat(options, cp_extra);
diff --git a/sequencer.c b/sequencer.c
index 9adb7bbf1d..72962fd2fa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -550,22 +550,32 @@ static int is_original_commit_empty(struct commit *commit)
/*
* Do we run "git commit" with "--allow-empty"?
+ *
+ * Or do we just skip this empty commit?
+ *
+ * Returns 1 if a commit should be done with --allow-empty,
+ * 0 if a commit should be done without --allow-empty,
+ * 2 if no commit should be done at all (skip empty commit)
+ * negative values in case of error
+ *
*/
-static int allow_empty(struct replay_opts *opts, struct commit *commit)
+static int allow_or_skip_empty(struct replay_opts *opts, struct commit *commit)
{
int index_unchanged, empty_commit;
/*
- * Three cases:
+ * Four cases:
*
- * (1) we do not allow empty at all and error out.
+ * (1) we do not allow empty at all and error out;
*
- * (2) we allow ones that were initially empty, but
+ * (2) we skip empty commits altogether;
+ *
+ * (3) we allow ones that were initially empty, but
* forbid the ones that become empty;
*
- * (3) we allow both.
+ * (4) we allow both.
*/
- if (!opts->allow_empty)
+ if (!opts->allow_empty && !opts->skip_empty)
return 0; /* let "git commit" barf as necessary */
index_unchanged = is_index_unchanged();
@@ -574,6 +584,9 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
if (!index_unchanged)
return 0; /* we do not have to say --allow-empty */
+ if (opts->skip_empty)
+ return 2;
+
if (opts->keep_redundant_commits)
return 1;
@@ -612,7 +625,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
- int res, unborn = 0, allow;
+ int res = 0, unborn = 0, allow;
if (opts->no_commit) {
/*
@@ -771,12 +784,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
goto leave;
}
- allow = allow_empty(opts, commit);
+ allow = allow_or_skip_empty(opts, commit);
if (allow < 0) {
res = allow;
goto leave;
}
- if (!opts->no_commit)
+ /* allow == 2 means skip this commit */
+ if (allow != 2 && !opts->no_commit)
res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
opts, allow, opts->edit, 0, 0);
@@ -983,6 +997,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
opts->signoff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.record-origin"))
opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.skip-empty"))
+ opts->skip_empty = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.allow-ff"))
opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.mainline"))
@@ -1231,6 +1247,8 @@ static int save_opts(struct replay_opts *opts)
res |= git_config_set_in_file_gently(opts_file, "options.signoff", "true");
if (opts->record_origin)
res |= git_config_set_in_file_gently(opts_file, "options.record-origin", "true");
+ if (opts->skip_empty)
+ res |= git_config_set_in_file_gently(opts_file, "options.skip-empty", "true");
if (opts->allow_ff)
res |= git_config_set_in_file_gently(opts_file, "options.allow-ff", "true");
if (opts->mainline) {
@@ -1306,6 +1324,14 @@ int sequencer_continue(struct replay_opts *opts)
if ((res = read_populate_todo(&todo_list, opts)))
goto release_todo_list;
+ /* check if there is something to commit */
+ res = is_index_unchanged();
+ if (res < 0)
+ goto release_todo_list;
+
+ if (res && opts->skip_empty)
+ goto skip_this_commit;
+
/* Verify that the conflict has been resolved */
if (file_exists(git_path_cherry_pick_head()) ||
file_exists(git_path_revert_head())) {
@@ -1317,6 +1343,7 @@ int sequencer_continue(struct replay_opts *opts)
res = error_dirty_index(opts);
goto release_todo_list;
}
+skip_this_commit:
todo_list.current++;
res = pick_commits(&todo_list, opts);
release_todo_list:
diff --git a/sequencer.h b/sequencer.h
index 7a513c576b..c747cfcfc7 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -23,6 +23,7 @@ struct replay_opts {
int allow_empty;
int allow_empty_message;
int keep_redundant_commits;
+ int skip_empty;
int mainline;
--
2.11.0.585.g56041942c3.dirty
^ permalink raw reply related
* Re: [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: Lars Schneider @ 2017-01-21 13:32 UTC (permalink / raw)
To: George Vanburgh; +Cc: Git mailing list, Luke Diamand
In-Reply-To: <01020159c0e82598-e373cf0d-2bad-41bb-b455-6896ad183e22-000000@eu-west-1.amazonses.com>
> On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me> wrote:
>
> From: George Vanburgh <gvanburgh@bloomberg.net>
>
> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
> git config - no user mappings are applied to the generated repository.
>
> Reproduction Steps:
>
> 1. Add multiple git-p4.mapUser entries to git config on a Windows
> machine
> 2. Attempt to clone a p4 repository
>
> None of the user mappings will be applied.
>
> This issue is caused by the fact that gitConfigList,
> uses split(os.linesep) to convert the output of
> git config --get-all into a list.
>
> On Windows, os.linesep is equal to '\r\n' - however git.exe
> returns configuration with a line seperator of '\n'. This leads to
> the list returned by gitConfigList containing only one element - which
> contains the full output of git config --get-all in string form. This
> causes problems for the code introduced to getUserMapFromPerforceServer
> in 10d08a1.
>
> This issue should be caught by the test introduced in 10d08a1, and
> would require running on Windows to reproduce. When running inside
> MinGW/Cygwin, however, os.linesep correctly returns '\n', and everything
> works as expected.
This surprises me. I would expect `\r\n` in a MinGW env...
Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests
on Windows...
> The simplest fix for this issue would be to convert the line split logic
> inside gitConfigList to use splitlines(), which splits on any standard
> line delimiter. However, this function was only introduced in Python
> 2.7, and would mean a bump in the minimum required version of Python
> required to run git-p4. The alternative fix, implemented here, is to use
> '\n' as a delimiter, which git.exe appears to output consistently on
> Windows anyway.
Well, that also means if we ever use splitlines() then your fix below
would brake the code, right?
Python 2.7 was released 7 years ago in 2010. Therefore, I would vote to
bump the minimum version. But that's just my opinion :-)
> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> ---
> git-p4.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f427bf6..c134a58 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -656,7 +656,7 @@ def gitConfigInt(key):
> def gitConfigList(key):
> if not _gitConfig.has_key(key):
> s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
> - _gitConfig[key] = s.strip().split(os.linesep)
> + _gitConfig[key] = s.strip().split("\n")
I can't easily reproduce this as I don't have a running git-p4 setup on Windows.
However, your explanation and your fix make sense to me. If we don't want to bump
the version then this looks good to me.
Cheers,
Lars
^ permalink raw reply
* [RFC] what content should go in https://git-scm.com/doc/ext
From: Jeff King @ 2017-01-21 13:55 UTC (permalink / raw)
To: git
I'm wondering if anybody has opinions on:
https://github.com/git/git-scm.com/pull/924
(and I suspect most people in this community do not read pull requests
there, hence this post).
Basically, we maintain a list of links to outside documentation, as well
as to books. Somebody has requested a link to their paid tutorial
course. How should we handle it?
If the resource is valuable, then it may make sense to link to it, even
if it costs money. We already do this with book links, and my policy has
been to link any relevant book that is requested (I don't read them for
quality, so I have no opinions).
Should we do the same for tutorial content like this?
-Peff
^ permalink raw reply
* Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard
From: Jeff King @ 2017-01-21 14:03 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CACsJy8DxCPCXUeEoR1pQBCNqYOHjpgfLy_vgqsK1H=C0Mf=L8g@mail.gmail.com>
On Sat, Jan 21, 2017 at 07:44:05PM +0700, Duy Nguyen wrote:
> You just gave me a reason to rebuild urxvt with perl support. It
> solves my problem with SHA-1 nicely (and fixes another problem with
> very large counter in my approach, when you scroll git-log further
> down, because I can't know what's on screen or already scrolled past).
Note that even without my module, you can probably use the "matcher"
extension which is included with urxvt to do something similar (it can
handle arbitrary numbers of matching regexps).
The things I like about my module (and why I bothered to write it) are:
- it does nothing at all until you trigger it, so you don't have to
worry about the cost of your regexes, or how ugly false positives
will look when underlined
- I think the hint mode is much better for keyboard navigation.
The matcher extension does have a "list" mode where you get an
overlay with the whole list and select by number. That's what I
started with in mine, but I found it's really hard to use when there
are a lot of matches. Especially with sha1s.
> Do you think it should be in contrib for better discoverability (than
> burying it in the mail archive)? A separate git repo is also ok, but I
> guess that's just more burden.
I'm not sure it makes sense in git's contrib. It's really more about
urxvt than it is about git.
-Peff
^ permalink raw reply
* Re: [PATCH] log: new option decorate reflog of remote refs
From: Jeff King @ 2017-01-21 14:08 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jacob Keller, Git Mailing List
In-Reply-To: <CACsJy8C41D0Qb0ZJoAPJ6Pwp904dVixUR9yXQmZr0NU9W-BU8A@mail.gmail.com>
On Sat, Jan 21, 2017 at 07:48:50PM +0700, Duy Nguyen wrote:
> OK. Next question, how do we deal with the reflog count (i..e the
> argument of --decorate-remote-reflog). Should it be shared for all ref
> type, or can be specified differently for remote, local and tags? I'm
> leaning towards the former. But I'll wait a bit for ideas before
> rewriting the patch.
I doubt that anybody really cares about different reflog depths for
different refs. But I would say that the natural syntax ends up as:
git log --decorate-reflog=10 --remotes \
--decorate-reflog=10 --tags
anyway, so you get the ability to do it anyway "for free" (at the cost
of having to repeat yourself).
I guess the other option is:
git log --decorate-reflog-depth=10 \
--decorate-reflog --remotes
--decorate-reflog --tags
That's actually _more_ typing, and besides being less flexible just
muddles the "is this option for the next ref-selector or not" question.
(The whole thing is obviously a lot of typing; I wonder if people would
want a config option to do this all the time).
-Peff
^ permalink raw reply
* Re: [RFC PATCH 0/5] Localise error headers
From: Jeff King @ 2017-01-21 14:19 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, Stefan Beller, Michael J Gruber,
git@vger.kernel.org
In-Reply-To: <CACsJy8BRdd4f7f7+7XyN8jd69GS6tnkEGhw05F=7c96eViRVQg@mail.gmail.com>
On Fri, Jan 20, 2017 at 08:08:43PM +0700, Duy Nguyen wrote:
> > In addition, "BUG: " is relatively recent introduction to our
> > codebase. Perhaps having a separate BUG(<string>) function help the
> > distinction further?
>
> I was going to write the same thing. On top of that I wonder if have
> enough "if (something) die("BUG:")" to justify stealing BUG_ON() from
> kernel (better than assert since the condition will always be
> evaluated).
I don't mind BUG_ON() as an improvement over assert(). However, one of
the things I really like about our current die("BUG") is that you
actually write a human-readable message. Quite often that serves as a
comment to the reader of the code as to what is going on, or you can
give additional context in the error message (e.g., which sha1 triggered
the bug).
Still, there are many cases where there's not much to say in the
message. Doing:
if (!foo)
BUG("foo is NULL in this function");
is just wasting everybody's time. Something like:
#define BUG_ON(code) do {
if (code)
BUG("%s", #code);
while(0)
is probably fine. One complication is that BUG() would ideally show the
file/line number. That has to happen in a macro, but we don't assume
variadic macros work everywhere, which we need for the printf-like
behavior. So either we have to loosen that restriction[1], or we end up
with two "tiers": a BUG(fmt, ...) for human-readable messages, and a
more boring version for BUG_ON() which just prints "file:line: BUG:
<code>".
-Peff
[1] I think avoiding variadic macros was reasonable in 2005; C99 was
only 6 years old then. Now it's turning 18. Maybe it's time?
^ permalink raw reply
* Re: [RFC PATCH 0/5] Localise error headers
From: Jeff King @ 2017-01-21 14:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Michael J Gruber, git@vger.kernel.org
In-Reply-To: <xmqq1sw9piz5.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 11, 2017 at 10:08:46AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yes, I would think die_errno() is a no-brainer for translation, since
> > the strerror() will be translated.
> >
> >> apply.c: die(_("internal error"));
> >>
> >> That is funny, too. I think we should substitute that with
> >>
> >> die("BUG: untranslated, but what went wrong instead")
> >
> > Yep. We did not consistently use "BUG:" in the early days. I would say
> > that "BUG" lines do not need to be translated. The point is that nobody
> > should ever see them, so it seems like there is little point in giving
> > extra work to translators.
>
> In addition, "BUG: " is relatively recent introduction to our
> codebase. Perhaps having a separate BUG(<string>) function help the
> distinction further?
Yes, I think so. I have often been tempted to dump core on BUGs for
further analysis. You can do that by string-matching "BUG:" from the
beginning of a die message, but it's kind of gross. :)
-Peff
^ permalink raw reply
* Re: [RFC PATCH 0/5] Localise error headers
From: Jeff King @ 2017-01-21 14:24 UTC (permalink / raw)
To: Stefan Beller; +Cc: Michael J Gruber, git@vger.kernel.org
In-Reply-To: <CAGZ79kYVFbaMy1_-6qqe9zYQyktE2-7xu1Zz_qyeLWK=scgFKQ@mail.gmail.com>
On Wed, Jan 11, 2017 at 09:15:22AM -0800, Stefan Beller wrote:
> > That's the assumption I'm challenging. Certainly the behavior and
> > certain aspects of the output of a plumbing command should remain the
> > same over time. But error messages to stderr?
>
> In an ideal world that assumption would hold true and any machine
> readable information from the plumbing commands are on stdout and
> nobody in their sane mind would ever try to parse stderr.
>
> There is no easy way to check though except auditing all the code.
> Having pointed out some string handling mistakes in the prior email,
> my confidence is not high that plumbing commands are that strict about
> separating useful machine output and errors for human consumption.
I think it's impossible to audit all the code, because "all the code"
includes things not in git itself. It would really take a willingness to
declare "if you are parsing stderr you're doing it wrong".
I suppose the unpack-trees example speaks against that. I'm not sure
that is sane overall, but it is something we tried to account for in the
past.
If we are just talking about translation, though, it seems like the
right action for a script that really wants to parse stderr is to run
the sub-command with LC_MESSAGES=C. I know that's a breaking change, but
I wonder if it's a reasonable path forward.
-Peff
^ permalink raw reply
* [PATCH v1] travis-ci: fix Perforce install on macOS
From: larsxschneider @ 2017-01-21 14:42 UTC (permalink / raw)
To: git; +Cc: gitster, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
The `perforce` and `perforce-server` package were moved from brew [1][2]
to cask [3]. Teach TravisCI the new location.
Perforce updates their binaries without version bumps. That made the
brew install (legitimately!) fail due to checksum mismatches. The
workaround is not necessary anymore as Cask [4] allows to disable the
checksum test for individual formulas.
[1] https://github.com/Homebrew/homebrew-binary/commit/1394e42de04d07445f82f9512627e864ff4ca4c6
[2] https://github.com/Homebrew/homebrew-binary/commit/f8da22d6b8dbcfcfdb2dfa9ac1a5e5d8e05aac2b
[3] https://github.com/caskroom/homebrew-cask/pull/29180
[4] https://caskroom.github.io/
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Hi Junio,
this patch should fix the TravisCI builds:
https://travis-ci.org/git/git/builds
Could you fast track the patch to `maint` if it works without trouble on
`next` (as it should!)?
Thanks,
Lars
Notes:
Base Commit: 787f75f056 (master)
Diff on Web: https://github.com/larsxschneider/git/commit/ec7106339d
Checkout: git fetch https://github.com/larsxschneider/git travisci/brew-perforce-fix-v1 && git checkout ec7106339d
.travis.yml | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 3843967a69..c6ba8c8ec5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -75,20 +75,13 @@ before_install:
popd
;;
osx)
- brew_force_set_latest_binary_hash () {
- FORMULA=$1
- SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
- sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
- "$(brew --repository homebrew/homebrew-binary)/$FORMULA.rb"
- }
brew update --quiet
brew tap homebrew/binary --quiet
- brew_force_set_latest_binary_hash perforce
- brew_force_set_latest_binary_hash perforce-server
# Uncomment this if you want to run perf tests:
# brew install gnu-time
- brew install git-lfs perforce-server perforce gettext
+ brew install git-lfs gettext
brew link --force gettext
+ brew install Caskroom/cask/perforce
;;
esac;
echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
--
2.11.0
^ permalink raw reply related
* RE: [PATCH] git-p4: Fix git-p4.mapUser on Windows
From: George Vanburgh @ 2017-01-21 15:21 UTC (permalink / raw)
To: 'Lars Schneider'
Cc: 'Git mailing list', 'Luke Diamand'
In-Reply-To: <A7425283-9C32-4AE8-A442-11B7CFEAB4E8@gmail.com>
> On 21 Jan 2017, at 13:33, Lars Schneider <larsxschneider@gmail.com>
> > On 21 Jan 2017, at 13:02, George Vanburgh <george@vanburgh.me>
> wrote:
> >
> > From: George Vanburgh <gvanburgh@bloomberg.net>
> >
> > When running git-p4 on Windows, with multiple git-p4.mapUser entries
> > in git config - no user mappings are applied to the generated
repository.
> >
> > Reproduction Steps:
> >
> > 1. Add multiple git-p4.mapUser entries to git config on a Windows
> > machine
> > 2. Attempt to clone a p4 repository
> >
> > None of the user mappings will be applied.
> >
> > This issue is caused by the fact that gitConfigList, uses
> > split(os.linesep) to convert the output of git config --get-all into a
> > list.
> >
> > On Windows, os.linesep is equal to '\r\n' - however git.exe returns
> > configuration with a line seperator of '\n'. This leads to the list
> > returned by gitConfigList containing only one element - which contains
> > the full output of git config --get-all in string form. This causes
> > problems for the code introduced to getUserMapFromPerforceServer in
> > 10d08a1.
> >
> > This issue should be caught by the test introduced in 10d08a1, and
> > would require running on Windows to reproduce. When running inside
> > MinGW/Cygwin, however, os.linesep correctly returns '\n', and
> > everything works as expected.
>
> This surprises me. I would expect `\r\n` in a MinGW env...
> Nevertheless, I wouldn't have caught that as I don't run the git-p4 tests
on
> Windows...
It appears I was mistaken - the successful tests I ran were actually under
the Ubuntu subsystem for Windows, which (obviously) passed.
Just did a quick experiment:
Git Bash (MinGW):
georg@TEMPEST MINGW64 ~
$ python -c "import os
print(repr(os.linesep))"
'\r\n'
Powershell:
PS C:\Users\georg> python -c "import os
>> print(repr(os.linesep))"
'\r\n'
Ubuntu subsystem for Windows:
george@TEMPEST:~$ python -c "import os
print(repr(os.linesep))"
'\n'
So this issue applies to git-p4 running under both PowerShell and MinGW.
>
>
> > The simplest fix for this issue would be to convert the line split
> > logic inside gitConfigList to use splitlines(), which splits on any
> > standard line delimiter. However, this function was only introduced in
> > Python 2.7, and would mean a bump in the minimum required version of
> > Python required to run git-p4. The alternative fix, implemented here,
> > is to use '\n' as a delimiter, which git.exe appears to output
> > consistently on Windows anyway.
>
> Well, that also means if we ever use splitlines() then your fix below
would
> brake the code, right?
>
> Python 2.7 was released 7 years ago in 2010.
Now I feel old...
> Therefore, I would vote to
> bump the minimum version. But that's just my opinion :-)
I feel like splitlines is the better/safer fix - but figured bumping the
minimum
Python version was probably part of a wider discussion. If it's something
people
are comfortable with - I'd be happy to rework the fix to use splitlines.
Luke - do you have any thoughts on this?
>
>
> > Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> > ---
> > git-p4.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/git-p4.py b/git-p4.py
> > index f427bf6..c134a58 100755
> > --- a/git-p4.py
> > +++ b/git-p4.py
> > @@ -656,7 +656,7 @@ def gitConfigInt(key):
> > def gitConfigList(key):
> > if not _gitConfig.has_key(key):
> > s = read_pipe(["git", "config", "--get-all", key],
ignore_error=True)
> > - _gitConfig[key] = s.strip().split(os.linesep)
> > + _gitConfig[key] = s.strip().split("\n")
>
> I can't easily reproduce this as I don't have a running git-p4 setup on
> Windows.
> However, your explanation and your fix make sense to me. If we don't want
> to bump the version then this looks good to me.
>
> Cheers,
> Lars
^ permalink raw reply
* Re: submodule network operations [WAS: Re: [RFC/PATCH 0/4] working tree operations: support superprefix]
From: Brian J. Davis @ 2017-01-21 15:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, git@vger.kernel.org, David Turner
In-Reply-To: <CAGZ79kaS7zt3DKrRuqzzODc1HHEP-xd-8HBC0JA-HvmqAJOZfw@mail.gmail.com>
On 1/19/2017 7:22 PM, Stefan Beller wrote:
>>> Between the init and the update step you can modify the URLs.
>>> These commands are just a repetition from the first email, but the
>>> git commands can be viewed as moving from one state to another
>>> for submodules; submodules itself can be seen as a state machine
>>> according to that proposed documentation. Maybe such a state machine
>>> makes it easier to understand for some people.
>>
>> "Between the init and the update step you can modify the URLs." Yes I can
>> and have to... wish it was not this way.
> So how would yo u rather want to do it?
> look at the .gitmodules file beforehand and then run a "submodule update" ?
> Or a thing like
>
> git -c url.https://internal.insteadOf git://github.com/ \
> -c submodule.record-rewritten-urls submodule update
>
> (no need for init there as theoretically there is not
> need for such an intermediate step)
>
"Yes please and thank you" ... both.
My thought was to simply allow addition to .gitmodules. If I understand
correctly you are proposing, to override these at the command line and
possibly rewrite them on submodule update, but maybe not save or add to
.gitmodules. I would then propose both.
1) allow user to add to .gitmodules for those who do not care that
"outsiders" know the internal dev server
and
2) allow to rewrite while not saving to .gitmodules on fresh clone and
submodule update for thoes that do not want ousiders to known internal
dev server.
and possibly:
3) allow at command line to add remote to .gitmodules on submodule
commands (note add optoin in -c <name> = <value> pair)
.gitmodules before:
[submodule "subprojects/wrangler"]
path = subprojects/wrangler
url = git://github.com/
Then your adapted command:
git -c url.https://internal.insteadOf git://github.com/ \
-c submodule.record-rewritten-urls=add,internal --add submodule update
would produce
[submodule "subprojects/projname"]
path = subprojects/projname
remote.origin.url = git://github.com/
remote.internal.url =https://internal.insteadOf
Or similar support.
>>> [remote "origin"]
>>> url = https://github.com/..
>>> [remote "inhouse"]
>>> url = https://inhouse.corp/..
>>>
>>> But where do we clone it from?
>>> (Or do we just do a "git init" on that submodule and fetch
>>> from both remotes? in which order?)
>> origin by default and inhouse if specified. There is already a implied
>> default (origin). The idea was not to do both but rather what is specified.
>> Origin and inhouse are just names for remotes. If one wanted a
>> "--all-remotes" could pull from everywhere in the Ether if feature was to be
>> implemented.
> How is origin implied to be the default?
> Should there be an order (e.g. if you cannot find it at inhouse get it
> from github,
> if they are down, get it from kernel.org)
As it is in the Highlander series... "there can be only one" (remote).
So that is what I mean by origin. The only remote allowed is the
"origin" unless changed by the user... but there can still only be one
*currently*. Though I see your point as it is not labeled "origin". It
is not labeled at all. Apologies for confusion there.
^ permalink raw reply
* [PATCH 0/3] stash: support filename argument
From: Thomas Gummerer @ 2017-01-21 20:08 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Thomas Gummerer
This is the first try to implement the RFC I posted a week ago [1]. It
introduces a new push verb for git stash. I couldn't come up with
any better name that wasn't already taken. If anyone has ideas I'd be
very happy to hear them.
Thanks everyone for your input in the previous thread.
[1]: https://public-inbox.org/git/20170115142542.11999-1-t.gummerer@gmail.com/T/
Thomas Gummerer (3):
Documentation/stash: remove mention of git reset --hard
stash: introduce push verb
stash: support filename argument
Documentation/git-stash.txt | 13 ++++-
git-stash.sh | 123 ++++++++++++++++++++++++++++++++++++++++----
t/t3903-stash.sh | 36 +++++++++++++
3 files changed, 159 insertions(+), 13 deletions(-)
--
2.11.0.483.g087da7b7c
^ permalink raw reply
* [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-01-21 20:08 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Thomas Gummerer
In-Reply-To: <20170121200804.19009-1-t.gummerer@gmail.com>
Don't mention git reset --hard in the documentation for git stash save.
It's an implementation detail that doesn't matter to the end user and
thus shouldn't be exposed to them.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-stash.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0ad5335a3e 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -47,8 +47,9 @@ OPTIONS
save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
- Save your local modifications to a new 'stash', and run `git reset
- --hard` to revert them. The <message> part is optional and gives
+ Save your local modifications to a new 'stash', and revert the
+ the changes in the working tree to match the index.
+ The <message> part is optional and gives
the description along with the stashed state. For quickly making
a snapshot, you can omit _both_ "save" and <message>, but giving
only <message> does not trigger this action to prevent a misspelled
--
2.11.0.483.g087da7b7c
^ permalink raw reply related
* [PATCH 2/3] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-21 20:08 UTC (permalink / raw)
To: git
Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
Johannes Schindelin, Thomas Gummerer
In-Reply-To: <20170121200804.19009-1-t.gummerer@gmail.com>
Introduce a new git stash push verb in addition to git stash save. The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is specified after a -m
parameter instead of being a positional argument.
This allows introducing a new filename argument to stash single files.
Using that as a positional argument is much more consistent with the
rest of git, than using the positional argument for the message.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
git-stash.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
t/t3903-stash.sh | 9 +++++++
2 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..d6b4ae3290 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -189,10 +189,11 @@ store_stash () {
return $ret
}
-save_stash () {
+push_stash () {
keep_index=
patch_mode=
untracked=
+ stash_msg=
while test $# != 0
do
case "$1" in
@@ -216,6 +217,10 @@ save_stash () {
-a|--all)
untracked=all
;;
+ -m|--message)
+ shift
+ stash_msg=$1
+ ;;
--help)
show_help
;;
@@ -251,8 +256,6 @@ save_stash () {
die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
fi
- stash_msg="$*"
-
git update-index -q --refresh
if no_changes
then
@@ -291,6 +294,74 @@ save_stash () {
fi
}
+save_stash () {
+ push_options=
+ while test $# != 0
+ do
+ case "$1" in
+ -k|--keep-index)
+ push_options="-k $push_options"
+ ;;
+ --no-keep-index)
+ push_options="--no-keep-index $push_options"
+ ;;
+ -p|--patch)
+ push_options="-p $push_options"
+ ;;
+ -q|--quiet)
+ push_options="-q $push_options"
+ ;;
+ -u|--include-untracked)
+ push_options="-u $push_options"
+ ;;
+ -a|--all)
+ push_options="-a $push_options"
+ ;;
+ --help)
+ show_help
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ option="$1"
+ # TRANSLATORS: $option is an invalid option, like
+ # `--blah-blah'. The 7 spaces at the beginning of the
+ # second line correspond to "error: ". So you should line
+ # up the second line with however many characters the
+ # translation of "error: " takes in your language. E.g. in
+ # English this is:
+ #
+ # $ git stash save --blah-blah 2>&1 | head -n 2
+ # error: unknown option for 'stash save': --blah-blah
+ # To provide a message, use git stash save -- '--blah-blah'
+ eval_gettextln "error: unknown option for 'stash save': \$option
+ To provide a message, use git stash save -- '\$option'"
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ # if test -n "$patch_mode" && test -n "$untracked"
+ # then
+ # die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
+ # fi
+
+ stash_msg="$*"
+
+ if test -z stash_msg
+ then
+ push_stash $push_options
+ else
+ push_stash $push_options -m "$stash_msg"
+ fi
+}
+
have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
@@ -617,6 +688,10 @@ save)
shift
save_stash "$@"
;;
+push)
+ shift
+ push_stash "$@"
+ ;;
apply)
shift
apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..0171b824c9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
test_path_is_missing file
'
+test_expect_success 'push -m shows right message' '
+ >foo &&
+ git add foo &&
+ git stash push -m "test message" &&
+ echo "stash@{0}: On master: test message" >expect &&
+ git stash list | head -n 1 >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.483.g087da7b7c
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox