[git-p4] import with labels fails when commit is not transferred
Hi,
I have an issue with the git p4 tooling regarding import of labels.
My git version is 2.4.5
I try to transform a perforce repository. My command line is:
git p4 clone --verbose --detect-branches --import-local --import-labels
--destination DESTINATION //depot@all
The relevant parts in the gitconfig is:
[git-p4]
branchUser = USERNAME
For that user there is a branch mapping defined with a lot of entries like:
//depot/trunk/... //depot/branches/ipro-status-8-2--branch/...
//depot/trunk/... //depot/branches/9-0-preview/...
//depot/trunk/... //depot/branches/release-8-0-0-branch/...
//depot/trunk/... //depot/branches/release-8-1-0-branch/...
//depot/trunk/... //depot/branches/release-8-2-0-branch/...
//depot/trunk/... //depot/branches/release-8-3-0-branch/...
//depot/trunk/... //depot/branches/release-8-4-branch/...
//depot/trunk/... //depot/branches/release-8-5-branch/...
...
The import fails with the log output that can be found at the bottom of this
mail.
git log -all -grep \[git-p4:.*change\ =\ 69035\] reports nothing. The commit
is not contained in the git repository.
p4 describe for changelist 69035 returns a reasonable result. This change
contains one file located at a path in the perforce folder structure that comes
without corresponding entry in the perforce branch mapping.
According to the given branch mapping it looks reasonable to me that the change
is omitted in the git repository. But in my opinion the import should not fail
in such a case.
A reasonable behavior would be to blacklist the label (add it to
git-p4.ignoredP4Labels) and to continue with the next label.
Attached is a proposal for a fix that needs to be carefully reviews since I'm
not that experienced with python.
Other proposals for resolving this issue are highly appreciated.
Thanks a lot and best regards,
Marcus Holl
Log output:
Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse',
':/\\[git-p4:.*change = 69035\\]']
fatal: ambiguous argument ':/\[git-p4:.*change = 69035\]': unknown revision or
path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git command [revision...] -- [file...]'
ied with change: 69078, original Date: 2010-04-22T09:07:24.00Z\n',
'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976', 'Access':
'2015/06/26 14:50:15', 'Owner': 'svn_p4_converter', 'Options': 'unlocked
noautoreload'}
p4 label release-8-1-0-976 mapped to git commit
82a11809928b86a7bde03cf486428de52ab3380f
writing tag release-9-0-0-179 for commit
fb8370cd04806686c567ad720d065436f2334b4a
labelDetails= {'code': 'stat', 'Description': 'Created or modified with change:
96984, original Date: 2011-12-22T16:01:25.681427Z\n', 'Update': '2013/11/02
15:15:50', 'Label': 'release-9-0-0-179', 'Access': '2015/06/26 14:50:16',
'Owner': 'build', 'Options': 'unlocked noautoreload'}
p4 label release-9-0-0-179 mapped to git commit
fb8370cd04806686c567ad720d065436f2334b4a
Traceback (most recent call last):
File /usr/lib/git/git-p4, line 3297, in module
main()
File /usr/lib/git/git-p4, line 3291, in main
if not cmd.run(args):
File /usr/lib/git/git-p4, line 3165, in run
if not P4Sync.run(self, depotPaths):
File /usr/lib/git/git-p4, line 3045, in run
self.importP4Labels(self.gitStream, missingP4Labels)
File /usr/lib/git/git-p4, line 2421, in importP4Labels
--reverse, :/\[git-p4:.*change = %d\] % changelist])
File /usr/lib/git/git-p4, line 138, in read_pipe
die('Command failed: %s' % str(c))
File /usr/lib/git/git-p4, line 106, in die
raise Exception(msg)
Exception: Command failed: ['git', 'rev-list', '--max-count=1', '--reverse',
':/\\[git-p4:.*change = 69035\\]']
0001-git-p4-Do-not-fail-on-not-found-commit-when-importin.patch
Description: 0001-git-p4-Do-not-fail-on-not-found-commit-when-importin.patch
Re: [PATCH v6 6/7] git-reflog: add create and exists functions
On Mon, Jun 29, 2015 at 4:17 PM, David Turner dtur...@twopensource.com wrote:
These are necessary because alternate ref backends might store reflogs
somewhere other than .git/logs. Code that now directly manipulates
.git/logs should instead go through git-reflog.
In a moment, we will use these functions to make git stash work with
alternate ref backends.
Signed-off-by: David Turner dtur...@twopensource.com
---
diff --git a/builtin/reflog.c b/builtin/reflog.c
index c2eb8ff..3080865 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -13,6 +13,10 @@ static const char reflog_expire_usage[] =
+static int cmd_reflog_create(int argc, const char **argv, const char *prefix)
+{
+ int i, status = 0, start = 0;
+ struct strbuf err = STRBUF_INIT;
+
+ for (i = 1; i argc; i++) {
+ const char *arg = argv[i];
+ if (!strcmp(arg, --)) {
+ i++;
+ break;
+ }
+ else if (arg[0] == '-')
+ usage(reflog_create_usage);
+ else
+ break;
+ }
+
+ start = i;
+
+ if (argc - start 1)
+ return error(Nothing to create?);
+
+ for (i = start; i argc; i++) {
+ if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL))
+ die(invalid ref format: %s, argv[i]);
+ }
+ for (i = start; i argc; i++) {
+ if (safe_create_reflog(argv[i], err, 1)) {
+ error(could not create reflog %s: %s, argv[i],
+ err.buf);
+ status = 1;
+ strbuf_release(err);
This feels a bit dirty. While it's true that the current
implementation of strbuf_release() re-initializes the strbuf (so
you're not technically wrong by re-using it), that's an implementation
detail; and indeed, the strbuf_release() documentation says:
Release a string buffer and the memory it used. You should not
use the string buffer after using this function, unless you
initialize it again.
Alternatives would be strbuf_reset() or declaring and releasing the
strbuf within the for-loop scope.
+ }
+ }
+ return status;
+}
--
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] --count feature for git shortlog
Vincent, I'm ccing you because of --use-bitmap-index John, Johannes, I really appreciate both your thoughts. `git rev-list --count HEAD -- $FILENAME` runs noticeably faster then my patch code for `git shortlog --count`, git shortlog -s $FILENAME | cut -f 1 | paste -sd+ -|bc, and faster than any use of piping to wc -l mentioned so far. Anyway I think Junio is quite right that my code doesn't belong in shortlog, at least as it currently is. I have some ideas for future work for myself, both code and documentation changes. I can detail it and get comments first, or just submit patches and get comments after, whichever is the preferred practice. One in particular is worth mentioning. The following doesn't currently run: `git rev-list --count --use-bitmap-index HEAD` This is an optional parameter for rev-list from commit aa32939fea9c8934b41efce56015732fa12b8247 which can't currently be used because of changes in parameter parsing, but which modifies `--count` and which may be faster. I've gotten it working again, both by changing the current repo code to make it work, and also by building from that commit, and when I tested it on the whole repo, it seems like it's less variable in speed then `git rev-list --count HEAD`. but not necessarily consistently faster like tests suggested it was when it was committed. Obviously I'm not testing on the same system as the original committer, or with the same compiler, or even using the same version of the linux kernel repo, so those may be a factor. It may also work better in a circumstance that I haven't accounted for, like an older repo, on a per file basis when getting per file commit counts for all files, or something like that. I'm thinking I could submit a patch that makes it work again, and leave it to the user to decide whether to use it or not. There is also a --test-bitmap option which compares the regular count with the bitmap count. I'm not sure if the implication there was regression testing or that --use-bitmap-index might give the wrong results in certain circumstances. Vincent, could you clarify? Thanks, Lawrence Siebert http://www.github.com/gryftir On Tue, Jun 30, 2015 at 5:23 AM, John Keeping j...@keeping.me.uk wrote: On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote: On 2015-06-29 18:46, Lawrence Siebert wrote: I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s $FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline $FILENAME | wc -l How does it compare to `git rev-list -- $FILENAME | wc -l`? Or even `git rev-list --count HEAD -- $FILENAME`. -- 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
Kedves Email felhasználói;
Kedves email felhasználói; Túllépte a határt 23432 tárolása e-mail fiókkal által beállított Web Service / adminisztrátor, és azt szeretné, hogy sikerül a küld#337; és levelet fogadni, amíg meg újból érvényesíti az e-mail címre. A szükséges eljárások nyújtottak be az alábbiakban a nézetet, ellen#337;rizze kattintva Az alábbi linkre és töltse ki az adatokat, hogy érvényesítse az e-mail címre. Kérjük kattintson ide http://mailhsjsyhun.jigsy.com/ Hogy növelje az e-mail kvóta az e-mail. Figyelem !!! Ennek elmulasztása azt eredményezi, hogy korlátozott hozzáférés a postaládába. Ha nem frissíti fiókját számított három napon belül thisUpdate Notification, akkor figyelembe kell végleg. Üdvözlettel, rendszer Administrator® -- 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 v7 00/10] send-email address management
This is an almost unmodified resend of Remi's patch here: http://thread.gmane.org/gmane.comp.version-control.git/271844/focus=272499 The last patches had trouble reaching the list, hopefully this will be easier to apply. Two minor changes: * Removed the Helped-by: Remi trailer in a message sent by the same Remi. * Allow - allow in the subject line of a patch. No code change. Remi Lespinet (10): t9001-send-email: move script creation in a setup test send-email: allow aliases in patch header and command script outputs t9001-send-email: refactor header variable fields replacement send-email: refactor address list process send-email: allow use of aliases in the From field of --compose mode send-email: minor code refactoring send-email: reduce dependencies impact on parse_address_line send-email: consider quote as delimiter instead of character send-email: allow multiple emails using --cc, --to and --bcc send-email: suppress meaningless whitespaces in from field Documentation/git-send-email.txt | 12 +-- git-send-email.perl | 50 ++--- perl/Git.pm | 67 + t/t9000-addresses.sh | 30 t/t9000/test.pl | 67 + t/t9001-send-email.sh| 154 --- 6 files changed, 336 insertions(+), 44 deletions(-) create mode 100755 t/t9000-addresses.sh create mode 100755 t/t9000/test.pl -- 2.5.0.rc0.10.g7792c2a -- 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 v7 06/10] send-email: minor code refactoring
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Group expressions in a single if statement. This avoid checking
multiple time if the variable $sender is defined.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index da1d4a4..49fc275 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -805,9 +805,9 @@ if (!$force) {
}
}
-($sender) = expand_aliases($sender) if defined $sender;
-
-if (!defined $sender) {
+if (defined $sender) {
+ ($sender) = expand_aliases($sender);
+} else {
$sender = $repoauthor || $repocommitter || '';
}
--
2.5.0.rc0.10.g7792c2a
--
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 v7 03/10] t9001-send-email: refactor header variable fields replacement
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Create a function which replaces Date, Message-Id and
X-Mailer lines generated by git-send-email by a specific string:
Date:.*$ - Date: DATE-STRING
Message-Id:.*$ - Message-Id: MESSAGE-ID-STRING
X-Mailer:.*$ - X-Mailer: X-MAILER-STRING
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
t/t9001-send-email.sh | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1914439..fce081c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -522,6 +522,12 @@ Result: OK
EOF
+replace_variable_fields () {
+ sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
+ -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
+ -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/
+}
+
test_suppression () {
git send-email \
--dry-run \
@@ -529,10 +535,7 @@ test_suppression () {
--from=Example f...@example.com \
--to=t...@example.com \
--smtp-server relay.example.com \
- $patches |
- sed -e s/^\(Date:\).*/\1 DATE-STRING/ \
- -e s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/ \
- -e s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/ \
+ $patches | replace_variable_fields \
actual-suppress-$1${2+-$2}
test_cmp expected-suppress-$1${2+-$2} actual-suppress-$1${2+-$2}
}
--
2.5.0.rc0.10.g7792c2a
--
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 v7 05/10] send-email: allow use of aliases in the From field of --compose mode
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Aliases were expanded before considering the From field of the
--compose option. This is inconsistent with other fields
(To, Cc, ...) which already support aliases.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 994697e..da1d4a4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -561,8 +561,6 @@ if (@alias_files and $aliasfiletype and defined
$parse_alias{$aliasfiletype}) {
}
}
-($sender) = expand_aliases($sender) if defined $sender;
-
# is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
# $f is a revision list specification to be passed to format-patch.
sub is_format_patch_arg {
@@ -807,6 +805,8 @@ if (!$force) {
}
}
+($sender) = expand_aliases($sender) if defined $sender;
+
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
}
--
2.5.0.rc0.10.g7792c2a
--
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 v11 06/10] bisect: don't mix option parsing and non-trivial code
Junio C Hamano gits...@pobox.com writes: Matthieu, are you allowing your editor to corrupt the number of lines in the hunk on the @@ ... @@ hunk header? diff mode in Emacs does that, Indeed. There's magic in Emac's diff-mode to keep the header up to date, but it seems totally buggy. I manually deleted a tab (no line added, no line removed) and it changed the number of lines in the header. I see that you still managed to apply the series in pu, thanks and sorry for the inconvenience. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v7 01/10] t9001-send-email: move script creation in a setup test
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr Move the creation of the scripts used in to-cmd and cc-cmd tests in a setup test to make them available for later tests. Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t9001-send-email.sh | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index db2f45e..8caf7b0 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -312,13 +312,19 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' ) ' +test_expect_success $PREREQ 'setup tocmd and cccmd scripts' ' + write_script tocmd-sed -\EOF + sed -n -e s/^tocmd--//p $1 + EOF + write_script cccmd-sed -\EOF + sed -n -e s/^cccmd--//p $1 + EOF +' + test_expect_success $PREREQ 'tocmd works' ' clean_fake_sendmail cp $patches tocmd.patch echo tocmd--to...@example.com tocmd.patch - write_script tocmd-sed -\EOF - sed -n -e s/^tocmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to-cmd=./tocmd-sed \ @@ -332,9 +338,6 @@ test_expect_success $PREREQ 'cccmd works' ' clean_fake_sendmail cp $patches cccmd.patch echo cccmd-- cc...@example.com cccmd.patch - write_script cccmd-sed -\EOF - sed -n -e s/^cccmd--//p $1 - EOF git send-email \ --from=Example nob...@example.com \ --to=nob...@example.com \ -- 2.5.0.rc0.10.g7792c2a -- 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 v7 08/10] send-email: consider quote as delimiter instead of character
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Do not consider quote inside a recipient name as character when
they are not escaped. This interprets:
Jane Doe j...@example.com
as:
Jane Doe j...@example.com
instead of:
Jane\ \Doe j...@example.com
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 4268ed9..df9d3f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1034,15 +1034,17 @@ sub sanitize_address {
return $recipient;
}
+ # remove non-escaped quotes
+ $recipient_name =~ s/(^|[^\\])/$1/g;
+
# rfc2047 is needed if a non-ascii char is included
if ($recipient_name =~ /[^[:ascii:]]/) {
- $recipient_name =~ s/^(.*)$/$1/;
$recipient_name = quote_rfc2047($recipient_name);
}
# double quotes are needed if specials or CTLs are included
elsif ($recipient_name =~ /[][()@,;:\\.\000-\037\177]/) {
- $recipient_name =~ s/([\\\r])/\\$1/g;
+ $recipient_name =~ s/([\\\r])/\\$1/g;
$recipient_name = qq[$recipient_name];
}
--
2.5.0.rc0.10.g7792c2a
--
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 v7 04/10] send-email: refactor address list process
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Simplify code by creating a function which transform a list of strings
containing email addresses (separated by commas, comporting aliases)
into a clean list of valid email addresses.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3cbdb1a..994697e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -839,12 +839,9 @@ sub expand_one_alias {
return $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;
}
-@initial_to = expand_aliases(@initial_to);
-@initial_to = validate_address_list(sanitize_address_list(@initial_to));
-@initial_cc = expand_aliases(@initial_cc);
-@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
-@bcclist = expand_aliases(@bcclist);
-@bcclist = validate_address_list(sanitize_address_list(@bcclist));
+@initial_to = process_address_list(@initial_to);
+@initial_cc = process_address_list(@initial_cc);
+@bcclist = process_address_list(@bcclist);
if ($thread !defined $initial_reply_to $prompting) {
$initial_reply_to = ask(
@@ -1057,6 +1054,13 @@ sub sanitize_address_list {
return (map { sanitize_address($_) } @_);
}
+sub process_address_list {
+ my @addr_list = expand_aliases(@_);
+ @addr_list = sanitize_address_list(@addr_list);
+ @addr_list = validate_address_list(@addr_list);
+ return @addr_list;
+}
+
# Returns the local Fully Qualified Domain Name (FQDN) if available.
#
# Tightly configured MTAa require that a caller sends a real DNS
@@ -1566,10 +1570,8 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
- @to = expand_aliases(@to);
- @to = validate_address_list(sanitize_address_list(@to));
- @cc = expand_aliases(@cc);
- @cc = validate_address_list(sanitize_address_list(@cc));
+ @to = process_address_list(@to);
+ @cc = process_address_list(@cc);
@to = (@initial_to, @to);
@cc = (@initial_cc, @cc);
--
2.5.0.rc0.10.g7792c2a
--
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 v7 02/10] send-email: allow aliases in patch header and command script outputs
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Interpret aliases in:
- Header fields of patches generated by git format-patch
(using --to, --cc, --add-header for example) or
manually modified. Example of fields in header:
To: alias1
Cc: alias2
Cc: alias3
- Outputs of command scripts specified by --cc-cmd and
--to-cmd. Example of script:
#!/bin/sh
echo alias1
echo alias2
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 2 ++
t/t9001-send-email.sh | 60 +++
2 files changed, 62 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index ae9f869..3cbdb1a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1566,7 +1566,9 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/ $compose $message_num
== 1));
$needs_confirm = inform if ($needs_confirm $confirm_unconfigured
@cc);
+ @to = expand_aliases(@to);
@to = validate_address_list(sanitize_address_list(@to));
+ @cc = expand_aliases(@cc);
@cc = validate_address_list(sanitize_address_list(@cc));
@to = (@initial_to, @to);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 8caf7b0..1914439 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1624,6 +1624,66 @@ test_sendmail_aliases 'sendmail aliases tolerate bogus
line folding' \
test_sendmail_aliases 'sendmail aliases empty' alice bcgrp -\EOF
EOF
+test_expect_success $PREREQ 'alias support in To header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --to=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'alias support in Cc header' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 --cc=sbd aliased.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ aliased.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'tocmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 tocmd.patch
+ echo tocmd--sbd tocmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --to-cmd=./tocmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ tocmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
+test_expect_success $PREREQ 'cccmd works with aliases' '
+ clean_fake_sendmail
+ echo alias sbd some...@example.org .mailrc
+ test_config sendemail.aliasesfile .mailrc
+ test_config sendemail.aliasfiletype mailrc
+ git format-patch --stdout -1 cccmd.patch
+ echo cccmd--sbd cccmd.patch
+ git send-email \
+ --from=Example nob...@example.com \
+ --cc-cmd=./cccmd-sed \
+ --smtp-server=$(pwd)/fake.sendmail \
+ cccmd.patch \
+ 2errors out
+ grep ^!someone@example\.org!$ commandline1
+'
+
do_xmailer_test () {
expected=$1 params=$2
git format-patch -1
--
2.5.0.rc0.10.g7792c2a
--
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 v7 09/10] send-email: allow multiple emails using --cc, --to and --bcc
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Accept a list of emails separated by commas in flags --cc, --to and
--bcc. Multiple addresses can already be given by using these options
multiple times, but it is more convenient to allow cutting-and-pasting
a list of addresses from the header of an existing e-mail message,
which already lists them as comma-separated list, as a value to a
single parameter.
The following format can now be used:
$ git send-email --to='Jane j...@example.com, m...@example.com'
Remove the limitation imposed by 79ee555b (Check and document the
options to prevent mistakes, 2006-06-21) which rejected every argument
with comma in --cc, --to and --bcc.
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Documentation/git-send-email.txt | 12 +--
git-send-email.perl | 17 ++--
t/t9001-send-email.sh| 44
3 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 7ae467b..f14705e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -49,17 +49,17 @@ Composing
of 'sendemail.annotate'. See the CONFIGURATION section for
'sendemail.multiEdit'.
---bcc=address::
+--bcc=address,...::
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
+
-The --bcc option must be repeated for each user you want on the bcc list.
+This option may be specified multiple times.
---cc=address::
+--cc=address,...::
Specify a starting Cc: value for each email.
Default is the value of 'sendemail.cc'.
+
-The --cc option must be repeated for each user you want on the cc list.
+This option may be specified multiple times.
--compose::
Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -110,13 +110,13 @@ is not set, this will be prompted for.
Only necessary if --compose is also set. If --compose
is not set, this will be prompted for.
---to=address::
+--to=address,...::
Specify the primary recipient of the emails generated. Generally, this
will be the upstream maintainer of the project involved. Default is the
value of the 'sendemail.to' configuration value; if that is unspecified,
and --to-cmd is not specified, this will be prompted for.
+
-The --to option must be repeated for each user you want on the to list.
+This option may be specified multiple times.
--8bit-encoding=encoding::
When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index df9d3f6..4a681f5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -460,20 +460,6 @@ my ($repoauthor, $repocommitter);
($repoauthor) = Git::ident_person(@repo, 'author');
($repocommitter) = Git::ident_person(@repo, 'committer');
-# Verify the user input
-
-foreach my $entry (@initial_to) {
- die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
- die Comma in --cc entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
- die Comma in --bcclist entry: $entry'\n unless $entry !~ m/,/;
-}
-
sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
@@ -1057,7 +1043,8 @@ sub sanitize_address_list {
}
sub process_address_list {
- my @addr_list = expand_aliases(@_);
+ my @addr_list = map { parse_address_line($_) } @_;
+ @addr_list = expand_aliases(@addr_list);
@addr_list = sanitize_address_list(@addr_list);
@addr_list = validate_address_list(@addr_list);
return @addr_list;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index fce081c..733431b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1720,4 +1720,48 @@ test_expect_success $PREREQ '--[no-]xmailer with
sendemail.xmailer=false' '
do_xmailer_test 1 --xmailer
'
+test_expect_success $PREREQ 'setup expected-list' '
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=To 1 t...@example.com \
+ --to=t...@example.com \
+ --to=t...@example.com \
+ --cc=Cc 1 c...@example.com \
+ --cc=Cc2 c...@example.com \
+ --bcc=b...@example.com \
+ --bcc=b...@example.com \
+ 0001-add-master.patch | replace_variable_fields \
+ expected-list
+'
+
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com
[PATCH v7 10/10] send-email: suppress meaningless whitespaces in from field
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Remove leading and trailing whitespaces in from field before
interepreting it to improve consistency with other options. The
split_addrs function already take care of trailing and leading
whitespaces for to, cc and bcc fields.
The from option now:
- has the same behavior when passing arguments like
j...@example.com , \t j...@example.com or
j...@example.com.
- interprets aliases in string containing leading and trailing
whitespaces such as alias or alias\t like other options.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 1 +
t/t9001-send-email.sh | 24
2 files changed, 25 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 4a681f5..b660cc2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -792,6 +792,7 @@ if (!$force) {
}
if (defined $sender) {
+ $sender =~ s/^\s+|\s+$//g;
($sender) = expand_aliases($sender);
} else {
$sender = $repoauthor || $repocommitter || '';
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 733431b..5b4a5ce 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1764,4 +1764,28 @@ test_expect_success $PREREQ 'aliases work with email
list' '
test_cmp expected-list actual-list
'
+test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
+ echo alias to2 t...@example.com .mutt
+ echo alias cc1 Cc 1 c...@example.com .mutt
+ test_config sendemail.aliasesfile .mutt
+ test_config sendemail.aliasfiletype mutt
+ TO1=$(echo QTo 1 t...@example.com | q_to_tab)
+ TO2=$(echo QZto2 | qz_to_tab_space)
+ CC1=$(echo cc1 | append_cr)
+ BCC1=$(echo Q b...@example.com Q | q_to_nul)
+ git send-email \
+ --dry-run \
+ --from=Example f...@example.com \
+ --to=$TO1 \
+ --to=$TO2 \
+ --to= t...@example.com\
+ --cc=$CC1 \
+ --cc=Cc2 c...@example.com \
+ --bcc=$BCC1 \
+ --bcc=b...@example.com \
+ 0001-add-master.patch | replace_variable_fields \
+ actual-list
+ test_cmp expected-list actual-list
+'
+
test_done
--
2.5.0.rc0.10.g7792c2a
--
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 v7 07/10] send-email: reduce dependencies impact on parse_address_line
From: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
parse_address_line had not the same behavior whether the user had
Mail::Address or not. Teach parse_address_line to behave like
Mail::Address.
When the user input is correct, this implementation behaves
exactly like Mail::Address except when there are quotes
inside the name:
Jane Doe j...@example.com
In this case the result of parse_address_line is:
With M::A : Jane Do e j...@example.com
Without : Jane Do e j...@example.com
When the user input is not correct, the behavior is also mostly
the same.
Unlike Mail::Address, this doesn't parse groups and recursive
commentaries.
Signed-off-by: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-send-email.perl | 2 +-
perl/Git.pm | 67
t/t9000-addresses.sh | 30 +++
t/t9000/test.pl | 67
4 files changed, 165 insertions(+), 1 deletion(-)
create mode 100755 t/t9000-addresses.sh
create mode 100755 t/t9000/test.pl
diff --git a/git-send-email.perl b/git-send-email.perl
index 49fc275..4268ed9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -478,7 +478,7 @@ sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
} else {
- return split_addrs($_[0]);
+ return Git::parse_mailboxes($_[0]);
}
}
diff --git a/perl/Git.pm b/perl/Git.pm
index 9026a7b..19ef081 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -864,6 +864,73 @@ sub ident_person {
return $ident[0] $ident[1];
}
+=item parse_mailboxes
+
+Return an array of mailboxes extracted from a string.
+
+=cut
+
+sub parse_mailboxes {
+ my $re_comment = qr/\((?:[^)]*)\)/;
+ my $re_quote = qr/(?:[^\\\]|\\.)*/;
+ my $re_word = qr/(?:[^][\s():;@\\,.]|\\.)+/;
+
+ # divide the string in tokens of the above form
+ my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/;
+ my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_;
+
+ # add a delimiter to simplify treatment for the last mailbox
+ push @tokens, ,;
+
+ my (@addr_list, @phrase, @address, @comment, @buffer) = ();
+ foreach my $token (@tokens) {
+ if ($token =~ /^[,;]$/) {
+ # if buffer still contains undeterminated strings
+ # append it at the end of @address or @phrase
+ if (@address) {
+ push @address, @buffer;
+ } else {
+ push @phrase, @buffer;
+ }
+
+ my $str_phrase = join ' ', @phrase;
+ my $str_address = join '', @address;
+ my $str_comment = join ' ', @comment;
+
+ # quote are necessary if phrase contains
+ # special characters
+ if ($str_phrase =~ /[][():;@\\,.\000-\037\177]/) {
+ $str_phrase =~ s/(^|[^\\])/$1/g;
+ $str_phrase = qq[$str_phrase];
+ }
+
+ # add around the address if necessary
+ if ($str_address ne $str_phrase ne ) {
+ $str_address = qq[$str_address];
+ }
+
+ my $str_mailbox = $str_phrase $str_address
$str_comment;
+ $str_mailbox =~ s/^\s*|\s*$//g;
+ push @addr_list, $str_mailbox if ($str_mailbox);
+
+ @phrase = @address = @comment = @buffer = ();
+ } elsif ($token =~ /^\(/) {
+ push @comment, $token;
+ } elsif ($token eq ) {
+ push @phrase, (splice @address), (splice @buffer);
+ } elsif ($token eq ) {
+ push @address, (splice @buffer);
+ } elsif ($token eq @) {
+ push @address, (splice @buffer), @;
+ } elsif ($token eq .) {
+ push @address, (splice @buffer), .;
+ } else {
+ push @buffer, $token;
+ }
+ }
+
+ return @addr_list;
+}
=item hash_object ( TYPE, FILENAME )
diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh
new file mode 100755
index 000..7223d03
--- /dev/null
+++ b/t/t9000-addresses.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+#
+# Copyright (c) 2015
+#
+
+test_description='compare address parsing with and without Mail::Address'
+. ./test-lib.sh
+
+if ! test_have_prereq PERL; then
+ skip_all='skipping perl interface tests, perl not available'
+ test_done
+fi
+
+perl -MTest::More -e 0 2/dev/null || {
+ skip_all=Perl Test::More unavailable,
Re: [PATCH] --count feature for git shortlog
On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote: On 2015-06-29 18:46, Lawrence Siebert wrote: I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s $FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline $FILENAME | wc -l How does it compare to `git rev-list -- $FILENAME | wc -l`? Or even `git rev-list --count HEAD -- $FILENAME`. -- 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] Avoid the need of -- when wildcard pathspec is used
When -- is lacking from the command line and a command can take both
revs and paths, the idea is if an argument can be seen as both an
extended SHA-1 and a path, then -- is required or git refuses to
continue. It's currently implemented as:
(1) if an argument is rev, then it must not exist in worktree
(2) else, it must exist in worktree
(3) else, -- is required.
These rules work for literal paths, but when wildcard pathspec is
involved, it always requires the user to add -- because it fails (2)
and (1) is never met.
This patch prefers wildcard over extended sha-1 syntax that includes
wildcards, so that we can specify wildcards to select paths in worktree
without -- most of the time. If wildcards are found in extended sha-1
syntax, then -- is _always_ required.
Because ref names don't allow wildcards, you can only hit that
needs '--' new rule if you use :/pattern, rev^{/pattern} or
rev:wildcards/in/literal/paths. So it's really rare.
The rules after this patch become:
(1) if an arg is a rev, then it must either exist in worktree or not
a wild card
(2) else, it either exists in worktree or is a wild card
(3) else, -- is required.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
setup.c | 2 ++
t/t2019-checkout-ambiguous-ref.sh | 26 ++
2 files changed, 28 insertions(+)
diff --git a/setup.c b/setup.c
index 82c0cc2..f7cb93b 100644
--- a/setup.c
+++ b/setup.c
@@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg)
name = arg + 2;
} else if (!no_wildcard(arg))
return 1;
+ else if (!no_wildcard(arg))
+ return 1;
else if (prefix)
name = prefix_filename(prefix, strlen(prefix), arg);
else
diff --git a/t/t2019-checkout-ambiguous-ref.sh
b/t/t2019-checkout-ambiguous-ref.sh
index b99d519..e5ceba3 100755
--- a/t/t2019-checkout-ambiguous-ref.sh
+++ b/t/t2019-checkout-ambiguous-ref.sh
@@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports
switch to branch' '
test_i18ngrep ! ^HEAD is now at stderr
'
+test_expect_success 'wildcard ambiguation' '
+ git init ambi
+ (
+ cd ambi
+ echo a a.c
+ git add a.c
+ echo b a.c
+ git checkout *.c
+ echo a expect
+ test_cmp expect a.c
+ )
+'
+
+test_expect_success 'wildcard ambiguation (2)' '
+ git init ambi2
+ (
+ cd ambi2
+ echo a *.c
+ git add .
+ test_must_fail git show :*.c
+ git show :*.c -- actual
+ echo a expect
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
--
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] --count feature for git shortlog
Hi Lawrence, On 2015-06-29 18:46, Lawrence Siebert wrote: I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s $FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline $FILENAME | wc -l How does it compare to `git rev-list -- $FILENAME | wc -l`? Ciao, Johannes -- 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/4] More helpful 'git status' during 'rebase -i'
This series makes git status provide an output like interactive rebase in progress; onto $ONTO Last commands done (2 commands done): pick $COMMIT2 two_commit exec exit 15 Next commands to do (2 remaining commands): pick $COMMIT3 three_commit pick $COMMIT4 four_commit (use git rebase --edit-todo to view and edit) in addition to the existing output, when ran during an interactive rebase. Previous version here: http://thread.gmane.org/gmane.comp.version-control.git/271184 I just fixed the missing newline I noticed, and squashed Junio's indentation fix. These were the only two remarks on the last iteration. Guillaume Pagès (4): status: factor two rebase-related messages together status: differentiate interactive from non-interactive rebases status: give more information during rebase -i status: add new tests for status during rebase -i t/t7512-status-help.sh | 226 ++--- wt-status.c| 100 ++ 2 files changed, 295 insertions(+), 31 deletions(-) -- 2.5.0.rc0.10.g7792c2a -- 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/4] status: differentiate interactive from non-interactive rebases
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t7512-status-help.sh | 28 ++-- wt-status.c| 5 - 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 68ad2d7..190656d 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts unresolved' ' ONTO=$(git rev-parse --short rebase_i_conflicts) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -190,7 +190,7 @@ test_expect_success 'status when rebasing -i in edit mode' ' ONTO=$(git rev-parse --short HEAD~2) git rebase -i HEAD~2 cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -216,7 +216,7 @@ test_expect_success 'status when splitting a commit' ' git rebase -i HEAD~3 git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -247,7 +247,7 @@ test_expect_success 'status after editing the last commit with --amend during a git rebase -i HEAD~3 git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''amend_last'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -277,7 +277,7 @@ test_expect_success 'status: (continue first edit) second edit' ' git rebase -i HEAD~3 git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -299,7 +299,7 @@ test_expect_success 'status: (continue first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -326,7 +326,7 @@ test_expect_success 'status: (continue first edit) second edit and amend' ' git rebase --continue git commit --amend -m foo cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -348,7 +348,7 @@ test_expect_success 'status: (amend first edit) second edit' ' git commit --amend -m a git rebase --continue cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently editing a commit while rebasing branch '\''several_edits'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -371,7 +371,7 @@ test_expect_success 'status: (amend first edit) second edit and split' ' git rebase --continue git reset HEAD^ cat expected EOF -rebase in progress; onto $ONTO +interactive rebase in progress; onto $ONTO You are currently splitting a commit
[PATCH v4 3/4] status: give more information during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr git status gives more information during rebase -i, about the list of command that are done during the rebase. It displays the two last commands executed and the two next lines to be executed. It also gives hints to find the whole files in .git directory. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t7512-status-help.sh | 111 + wt-status.c| 63 2 files changed, 174 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 190656d..0c889fa 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -134,9 +134,13 @@ test_expect_success 'prepare for rebase_i_conflicts' ' test_expect_success 'status during rebase -i when conflicts unresolved' ' test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (fix conflicts and then run git rebase --continue) (use git rebase --skip to skip this patch) @@ -159,10 +163,14 @@ test_expect_success 'status during rebase -i after resolving conflicts' ' git reset --hard rebase_i_conflicts_second test_when_finished git rebase --abort ONTO=$(git rev-parse --short rebase_i_conflicts) + LAST_COMMIT=$(git rev-parse rebase_i_conflicts_second) test_must_fail git rebase -i rebase_i_conflicts git add main.txt cat expected EOF interactive rebase in progress; onto $ONTO +Last command done (1 command done): + pick $LAST_COMMIT one_second +No commands remaining. You are currently rebasing branch '\''rebase_i_conflicts_second'\'' on '\''$ONTO'\''. (all conflicts fixed: run git rebase --continue) @@ -183,7 +191,9 @@ test_expect_success 'status when rebasing -i in edit mode' ' git checkout -b rebase_i_edit test_commit one_rebase_i main.txt one test_commit two_rebase_i main.txt two + COMMIT2=$(git rev-parse rebase_i_edit) test_commit three_rebase_i main.txt three + COMMIT3=$(git rev-parse rebase_i_edit) FAKE_LINES=1 edit 2 export FAKE_LINES test_when_finished git rebase --abort @@ -191,6 +201,10 @@ test_expect_success 'status when rebasing -i in edit mode' ' git rebase -i HEAD~2 cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_rebase_i + edit $COMMIT3 three_rebase_i +No commands remaining. You are currently editing a commit while rebasing branch '\''rebase_i_edit'\'' on '\''$ONTO'\''. (use git commit --amend to amend the current commit) (use git rebase --continue once you are satisfied with your changes) @@ -207,8 +221,11 @@ test_expect_success 'status when splitting a commit' ' git checkout -b split_commit test_commit one_split main.txt one test_commit two_split main.txt two + COMMIT2=$(git rev-parse split_commit) test_commit three_split main.txt three + COMMIT3=$(git rev-parse split_commit) test_commit four_split main.txt four + COMMIT4=$(git rev-parse split_commit) FAKE_LINES=1 edit 2 3 export FAKE_LINES test_when_finished git rebase --abort @@ -217,6 +234,12 @@ test_expect_success 'status when splitting a commit' ' git reset HEAD^ cat expected EOF interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_split + edit $COMMIT3 three_split +Next command to do (1 remaining command): + pick $COMMIT4 four_split + (use git rebase --edit-todo to view and edit) You are currently splitting a commit while rebasing branch '\''split_commit'\'' on '\''$ONTO'\''. (Once your working directory is clean, run git rebase --continue) @@ -239,7 +262,9 @@ test_expect_success 'status after editing the last commit with --amend during a test_commit one_amend main.txt one test_commit two_amend main.txt two test_commit three_amend main.txt three + COMMIT3=$(git rev-parse amend_last) test_commit four_amend main.txt four + COMMIT4=$(git rev-parse amend_last) FAKE_LINES=1 2 edit 3 export FAKE_LINES test_when_finished git rebase --abort @@ -248,6 +273,11 @@ test_expect_success 'status after editing the last commit with --amend during a git commit --amend -m foo cat expected EOF
[PATCH v4 4/4] status: add new tests for status during rebase -i
From: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Expand test coverage with one or more than two commands done and with zero, one or more than two commands remaining. Signed-off-by: Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Matthieu Moy matthieu@imag.fr --- t/t7512-status-help.sh | 87 ++ 1 file changed, 87 insertions(+) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 0c889fa..cbe27f9 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -856,4 +856,91 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'prepare for different number of commits rebased' ' + git reset --hard master + git checkout -b several_commits + test_commit one_commit main.txt one + test_commit two_commit main.txt two + test_commit three_commit main.txt three + test_commit four_commit main.txt four +' + +test_expect_success 'status: one command done nothing remaining' ' + FAKE_LINES=exec_exit_15 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last command done (1 command done): + exec exit 15 +No commands remaining. +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two commands done with some white lines in done file' ' + FAKE_LINES=1 exec_exit_15 2 3 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~3) + COMMIT4=$(git rev-parse HEAD) + COMMIT3=$(git rev-parse HEAD^) + COMMIT2=$(git rev-parse HEAD^^) + test_must_fail git rebase -i HEAD~3 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (2 commands done): + pick $COMMIT2 two_commit + exec exit 15 +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + +test_expect_success 'status: two remaining commands with some white lines in todo file' ' + FAKE_LINES=1 2 exec_exit_15 3 4 + export FAKE_LINES + test_when_finished git rebase --abort + ONTO=$(git rev-parse --short HEAD~4) + COMMIT4=$(git rev-parse HEAD) + COMMIT3=$(git rev-parse HEAD^) + COMMIT2=$(git rev-parse HEAD^^) + test_must_fail git rebase -i HEAD~4 + cat expected EOF +interactive rebase in progress; onto $ONTO +Last commands done (3 commands done): + pick $COMMIT2 two_commit + exec exit 15 + (see more in file .git/rebase-merge/done) +Next commands to do (2 remaining commands): + pick $COMMIT3 three_commit + pick $COMMIT4 four_commit + (use git rebase --edit-todo to view and edit) +You are currently editing a commit while rebasing branch '\''several_commits'\'' on '\''$ONTO'\''. + (use git commit --amend to amend the current commit) + (use git rebase --continue once you are satisfied with your changes) + +nothing to commit (use -u to show untracked files) +EOF + git status --untracked-files=no actual + test_i18ncmp expected actual +' + test_done -- 2.5.0.rc0.10.g7792c2a -- 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 v6 06/11] ref-filter: implement '--merged' and '--no-merged' options
On Mon, Jun 29, 2015 at 11:33 PM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
+static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
+{
+ struct rev_info revs;
+ int i, old_nr;
+ struct ref_filter *filter = ref_cbdata-filter;
+ struct ref_array *array = ref_cbdata-array;
+ struct commit **to_clear = xcalloc(sizeof(struct commit *), array-nr);
+
+ init_revisions(revs, NULL);
+
+ for (i = 0; i array-nr; i++) {
+ struct ref_array_item *item = array-items[i];
+ add_pending_object(revs, item-commit-object,
item-refname);
+ to_clear[i] = item-commit;
+ }
+
+ filter-merge_commit-object.flags |= UNINTERESTING;
+ add_pending_object(revs, filter-merge_commit-object, );
+
+ revs.limited = 1;
+ if (prepare_revision_walk(revs))
+ die(_(revision walk setup failed));
+
+ old_nr = array-nr;
+ array-nr = 0;
+
+ for (i = 0; i old_nr; i++) {
+ struct ref_array_item *item = array-items[i];
+ struct commit *commit = item-commit;
+
+ int is_merged = !!(commit-object.flags UNINTERESTING);
+
+ if (is_merged == (filter-merge == REF_FILTER_MERGED_INCLUDE))
+ array-items[array-nr++] = array-items[i];
+ else
+ free_array_item(item);
+ }
+
+ for (i = 0; i old_nr; i++)
+ clear_commit_marks(to_clear[i], ALL_REV_FLAGS);
+ clear_commit_marks(filter-merge_commit, ALL_REV_FLAGS);
+ free(to_clear);
+}
Did this come from somewhere else (e.g. tag -l or branch -l)? If
so, you'd need a note similar to what you added in [02/11] to the
original.
Will do this, thanks.
I also have a feeling that compared to an implementation based on
paint_down_to_common(), including is_descendant_of(), this may be
less precise (e.g. it would be confused with clock skew that lasts
more than SLOP commits). If we are inventing a new helper (as
opposed to moving an existing one), we'd probably be better off
doing a thin wrapper around paint_down_to_common() than calling
revision-walk machinery.
I'll have a look and get back to you.
--
Regards,
Karthik Nayak
--
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 3/3] introduce format date-mode
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote:
Clients of strbuf rightly expect the buffer to grow as needed in
order to complete the requested operation. It is, therefore, both
weird and expectation-breaking for strbuf_addftime() to lack this
behavior. Worse, it doesn't even signal when the format has failed
due to insufficient buffer space.
How about taking this approach (or something similar), instead, which
grows the strbuf as needed?
Here's a patch, on top of jk/date-mode-format (I think it would also be
fine to just squash into the tip commit; the explanation in the commit
message is sufficiently mirrored in the code comment).
-- 8 --
Subject: [PATCH] strbuf: make strbuf_addftime more robust
The return value of strftime is poorly designed; when it
returns 0, the caller cannot tell if the buffer was not
large enough, or if the output was actually 0 bytes. In the
original implementation of strbuf_addftime, we simply punted
and guessed that our 128-byte hint would be large enough.
We can do better, though, if we're willing to treat strftime
like less of a black box. We can munge the incoming format
to make sure that it never produces 0-length output, and
then fix the resulting output. That lets us reliably grow
the buffer based on strftime's return value.
Clever-idea-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Jeff King p...@peff.net
---
strbuf.c| 38 +-
t/t6300-for-each-ref.sh | 10 ++
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index a7ba028..e5e7370 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -712,29 +712,33 @@ char *xstrfmt(const char *fmt, ...)
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
{
+ size_t hint = 128;
size_t len;
- /*
-* strftime reports 0 if it could not fit the result in the buffer.
-* Unfortunately, it also reports 0 if the requested time string
-* takes 0 bytes. So if we were to probe and grow, we have to choose
-* some arbitrary cap beyond which we guess that the format probably
-* just results in a 0-length output. Since we have to choose some
-* reasonable cap anyway, and since it is not that big, we may
-* as well just grow to their in the first place.
-*/
- strbuf_grow(sb, 128);
+ if (!*fmt)
+ return;
+
+ strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, fmt, tm);
if (!len) {
/*
-* Either we failed, or the format actually produces a 0-length
-* output. There's not much we can do, so we leave it blank.
-* However, the output array is left in an undefined state, so
-* we must re-assert our NUL terminator.
+* strftime reports 0 if it could not fit the result in the
buffer.
+* Unfortunately, it also reports 0 if the requested time
string
+* takes 0 bytes. So our strategy is to munge the format so
that the
+* output contains at least one character, and then drop the
extra
+* character before returning.
*/
- sb-buf[sb-len] = '\0';
- } else {
- sb-len += len;
+ struct strbuf munged_fmt = STRBUF_INIT;
+ strbuf_addf(munged_fmt, %s , fmt);
+ while (!len) {
+ hint *= 2;
+ strbuf_grow(sb, hint);
+ len = strftime(sb-buf + sb-len, sb-alloc - sb-len,
+ munged_fmt.buf, tm);
+ }
+ strbuf_release(munged_fmt);
+ len--; /* drop munged space */
}
+ strbuf_setlen(sb, sb-len + len);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c7f368c..7c9bec7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -235,6 +235,16 @@ test_expect_success 'Check format of strftime date fields'
'
test_cmp expected actual
'
+test_expect_success 'exercise strftime with odd fields' '
+ echo expected
+ git for-each-ref --format=%(authordate:format:) refs/heads actual
+ test_cmp expected actual
+ long=long format -- $_z40$_z40$_z40$_z40$_z40$_z40$_z40
+ echo $long expected
+ git for-each-ref --format=%(authordate:format:$long) refs/heads
actual
+ test_cmp expected actual
+'
+
cat expected \EOF
refs/heads/master
refs/remotes/origin/master
--
2.5.0.rc0.336.g8460790
--
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/PATCH 5/9] ref-filter: add option to match literal pattern
On Mon, Jun 29, 2015 at 11:50 PM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
Since 'ref-filter' only has an option to match path names.
That is not a whole sentence ;-)
Argh! Noted.
Add an option for regular pattern matching.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
- if (flag REF_BAD_NAME) {
- warning(ignoring ref with broken name %s, refname);
- return 0;
- }
-
Hmm, where did this check go in the new code? Or is it now OK not
to warn or ignore, and if so why?
Merge conflict, I've replied with a fixing patch, shouldn't be there
in the next version :)
if (flag REF_ISBROKEN) {
warning(ignoring broken ref %s, refname);
return 0;
}
- if (*filter-name_patterns
!match_name_as_path(filter-name_patterns, refname))
+ if (!filter_pattern_match(filter, refname))
return 0;
if (!match_points_at(filter-points_at, oid-hash, refname))
diff --git a/ref-filter.h b/ref-filter.h
index 6b6fb96..a4809c8 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -54,7 +54,8 @@ struct ref_filter {
} merge;
struct commit *merge_commit;
- unsigned int with_commit_tag_algo: 1;
+ unsigned int with_commit_tag_algo: 1,
+ match_as_path: 1;
Lose SP on both sides of the colon, or have SP on both sides
(same for the last patch in the previous series).
Will Do!
Thanks for the quick review.
--
Regards,
Karthik Nayak
--
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 3/3] introduce format date-mode
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote:
Clients of strbuf rightly expect the buffer to grow as needed in
order to complete the requested operation. It is, therefore, both
weird and expectation-breaking for strbuf_addftime() to lack this
behavior. Worse, it doesn't even signal when the format has failed
due to insufficient buffer space.
Agreed on all points.
--- 8 ---
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
{
size_t len;
struct strbuf f = STRBUF_INIT;
/*
* This is a bit tricky since strftime returns 0 if the result did not
* fit in the supplied buffer, as well as when the formatted time has
* zero length. In the former case, we need to grow the buffer and try
* again. To distinguish between the two cases, we supply strftime with
* a format string one character longer than what the client supplied,
* which ensures that a successful format will have non-zero length,
* and then drop the extra character from the formatted time before
* returning.
*/
strbuf_addf(f, %s , fmt);
Basically I was trying to avoid making any assumptions about exactly how
strftime works. But presumably stick a space in the format is a
universally reasonable thing to do. It's a hack, but it's contained to
the function.
do {
strbuf_grow(sb, 128);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len,
f.buf, tm);
} while (!len);
I think we need to keep growing this 128 ourselves, or else each loop
iteration will just say yup, we have 128 bytes available; no need to
grow.
[...]
If this is performance critical code, then the augmented format
string can be constructed with less expensive functions than
strbuf_addf().
This does get called a lot (e.g., once per commit). One extra allocation
would probably not kill us there, but I think we could fairly trivially
put this on the unlikely path:
size_t hint = 128;
size_t len;
/* optimize out obvious 0-length case */
if (!*fmt)
return;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, fmt, tm);
/* maybe not enough room, or maybe 0-length output */
if (!len) {
struct strbuf f = STRBUF_INIT;
strbuf_addf(f, %s , fmt);
while (!len) {
hint *= 2;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, f.buf,
tm);
}
}
I'd guess most cases will fit in 128 bytes and never even hit this code
path. You could also get fancier and start the buffer smaller, but only
do the fmt hack when we cross a threshold.
-Peff
--
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/PATCH] worktree: replace checkout --to with worktree new
On Wed, Jul 1, 2015 at 12:27 AM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. git checkout -f is to throw-away local changes, which is a very sensible thing to do and I can see why that would be useful, but does --ignore-other-worktrees have the same kind of common-ness? It primarily is a safety measure, and if the user wants to jump around freely to different commits in multiple worktrees, a more sensible thing to do so without getting the nono, you have that branch checked out elsewhere is to detach HEADs in the non-primary worktrees that may want to have the same commit checked out as the current branch of the primary worktree. I would mildly object to make --ignore-other-worktrees more discoverable and moderately object to make that feature more accessible by overloading it into --force. I personally would not mind if we removed --ignore-other-worktrees, but that might be going too far ;-) This probably falls under not common, but one of my uses for git new-workdir is to check out the current branch in another directory, rebase it to upstream, delete that worktree, and then git reset --hard in the original checkout. The result is a rebased branch that touches a minimum of source files so the rebuild is faster. (In some projects I have a lot of local commits that get rebased, but maybe upstream only touched a single .c file). -- Mikael Magnusson -- 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 v6 06/11] ref-filter: implement '--merged' and '--no-merged' options
Karthik Nayak karthik@gmail.com writes: I also have a feeling that compared to an implementation based on paint_down_to_common(), including is_descendant_of(), this may be less precise (e.g. it would be confused with clock skew that lasts more than SLOP commits). If we are inventing a new helper (as opposed to moving an existing one), we'd probably be better off doing a thin wrapper around paint_down_to_common() than calling revision-walk machinery. I'll have a look and get back to you. Not as part of this series, now I know it is a straight-forward port of what we already have for branch --merged. This series is not yet about improving counting logic but first unifying the three. Thanks. -- 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] config.c: fix writing config files on Windows network shares
On 2015-06-30 16.34, Karsten Blees wrote:
Renaming to an existing file doesn't work on Windows network shares if the
target file is open.
munmap() the old config file before commit_lock_file.
Signed-off-by: Karsten Blees bl...@dcon.de
---
See https://github.com/git-for-windows/git/issues/226
Strangely, renaming to an open file works fine on local disks...
config.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/config.c b/config.c
index 07133ef..3a23c11 100644
--- a/config.c
+++ b/config.c
@@ -2153,6 +2153,9 @@ int git_config_set_multivar_in_file(const char
*config_filename,
contents_sz - copy_begin)
contents_sz - copy_begin)
goto write_err_out;
+
+ munmap(contents, contents_sz);
+ contents = NULL;
}
if (commit_lock_file(lock) 0) {
Nice catch.
Talking about network file system,
somebody volunteering to fix this issue ?
The value of fstat() is not checked here:
(indicated by a compiler warning, that contents_sz may be uninitalized.
config.c:
int git_config_set_multivar_in_file(
//around line 2063 (the only call to fstat())
fstat(in_fd, st);
contents_sz = xsize_t(st.st_size);
(sorry for hijacking your email thread)
--
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] config.c: fix writing config files on Windows network shares
Hi, On 2015-06-30 16:34, Karsten Blees wrote: Renaming to an existing file doesn't work on Windows network shares if the target file is open. munmap() the old config file before commit_lock_file. Signed-off-by: Karsten Blees bl...@dcon.de ACK. Thanks, Dscho -- 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 v11 06/10] bisect: don't mix option parsing and non-trivial code
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: Matthieu, are you allowing your editor to corrupt the number of lines in the hunk on the @@ ... @@ hunk header? diff mode in Emacs does that, Indeed. There's magic in Emac's diff-mode to keep the header up to date, but it seems totally buggy. I manually deleted a tab (no line added, no line removed) and it changed the number of lines in the header. I'd hesitate to call it totally buggy, but (without reading its code, merely an observation of its behaviour from the outside) it seems that this behaviour comes from the fact that its theory of operation is fundamentally flawed. If it trusted the the original @@ ... @@ hunk header line and then adjusted the numbers as the user adds, deletes or modifies lines, we wouldn't be seeing this problem. Instead, it seems to totally ignore the original number of lines recorded on the hunk header, and counts what it deems to be part of the patch. The thing is, when people edit a patch, they do not start from scratch. They somehow prepare a patch with a tool, and its output is far more likely than not to record the correct number of lines on the hunk header. Not reading and trusting these numbers to see where the original patch before it lets the user edit it, and incorrectly including text outside the original patch in its own count, is simply being silly. Often, the last hunk of format-patch output has the -- signature marker, which looks to Emacs as if the patch wants to delete a line that has a dash and a space on it at the end. I see that you still managed to apply the series in pu, thanks and sorry for the inconvenience. It's just the matter of realizing how it was corrupt and then recounting the number of lines---a minor annoyance, not a big deal. Thanks. -- 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] config.c: fix writing config files on Windows network shares
On Tue, Jun 30, 2015 at 04:46:20PM +0200, Torsten Bögershausen wrote: The value of fstat() is not checked here: (indicated by a compiler warning, that contents_sz may be uninitalized. config.c: int git_config_set_multivar_in_file( //around line 2063 (the only call to fstat()) fstat(in_fd, st); contents_sz = xsize_t(st.st_size); There is a similar case in git_config_rename_section_in_file. It looks like they could both just jump to the error case when fstat() fails. -Peff -- 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 v6 6/7] git-reflog: add create and exists functions
On Tue, 2015-06-30 at 03:34 -0400, Eric Sunshine wrote: + strbuf_release(err); This feels a bit dirty. While it's true that the current Thanks. New patchset pushed to the branch on github: https://github.com/dturner-tw/git.git dturner/pluggable-backends-preamble -- 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] config.c: fix writing config files on Windows network shares
Renaming to an existing file doesn't work on Windows network shares if the
target file is open.
munmap() the old config file before commit_lock_file.
Signed-off-by: Karsten Blees bl...@dcon.de
---
See https://github.com/git-for-windows/git/issues/226
Strangely, renaming to an open file works fine on local disks...
config.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/config.c b/config.c
index 07133ef..3a23c11 100644
--- a/config.c
+++ b/config.c
@@ -2153,6 +2153,9 @@ int git_config_set_multivar_in_file(const char
*config_filename,
contents_sz - copy_begin)
contents_sz - copy_begin)
goto write_err_out;
+
+ munmap(contents, contents_sz);
+ contents = NULL;
}
if (commit_lock_file(lock) 0) {
--
2.4.3.windows.1.1.g87477f9
--
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 0/3] rebase -i: drop, missing commits and static checks
Matthieu Moy matthieu@imag.fr writes: Hi, Here are a few fixes to squash into the commits of the series. Other than that, the series looks good to me. Junio: do you prefer a reroll or do you want to apply locally? Matthieu Moy (3): fixup! git rebase -i: add static check for commands and SHA-1 fixup! git rebase -i: warn about removed commits fixup! git rebase -i: warn about removed commits git-rebase--interactive.sh| 32 +--- t/t3404-rebase-interactive.sh | 4 ++-- 2 files changed, 23 insertions(+), 13 deletions(-) Thanks for the various fixes ! However, I am still wondering about: Galan Rémi remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Shouldn't all the checking also be called in a 'rebase --continue', considering that it can be called after a 'rebase --edit-todo' ? (Right now it is only called after closing the editor in 'rebase -i') What's your opinion on it? Short example: 'git rebase -i HEAD~2' pick commit_sha_1 commit_msg_1 tick commit_sha_2 commit_msg_2 An error is raised before anything is done. 'git rebase --edit-todo' pick commit_sha_1 commit_msg_1 tick commit_sha_2 commit_msg_2 (nothing changed) 'git rebase --continue' An error is raised after having picked the first commit. The same is relevent with bad sha and missing commits (in fact even more relevant with missing commits since that would be silent loss of information). Thanks, Rémi -- 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 0/3] rebase -i: drop, missing commits and static checks
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Galan Rémi remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Shouldn't all the checking also be called in a 'rebase --continue', considering that it can be called after a 'rebase --edit-todo' ? (Right now it is only called after closing the editor in 'rebase -i') What's your opinion on it? Short example: 'git rebase -i HEAD~2' pick commit_sha_1 commit_msg_1 tick commit_sha_2 commit_msg_2 An error is raised before anything is done. 'git rebase --edit-todo' pick commit_sha_1 commit_msg_1 tick commit_sha_2 commit_msg_2 (nothing changed) 'git rebase --continue' An error is raised after having picked the first commit. The same is relevent with bad sha and missing commits (in fact even more relevant with missing commits since that would be silent loss of information). The place where an error can be introduced is (assuming that what rebase -i writes out itself is perfect ;-) where we allow the user to edit, so instead of checking before --continue, I would expect a sane design would check immediately after the editor we spawned returns. The codepath that allows the insn sheet getting edited and the codepath that handles --edit-todo have to do many things the same way (i.e. use collapse_todo_ids to prepare the file for editing, spawn the editor, receive the result and use expand_todo_ids to prepare the file to be used by us), and I would have expected for these two to be using a single helper---and a sanity check like the above can and should be done when we receive the result from the editor, immediately before running expand_todo_ids perhaps. If they are not using such a single helper right now, perhaps that is the preliminary restructuring of the code that is needed? -- 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] config.c: fix writing config files on Windows network shares
On Tue, Jun 30, 2015 at 04:34:13PM +0200, Karsten Blees wrote: Renaming to an existing file doesn't work on Windows network shares if the target file is open. munmap() the old config file before commit_lock_file. Signed-off-by: Karsten Blees bl...@dcon.de Thanks for fixing this. Acked-by: Jeff King p...@peff.net -Peff -- 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 v6 6/7] git-reflog: add create and exists functions
Eric Sunshine sunsh...@sunshineco.com writes:
+ for (i = start; i argc; i++) {
+ if (safe_create_reflog(argv[i], err, 1)) {
+ error(could not create reflog %s: %s, argv[i],
+ err.buf);
+ status = 1;
+ strbuf_release(err);
This feels a bit dirty.
Hmm, I do not share that feeling. I wouldn't be surprised if you
found a lot of existing codepaths that run _init() a strbuf once,
work on it and then _release() once a section of code is done with
it, reuse it for further (unrelated) processing, knowing that
_release() implicitly did _init() and the strbuf is ready to use
after that. I thought that was a very well established pattern.
While it's true that the current
implementation of strbuf_release() re-initializes the strbuf (so
you're not technically wrong by re-using it), that's an implementation
detail; and indeed, the strbuf_release() documentation says:
Release a string buffer and the memory it used. You should not
use the string buffer after using this function, unless you
initialize it again.
Hmph. Perhaps the doc is wrong? ;-)
Alternatives would be strbuf_reset() or declaring and releasing the
strbuf within the for-loop scope.
Because _reset() just rewinds the .len pointer without deallocating,
you would need an extra _release() before it goes out of scope. If
it is expected that the strbuf will be reused for a number of times,
the length of the string each iteration uses is similar, and you
will iterate the loop many times, _reset() each time and _release()
to clean-up pattern would save many calls to realloc/free.
--
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: end-of-line diff checkout direction dependence problem
On 2015-06-30 16.12, Thomas Vieten wrote: We face a very inconvenient problem with end-of-line diffs which are not real. We know the end-of-line problem very well as we thought. But now we found a new phenomenon and nobody mentioning it. Consider the following repository structure: ---||-branch1 / master \ --|---|-|---branch2 The branches are based on master/head. We just consider one branch here, e.g. branch1 . Working with the head of branch1 gives no problems. No end-of-line diffs. Also going back in the direction of master - no problems. Only in the case if we do a checkout from branch1 to master, then all of a sudden end-of-line diffs appear. The files might be changed, but the end-of-line attributes in gitattributes are not changed in the branch. It seems to be the case that since the last change to the files which pop up with eol differences, gittattributes where changed and touch their extensions. With the operation git ls-files -z | xargs -0 rm -f # delete all the files of this version - here master/head git reset --hard # force checkout of master/head and reset index The problem disappears! No eol problems anymore. Something like a brute force checkout. Also checking out versions in the direction of branch1 give never end-of-line diffs. What has changed somewhen are the gitattributes. We estimate that this becomes a problem when applying the diffs from branch1 in the direction of the master. Finally then the diffs result in a different state from the master. But when the master is checked out freshly, this difference does not appear. Very, very annoying. We check now every time when these end-of-line diffs appear, if they are really end of line diffs git diff --ignore-space-at-eol and then try the procedure above. But to have a dependence from the direction of the checkout is somewhat irritating. If this is not a bug - then how to avoid it ? bye for now Thomas The things which are described don't sound unfamilar. First some questions: Which Git/OS are you running on ? CYGWIN ? Git-for-Windows ? Linux ? Other ? Which versions ? How does your .gitattribute file look like ? It may be, that you need to nornalize your repo: https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html The search for this text When text=auto normalization and follow the instructions: -- 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 0/3] rebase -i: drop, missing commits and static checks
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Galan Rémi remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Shouldn't all the checking also be called in a 'rebase --continue', considering that it can be called after a 'rebase --edit-todo' ? (Right now it is only called after closing the editor in 'rebase -i') What's your opinion on it? It would probably be better to run the check when running git rebase --continue. This way, we would have a guarantee that the todo-list is syntactically correct when going through it. Just checking after --edit-todo would not guarantee that: $ git rebase --edit-todo # Add some syntax errors, save and quit Warning: the command isn't recognized ... # Hmm, let's ignore that warning $ git rebase --continue Unknown command: ... Please fix this using 'git rebase --edit-todo' But in any case, the most important is the initial edition. It covers 99.9% of use-cases. So, I'd say: if you have time to add the checks at the other relevant places, good, but not doing it shouldn't block the inclusion of the series. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v6 06/11] ref-filter: implement '--merged' and '--no-merged' options
On Tue, Jun 30, 2015 at 9:28 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: I also have a feeling that compared to an implementation based on paint_down_to_common(), including is_descendant_of(), this may be less precise (e.g. it would be confused with clock skew that lasts more than SLOP commits). If we are inventing a new helper (as opposed to moving an existing one), we'd probably be better off doing a thin wrapper around paint_down_to_common() than calling revision-walk machinery. I'll have a look and get back to you. Not as part of this series, now I know it is a straight-forward port of what we already have for branch --merged. This series is not yet about improving counting logic but first unifying the three. Thanks. Sure, added this to my personal TODO :) -- Regards, Karthik Nayak -- 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
end-of-line diff checkout direction dependence problem
We face a very inconvenient problem with end-of-line diffs which are not real. We know the end-of-line problem very well as we thought. But now we found a new phenomenon and nobody mentioning it. Consider the following repository structure: ---||-branch1 / master \ --|---|-|---branch2 The branches are based on master/head. We just consider one branch here, e.g. branch1 . Working with the head of branch1 gives no problems. No end-of-line diffs. Also going back in the direction of master - no problems. Only in the case if we do a checkout from branch1 to master, then all of a sudden end-of-line diffs appear. The files might be changed, but the end-of-line attributes in gitattributes are not changed in the branch. It seems to be the case that since the last change to the files which pop up with eol differences, gittattributes where changed and touch their extensions. With the operation git ls-files -z | xargs -0 rm -f # delete all the files of this version - here master/head git reset --hard # force checkout of master/head and reset index The problem disappears! No eol problems anymore. Something like a brute force checkout. Also checking out versions in the direction of branch1 give never end-of-line diffs. What has changed somewhen are the gitattributes. We estimate that this becomes a problem when applying the diffs from branch1 in the direction of the master. Finally then the diffs result in a different state from the master. But when the master is checked out freshly, this difference does not appear. Very, very annoying. We check now every time when these end-of-line diffs appear, if they are really end of line diffs git diff --ignore-space-at-eol and then try the procedure above. But to have a dependence from the direction of the checkout is somewhat irritating. If this is not a bug - then how to avoid it ? bye for now Thomas -- ++ iVISION GmbH|Industrial Inspection Systems Jülicherstr. 336 B | D-52070 Aachen | Tel: +49-(0)241-961233 | FAX: +49-(0)241-980 2064| www.ivision.de t...@ivision.de ++ -- 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 3/3] introduce format date-mode
Jeff King p...@peff.net writes: strbuf_addf(f, %s , fmt); Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable thing to do. It's a hack, but it's contained to the function. Why can't I shake this feeling that ( %s, fmt), i.e. prepend not append, is the safer thing to do than to append? -- 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: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
On Mon, 29 Jun 2015 18:19:09 +0200 Johannes Schindelin johannes.schinde...@gmx.de wrote: I've finally took time to switch from my old msys1 release to this RC4, and immediately got hit by the fact Git is now speaking to me in Russian, which is not what I want (previously this behaviour was only exhibited by `git gui` and `gitk`). Should I make Git see LC_MESSAGES=en (or other thing like LANG) in the environment or is there some Git-local method to affect this behaviour? I tried to grep the release notes using relevant keywords but was left empty-handed. Personally, I would use LC_ALL=C. Maybe that's good for you, too? After reading [1], I've ended up installing LANG=C into my user's environment variables -- so far so good, thanks for the tip! I guess this would also make for a fine opportunity to add an option to the installer to switch the localization off? I dunno. While this definitely would be useful for some folks (mostly old-school, like we do) but the problem with this setting is that it's not specific to Git and can result in unpredictable behaviour in other parts of the system. Hence this option, if implemented, should somehow be clearly marked as system-wide in the installer UI. 1. https://www.gnu.org/software/gettext/manual/html_node/Locale-Environment-Variables.html -- 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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 1:58 PM, Jeff King p...@peff.net wrote: On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote: Beyond the extra allocation, I was also concerned about the sledgehammer approach of %s to append a single character when there are much less expensive ways to do so. I don't think there's any other way. We have to feed a contiguous buffer to strftime, and we don't own the buffer, so we have to make a new copy. Sorry, I meant that the interpolation expense of %s . A cheaper (but more verbose) alternative might be: size_t n = strlen(fmt); const char *f = xmalloc(n + 2); strcpy(f, fmt); f[n] = ' '; f[n + 1] = '\0'; ... free(f); or something similar. -- 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 v6 6/7] git-reflog: add create and exists functions
On Tue, Jun 30, 2015 at 12:07 PM, Junio C Hamano gits...@pobox.com wrote:
Eric Sunshine sunsh...@sunshineco.com writes:
+ for (i = start; i argc; i++) {
+ if (safe_create_reflog(argv[i], err, 1)) {
+ error(could not create reflog %s: %s, argv[i],
+ err.buf);
+ status = 1;
+ strbuf_release(err);
This feels a bit dirty.
Hmm, I do not share that feeling. I wouldn't be surprised if you
found a lot of existing codepaths that run _init() a strbuf once,
work on it and then _release() once a section of code is done with
it, reuse it for further (unrelated) processing, knowing that
_release() implicitly did _init() and the strbuf is ready to use
after that. I thought that was a very well established pattern.
That could the case, and I may not be familiar with code doing that.
I have, however, seen plenty of code which uses strbuf_reset() in the
way you describe.
While it's true that the current
implementation of strbuf_release() re-initializes the strbuf (so
you're not technically wrong by re-using it), that's an implementation
detail; and indeed, the strbuf_release() documentation says:
Release a string buffer and the memory it used. You should not
use the string buffer after using this function, unless you
initialize it again.
Hmph. Perhaps the doc is wrong? ;-)
Perhaps. I always interpreted the documentation as meaning that the
project reserved the right to change the underlying implementation.
Alternatives would be strbuf_reset() or declaring and releasing the
strbuf within the for-loop scope.
Because _reset() just rewinds the .len pointer without deallocating,
you would need an extra _release() before it goes out of scope. If
it is expected that the strbuf will be reused for a number of times,
the length of the string each iteration uses is similar, and you
will iterate the loop many times, _reset() each time and _release()
to clean-up pattern would save many calls to realloc/free.
Yep, that's why I suggested strbuf_reset() as an alternative (and
likely would have chosen it myself).
--
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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 09:22:18AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: strbuf_addf(f, %s , fmt); Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable thing to do. It's a hack, but it's contained to the function. Why can't I shake this feeling that ( %s, fmt), i.e. prepend not append, is the safer thing to do than to append? Because then removing the extra space involves `memmove` of the buffer, rather than just shortening the length by one. -Peff -- 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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 6:20 AM, Jeff King p...@peff.net wrote:
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote:
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
{
size_t len;
struct strbuf f = STRBUF_INIT;
/*
* This is a bit tricky since strftime returns 0 if the result did not
* fit in the supplied buffer, as well as when the formatted time has
* zero length. In the former case, we need to grow the buffer and try
* again. To distinguish between the two cases, we supply strftime with
* a format string one character longer than what the client supplied,
* which ensures that a successful format will have non-zero length,
* and then drop the extra character from the formatted time before
* returning.
*/
strbuf_addf(f, %s , fmt);
Basically I was trying to avoid making any assumptions about exactly how
strftime works. But presumably stick a space in the format is a
universally reasonable thing to do. It's a hack, but it's contained to
the function.
I don't think we're making any assumptions about strftime(). POSIX states:
The format string consists of zero or more conversion
specifications and ordinary characters. [...] All ordinary
characters (including the terminating NUL character) are copied
unchanged into the array.
So, we seem to be on solid footing with this approach (even though
it's a localized hack).
do {
strbuf_grow(sb, 128);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len,
f.buf, tm);
} while (!len);
I think we need to keep growing this 128 ourselves, or else each loop
iteration will just say yup, we have 128 bytes available; no need to
grow.
Yeah, I toyed with the idea of increasing the requested amount each
iteration but wanted to keep the example simple, thus left it out.
However, for some reason, I was thinking that strbuf_grow() was
unconditionally expanding the buffer by the requested amount rather
than merely ensuring that that amount was availabile, so the amount
clearly needs to be increased on each iteration. Thanks for pointing
that out.
If this is performance critical code, then the augmented format
string can be constructed with less expensive functions than
strbuf_addf().
This does get called a lot (e.g., once per commit). One extra allocation
would probably not kill us there [...]
Beyond the extra allocation, I was also concerned about the
sledgehammer approach of %s to append a single character when there
are much less expensive ways to do so.
[...] but I think we could fairly trivially put this on the unlikely path:
size_t hint = 128;
size_t len;
/* optimize out obvious 0-length case */
if (!*fmt)
return;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, fmt, tm);
/* maybe not enough room, or maybe 0-length output */
if (!len) {
struct strbuf f = STRBUF_INIT;
strbuf_addf(f, %s , fmt);
while (!len) {
hint *= 2;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, f.buf,
tm);
}
}
I'd guess most cases will fit in 128 bytes and never even hit this code
path. You could also get fancier and start the buffer smaller, but only
do the fmt hack when we cross a threshold.
Yep, I toyed with other looping constructs as well before settling
upon do-while in the example for its simplicity. If called often
enough, this sort of optimization seems reasonable enough.
--
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 3/3] introduce format date-mode
Jeff King p...@peff.net writes:
This does get called a lot (e.g., once per commit). One extra allocation
would probably not kill us there, but I think we could fairly trivially
put this on the unlikely path:
size_t hint = 128;
size_t len;
/* optimize out obvious 0-length case */
if (!*fmt)
return;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, fmt, tm);
/* maybe not enough room, or maybe 0-length output */
if (!len) {
struct strbuf f = STRBUF_INIT;
strbuf_addf(f, %s , fmt);
while (!len) {
hint *= 2;
strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, f.buf,
tm);
}
}
I'd guess most cases will fit in 128 bytes and never even hit this code
path. You could also get fancier and start the buffer smaller, but only
do the fmt hack when we cross a threshold.
I'd assume that the hint thing will persist across calls somehow?
In an invocation of git log --date=format:some format for
millions of commits, it is likely that the length of the formatted
date string will stay the same or close to the same (yeah, I know
Wednesday would be longer than Monday).
Answering myself to my earlier question, the reason is because I was
worried what happens when given fmt is a malformed strftime format
specifier. Perhaps it ends with a lone % and % may format to
something unexpected, or something.
Are we checking an error from strftime(3)?
--
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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 9:26 AM, Jeff King p...@peff.net wrote:
On Mon, Jun 29, 2015 at 06:22:47PM -0400, Eric Sunshine wrote:
Clients of strbuf rightly expect the buffer to grow as needed in
order to complete the requested operation. It is, therefore, both
weird and expectation-breaking for strbuf_addftime() to lack this
behavior. Worse, it doesn't even signal when the format has failed
due to insufficient buffer space.
How about taking this approach (or something similar), instead, which
grows the strbuf as needed?
Here's a patch, on top of jk/date-mode-format (I think it would also be
fine to just squash into the tip commit; the explanation in the commit
message is sufficiently mirrored in the code comment).
As a logical change in itself, I could also see introduction of
strbuf_addftime() split out into its own patch (with this patch
squashed in). Either way, it's a welcome improvement over the
original.
-- 8 --
Subject: [PATCH] strbuf: make strbuf_addftime more robust
The return value of strftime is poorly designed; when it
returns 0, the caller cannot tell if the buffer was not
large enough, or if the output was actually 0 bytes. In the
original implementation of strbuf_addftime, we simply punted
and guessed that our 128-byte hint would be large enough.
We can do better, though, if we're willing to treat strftime
like less of a black box. We can munge the incoming format
to make sure that it never produces 0-length output, and
then fix the resulting output. That lets us reliably grow
the buffer based on strftime's return value.
Clever-idea-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Jeff King p...@peff.net
---
strbuf.c| 38 +-
t/t6300-for-each-ref.sh | 10 ++
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index a7ba028..e5e7370 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -712,29 +712,33 @@ char *xstrfmt(const char *fmt, ...)
void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
{
+ size_t hint = 128;
size_t len;
- /*
-* strftime reports 0 if it could not fit the result in the buffer.
-* Unfortunately, it also reports 0 if the requested time string
-* takes 0 bytes. So if we were to probe and grow, we have to choose
-* some arbitrary cap beyond which we guess that the format probably
-* just results in a 0-length output. Since we have to choose some
-* reasonable cap anyway, and since it is not that big, we may
-* as well just grow to their in the first place.
-*/
- strbuf_grow(sb, 128);
+ if (!*fmt)
+ return;
+
+ strbuf_grow(sb, hint);
len = strftime(sb-buf + sb-len, sb-alloc - sb-len, fmt, tm);
if (!len) {
/*
-* Either we failed, or the format actually produces a
0-length
-* output. There's not much we can do, so we leave it blank.
-* However, the output array is left in an undefined state, so
-* we must re-assert our NUL terminator.
+* strftime reports 0 if it could not fit the result in the
buffer.
+* Unfortunately, it also reports 0 if the requested time
string
+* takes 0 bytes. So our strategy is to munge the format so
that the
+* output contains at least one character, and then drop the
extra
+* character before returning.
*/
- sb-buf[sb-len] = '\0';
- } else {
- sb-len += len;
+ struct strbuf munged_fmt = STRBUF_INIT;
+ strbuf_addf(munged_fmt, %s , fmt);
+ while (!len) {
+ hint *= 2;
+ strbuf_grow(sb, hint);
+ len = strftime(sb-buf + sb-len, sb-alloc - sb-len,
+ munged_fmt.buf, tm);
+ }
+ strbuf_release(munged_fmt);
+ len--; /* drop munged space */
}
+ strbuf_setlen(sb, sb-len + len);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c7f368c..7c9bec7 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -235,6 +235,16 @@ test_expect_success 'Check format of strftime date
fields' '
test_cmp expected actual
'
+test_expect_success 'exercise strftime with odd fields' '
+ echo expected
+ git for-each-ref --format=%(authordate:format:) refs/heads actual
+ test_cmp expected actual
+ long=long format -- $_z40$_z40$_z40$_z40$_z40$_z40$_z40
+ echo $long expected
+ git for-each-ref --format=%(authordate:format:$long) refs/heads
actual
+ test_cmp expected actual
+'
+
cat expected \EOF
refs/heads/master
Re: [PATCH 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote: Basically I was trying to avoid making any assumptions about exactly how strftime works. But presumably stick a space in the format is a universally reasonable thing to do. It's a hack, but it's contained to the function. I don't think we're making any assumptions about strftime(). POSIX states: The format string consists of zero or more conversion specifications and ordinary characters. [...] All ordinary characters (including the terminating NUL character) are copied unchanged into the array. So, we seem to be on solid footing with this approach (even though it's a localized hack). Yeah, sorry I wasn't more clear. I had originally been thinking of making assumptions like well, %c cannot ever be blank. But your solution does not suffer from that level of knowledge. I think it is reasonably clever. Yeah, I toyed with the idea of increasing the requested amount each iteration but wanted to keep the example simple, thus left it out. However, for some reason, I was thinking that strbuf_grow() was unconditionally expanding the buffer by the requested amount rather than merely ensuring that that amount was availabile, so the amount clearly needs to be increased on each iteration. Thanks for pointing that out. FWIW, I had to look at it to double-check. I've often made the same mistake. Beyond the extra allocation, I was also concerned about the sledgehammer approach of %s to append a single character when there are much less expensive ways to do so. I don't think there's any other way. We have to feed a contiguous buffer to strftime, and we don't own the buffer, so we have to make a new copy. -Peff -- 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 0/3] rebase -i: drop, missing commits and static checks
Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: The place where an error can be introduced is (assuming that what rebase -i writes out itself is perfect ;-) where we allow the user to edit, so instead of checking before --continue, I would expect a sane design would check immediately after the editor we spawned returns. Makes sense but we would have the problem mentioned by Matthieu: Warning: the command isn't recognized ... # Hmm, let's ignore that warning $ git rebase --continue There's an alternative: $ git rebase --edit-todo # Make mistakes, save and quit Your todo-list has the following issues: - ... Do you want to edit again (no aborts the rebase) [Y/n]? There's a precedent with the 'e' command of git add -p. I have a slight preference for non-interactive commands so I prefer not going this way though. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/PATCH] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: * t2025-checkout-to.sh became t2025-worktree-new.sh. I'm not sure if the test number still makes sense or if it should be changed, however, it resides alongside its t2026-prune-linked-checkouts.sh counterpart. You'd need to adjust t7410 as well, perhaps like so: diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh index 8f30aed..d037e51 100755 --- a/t/t7410-submodule-checkout-to.sh +++ b/t/t7410-submodule-checkout-to.sh @@ -33,7 +33,7 @@ rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q HEAD~1 test_expect_success 'checkout main' \ 'mkdir default_checkout (cd clone/main - git checkout --to $base_path/default_checkout/main $rev1_hash_main)' + git worktree new $base_path/default_checkout/main $rev1_hash_main)' test_expect_failure 'can see submodule diffs just after checkout' \ '(cd default_checkout/main git diff --submodule master^! | grep file1 updated)' @@ -41,7 +41,7 @@ test_expect_failure 'can see submodule diffs just after checkout' \ test_expect_success 'checkout main and initialize independed clones' \ 'mkdir fully_cloned_submodule (cd clone/main - git checkout --to $base_path/fully_cloned_submodule/main $rev1_hash_main) + git worktree new $base_path/fully_cloned_submodule/main $rev1_hash_main) (cd fully_cloned_submodule/main git submodule update)' test_expect_success 'can see submodule diffs after independed cloning' \ -- 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 0/3] rebase -i: drop, missing commits and static checks
Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Galan Alfonso remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Galan Rémi remi.galan-alfo...@ensimag.grenoble-inp.fr writes: Shouldn't all the checking also be called in a 'rebase --continue', considering that it can be called after a 'rebase --edit-todo' ? (Right now it is only called after closing the editor in 'rebase -i') What's your opinion on it? It would probably be better to run the check when running git rebase --continue. This way, we would have a guarantee that the todo-list is syntactically correct when going through it. Just checking after --edit-todo would not guarantee that: That's actually what I had in mind, my message wasn't clear enough, my bad. But in any case, the most important is the initial edition. It covers 99.9% of use-cases. So, I'd say: if you have time to add the checks at the other relevant places, good, but not doing it shouldn't block the inclusion of the series. Agreed, that's something that can be done in a future patch without interfering with this one. Junio C Hamano gits...@pobox.com writes: The place where an error can be introduced is (assuming that what rebase -i writes out itself is perfect ;-) where we allow the user to edit, so instead of checking before --continue, I would expect a sane design would check immediately after the editor we spawned returns. Makes sense but we would have the problem mentioned by Matthieu: Warning: the command isn't recognized ... # Hmm, let's ignore that warning $ git rebase --continue While in most cases it would be irrelevant (it would be the user that ignored the error on purpose), I can imagine the following situation: - 'git rebase --edit-todo' - get an error - go do something else, forgot where I was when I come back - 'git status', you are in the middle of a rebasing - 'git rebase --continue' The codepath that allows the insn sheet getting edited and the codepath that handles --edit-todo have to do many things the same way (i.e. use collapse_todo_ids to prepare the file for editing, spawn the editor, receive the result and use expand_todo_ids to prepare the file to be used by us), and I would have expected for these two to be using a single helper---and a sanity check like the above can and should be done when we receive the result from the editor, immediately before running expand_todo_ids perhaps. If they are not using such a single helper right now, perhaps that is the preliminary restructuring of the code that is needed? Good point, maybe I'll try doing that at a later date, however as Matthieu said, I think it doesn't hurt to add this patch (if there is no further corrections to do) and do additional things in a later patch. Thanks, Rémi -- 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 0/3] rebase -i: drop, missing commits and static checks
Matthieu Moy matthieu@grenoble-inp.fr writes: There's an alternative: $ git rebase --edit-todo # Make mistakes, save and quit Your todo-list has the following issues: - ... Do you want to edit again (no aborts the rebase) [Y/n]? There's a precedent with the 'e' command of git add -p. I have a slight preference for non-interactive commands so I prefer not going this way though. edit-todo (and rebase -i in general) is all about going interactive, isn't it? I was almost going to suggest to keep spawning the editor until the user gets it right, but that would infinitely repeat when GIT_EDITOR is set to a broken script, so I didn't ;-). -- 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] Avoid the need of -- when wildcard pathspec is used
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: When -- is lacking from the command line and a command can take both revs and paths, the idea is if an argument can be seen as both an extended SHA-1 and a path, then -- is required or git refuses to continue. It's currently implemented as: ... Hmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of -- when wildcard is used, 2015-05-02)? A follow-up? Oops, I did it wrong? something else? diff --git a/setup.c b/setup.c index 82c0cc2..f7cb93b 100644 --- a/setup.c +++ b/setup.c @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg) name = arg + 2; } else if (!no_wildcard(arg)) return 1; + else if (!no_wildcard(arg)) + return 1; Puzzling. You already checked if arg has an wildcard and returned with 1 if there is none. The added check looks like a no-op to me. diff --git a/t/t2019-checkout-ambiguous-ref.sh b/t/t2019-checkout-ambiguous-ref.sh index b99d519..e5ceba3 100755 --- a/t/t2019-checkout-ambiguous-ref.sh +++ b/t/t2019-checkout-ambiguous-ref.sh @@ -56,4 +56,30 @@ test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' ' test_i18ngrep ! ^HEAD is now at stderr ' +test_expect_success 'wildcard ambiguation' ' + git init ambi + ( + cd ambi + echo a a.c + git add a.c + echo b a.c + git checkout *.c + echo a expect + test_cmp expect a.c + ) +' + +test_expect_success 'wildcard ambiguation (2)' ' + git init ambi2 + ( + cd ambi2 + echo a *.c + git add . + test_must_fail git show :*.c + git show :*.c -- actual + echo a expect + test_cmp expect actual + ) +' + test_done -- 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 v7 07/10] send-email: reduce dependencies impact on parse_address_line
Matthieu Moy matthieu@imag.fr writes: diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 000..7223d03 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# +# Copyright (c) 2015 That does not look like a valid copyright notice. In the modern age, I'd personally perfer not to add one (I would not have a strong objection to others asserting their copyright), but if you want to add one, you would need the name of the copyright holder after the year (I presume that it would be your school name?). IIRC, (c) in place of circle-C does no carry legal weight, but having the word Copyright spelled out there is sufficient. Thanks for tying the loose ends (not just this topic, but the other ones, too). Very much appreciated. -- 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/PATCH] worktree: replace checkout --to with worktree new
Duy Nguyen pclo...@gmail.com writes: I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. I didn't know checkout --to included create a worktree elsewhere and chdir there; if that and chdir there is not something you are doing, then I do not think checkout -b vs branch analogy applies. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I'm in favor of removing --to before it escapes the lab. I am ambivalent about new, but that is only because I know about the 'new-workdir' in contrib/. If I pretend to be a naive end user, I'd think a verb subcommand would be more in line with the rest of the system than new. I however do not think create is a good verb. Wouldn't git worktree $the-command-in-question be a management command that adds a new worktree to the existing collection, like remote add, notes add, etc. do? Perhaps git worktree list and git worktree remove $that_one would be in its future? That suggests add may be a better choice for worktree. The only subcommand that I can think of offhand that says create is bundle; after generates a new bundle, its presence is not known to the repository the bundle was created out of, so not using add but calling the operation create is fine for bundle. -- 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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 1:05 PM, Junio C Hamano gits...@pobox.com wrote: Answering myself to my earlier question, the reason is because I was worried what happens when given fmt is a malformed strftime format specifier. Perhaps it ends with a lone % and % may format to something unexpected, or something. That's a good point. I had considered prepending the extra character (space) rather than appending it but eventually rejected it to avoid the expense of shifting the characters down by one before returning the formatted string. However, is it our responsibility to guard against a malformed format? POSIX doesn't state the behavior of % , so I don't think we are any worse off by appending space to a malformed format ending with % since the malformed format could wreak havoc even without our transformation. Are we checking an error from strftime(3)? According to the BUGS section in POSIX: If the output string would exceed max bytes, errno is not set. This makes it impossible to distinguish this error case from cases where the format string legitimately produces a zero-length output string. POSIX.1-2001 does not specify any errno settings for strftime(). So, there does not seem to be a point in checking 'errno'. -- 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 v6 6/7] git-reflog: add create and exists functions
Eric Sunshine sunsh...@sunshineco.com writes:
Alternatives would be strbuf_reset() or declaring and releasing the
strbuf within the for-loop scope.
Because _reset() just rewinds the .len pointer without deallocating,
you would need an extra _release() before it goes out of scope. If
it is expected that the strbuf will be reused for a number of times,
the length of the string each iteration uses is similar, and you
will iterate the loop many times, _reset() each time and _release()
to clean-up pattern would save many calls to realloc/free.
Yep, that's why I suggested strbuf_reset() as an alternative (and
likely would have chosen it myself).
OK, then let's do that by squashing this in.
builtin/reflog.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3080865..e9ba600 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -733,10 +733,11 @@ static int cmd_reflog_create(int argc, const char **argv,
const char *prefix)
if (safe_create_reflog(argv[i], err, 1)) {
error(could not create reflog %s: %s, argv[i],
err.buf);
+ strbuf_reset(err);
status = 1;
- strbuf_release(err);
}
}
+ strbuf_release(err);
return status;
}
--
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: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Am 30.06.2015 um 19:15 schrieb Konstantin Khomoutov: On Mon, 29 Jun 2015 18:19:09 +0200 Johannes Schindelin johannes.schinde...@gmx.de wrote: I've finally took time to switch from my old msys1 release to this RC4, and immediately got hit by the fact Git is now speaking to me in Russian, which is not what I want (previously this behaviour was only exhibited by `git gui` and `gitk`). Should I make Git see LC_MESSAGES=en (or other thing like LANG) in the environment or is there some Git-local method to affect this behaviour? I tried to grep the release notes using relevant keywords but was left empty-handed. Personally, I would use LC_ALL=C. Maybe that's good for you, too? After reading [1], I've ended up installing LANG=C into my user's environment variables -- so far so good, thanks for the tip! Just for the record. I created the file lang.sh with contents export LC_ALL=C in /etc/profile.d which also fixes the problem. And also survives new versions of git. -- 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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 10:05:33AM -0700, Junio C Hamano wrote: I'd guess most cases will fit in 128 bytes and never even hit this code path. You could also get fancier and start the buffer smaller, but only do the fmt hack when we cross a threshold. I'd assume that the hint thing will persist across calls somehow? In an invocation of git log --date=format:some format for millions of commits, it is likely that the length of the formatted date string will stay the same or close to the same (yeah, I know Wednesday would be longer than Monday). I hadn't thought about that. It could persist, but I don't think this is necessarily the right place to do it. For two reasons: 1. You have no idea in strbuf_addftime if it's the same fmt being added over and over. This is the wrong place to make that optimization. 2. If you are interested in efficiency in a loop, then you should be reusing the same strbuf over and over, and avoiding the extra allocation in the first place. And that is indeed what we do for git log --date, as we will always use the same static-local buffer in show_date(). Answering myself to my earlier question, the reason is because I was worried what happens when given fmt is a malformed strftime format specifier. Perhaps it ends with a lone % and % may format to something unexpected, or something. Are we checking an error from strftime(3)? POSIX doesn't define any errno values for strftime (and in fact says No errors are defined). The return value description for POSIX (and the glibc manpage) talk about only whether or not the output fits. However, POSIX does say If a conversion specifier is not one of the above, the behavior is undefined. So certainly I could imagine an implementation that returns 0 when you feed it a bogus value. If you (as a user) feed us crap to give to strftime, I am not particularly concerned with whether you get crap out. My main concern is that it would return 0 and we would loop forever. OTOH, I think any sane implementation would simply copy unknown placeholders out (certainly glibc does that). So I think we could simply consider it a quality of implementation issue, and deal with any particular crappy implementations if and when they get reported. We could add something tricky (like --date=format:%) to the test suite to make it likelier to catch such a thing. -Peff -- 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 3/3] introduce format date-mode
On Tue, Jun 30, 2015 at 02:13:53PM -0400, Eric Sunshine wrote: Sorry, I meant that the interpolation expense of %s . A cheaper (but more verbose) alternative might be: size_t n = strlen(fmt); const char *f = xmalloc(n + 2); strcpy(f, fmt); f[n] = ' '; f[n + 1] = '\0'; ... free(f); or something similar. I think you're probably getting into premature optimization here. Using strbuf_vaddf should be within the same order of magnitude of instructions (and I think we should almost never hit this code path anyway, because we'll be reading into a static strbuf generally which will come pre-sized to hold a date). -Peff -- 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 v7 07/10] send-email: reduce dependencies impact on parse_address_line
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@imag.fr writes: diff --git a/t/t9000-addresses.sh b/t/t9000-addresses.sh new file mode 100755 index 000..7223d03 --- /dev/null +++ b/t/t9000-addresses.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# +# Copyright (c) 2015 That does not look like a valid copyright notice. In the modern age, I'd personally perfer not to add one I'd vote for keeping it simple and not having the copyright notice. Most t/*.sh do not have one. The Git history + mailing-list archives are much better than in-code comments to keep track of who wrote what. Remi: any objection on removing it? Junio: do you want me to resend? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 0/3] rebase -i: drop, missing commits and static checks
Hi, Here are a few fixes to squash into the commits of the series. Other than that, the series looks good to me. Junio: do you prefer a reroll or do you want to apply locally? Matthieu Moy (3): fixup! git rebase -i: add static check for commands and SHA-1 fixup! git rebase -i: warn about removed commits fixup! git rebase -i: warn about removed commits git-rebase--interactive.sh| 32 +--- t/t3404-rebase-interactive.sh | 4 ++-- 2 files changed, 23 insertions(+), 13 deletions(-) -- 2.5.0.rc0.10.g7792c2a -- 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/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. As a side-effect, linked worktree creation becomes much more discoverable with its own dedicated command, whereas `--to` was easily overlooked amid the plethora of options recognized by git-checkout. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- I've long felt that Duy's linked-worktree functionality was a bit oddly named as git checkout --to, but, since I could never come up with a better name, I never made mention of it. However, with Duy's introduction of the git-worktree command[1], we now have a much more appropriate and discoverable place to house the git checkout --to functionality, and upon seeing his patch, I was ready to reply with the suggestion to relocate git checkout --to to git worktree new, however, Junio beat me to it[2]. Didn't know you guys were so eager to move this code around :D Jokes aside, it's good that it's raised now before --to is set in stone. I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch and if I had a way to tell checkout to move away and delete previous branch, I would probably stop using git branch -d/-D too. --to is another -b in this sense. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. Maybe --to is not used that often that git worktree new would feel less convenient as a replacement. If we are not sure about --to (I'm not), I think we just remove it now because we can always add it back later. diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 41103e5..8f13281 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,16 +9,85 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] +'git worktree new' [-f] path [checkout-options] branch Should we follow clone syntax and put the path (as destination) after branch (source)? Maybe not, because in the clone case, explicit destination is optional, not like this.. Or.. maybe branch could be optional in this case. 'git worktree new' without a branch will create a new branch, named closely after the destination. Existing branch can be specified via an option.. -- Duy -- 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 3/3] fixup! git rebase -i: warn about removed commits
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0117791..8090d80 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -985,7 +985,7 @@ check_todo_list () {
warn To avoid this message, use \drop\ to \
explicitly remove a commit.
warn
- warn Use 'git --config rebase.missingCommitsCheck' to
change \
+ warn Use 'git config rebase.missingCommitsCheck' to
change \
the level of warnings.
warn The possible behaviours are: ignore, warn, error.
warn
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9b2c51c..ebdab4b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1140,7 +1140,7 @@ Dropped commits (newer to older):
- $(git rev-list --pretty=oneline --abbrev-commit -1 master)
To avoid this message, use drop to explicitly remove a commit.
-Use 'git --config rebase.missingCommitsCheck' to change the level of warnings.
+Use 'git config rebase.missingCommitsCheck' to change the level of warnings.
The possible behaviours are: ignore, warn, error.
Successfully rebased and updated refs/heads/missing-commit.
@@ -1163,7 +1163,7 @@ Dropped commits (newer to older):
- $(git rev-list --pretty=oneline --abbrev-commit -1 master~2)
To avoid this message, use drop to explicitly remove a commit.
-Use 'git --config rebase.missingCommitsCheck' to change the level of warnings.
+Use 'git config rebase.missingCommitsCheck' to change the level of warnings.
The possible behaviours are: ignore, warn, error.
You can fix this with 'git rebase --edit-todo'.
--
2.5.0.rc0.10.g7792c2a
--
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 1/3] fixup! git rebase -i: add static check for commands and SHA-1
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-rebase--interactive.sh | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ec4a068..9041d15 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -871,8 +871,7 @@ check_bad_cmd_and_sha () {
while read -r line
do
IFS=' '
- set x $line
- shift
+ set -- $line
command=$1
sha1=$2
@@ -881,8 +880,7 @@ check_bad_cmd_and_sha () {
# Doesn't expect a SHA-1
;;
pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
- check_commit_sha $sha1 $line
- if test $? -ne 0
+ if ! check_commit_sha $sha1 $line
then
retval=1
fi
@@ -988,8 +986,7 @@ check_todo_list () {
;;
esac
- check_bad_cmd_and_sha $todo
- if test $? -ne 0
+ if ! check_bad_cmd_and_sha $todo
then
raise_error=t
fi
--
2.5.0.rc0.10.g7792c2a
--
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 2/3] fixup! git rebase -i: warn about removed commits
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
git-rebase--interactive.sh | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9041d15..0117791 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -156,8 +156,17 @@ Commands:
These lines can be re-ordered; they are executed from top to bottom.
+EOF
+ if test $(get_missing_commit_check_level) = error
+ then
+ git stripspace --comment-lines $todo \EOF
+Do not remove any line. Use 'drop' explicitly to remove a commit.
+EOF
+ else
+ git stripspace --comment-lines $todo \EOF
If you remove a line here THAT COMMIT WILL BE LOST.
EOF
+ fi
}
make_patch () {
@@ -931,6 +940,13 @@ checkout_onto () {
git update-ref ORIG_HEAD $orig_head
}
+get_missing_commit_check_level () {
+ check_level=$(git config --get rebase.missingCommitsCheck)
+ check_level=${check_level:-ignore}
+ # Don't be case sensitive
+ printf '%s' $check_level | tr 'A-Z' 'a-z'
+}
+
# Check if the user dropped some commits by mistake
# Behaviour determined by rebase.missingCommitsCheck.
# Check if there is an unrecognized command or a
@@ -938,10 +954,7 @@ checkout_onto () {
check_todo_list () {
raise_error=f
- check_level=$(git config --get rebase.missingCommitsCheck)
- check_level=${check_level:-ignore}
- # Don't be case sensitive
- check_level=$(printf '%s' $check_level | tr 'A-Z' 'a-z')
+ check_level=$(get_missing_commit_check_level)
case $check_level in
warn|error)
--
2.5.0.rc0.10.g7792c2a
--
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/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. [...] Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- This is primarily a code and documentation relocation patch, with minor new code added to builtin/worktree.c. Specifically: * builtin/worktree.c:new() is new. It recognizes a --force option (git worktree new --force path branch) which allows a branch to be checked out in a new worktree even if already checked out in some other worktree (thus, mirroring the functionality of git checkout --ignore-other-worktrees). Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. -- 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] revision.c: Remove unneeded check for NULL
Jeff King wrote: On Fri, Jun 26, 2015 at 12:40:19PM -0700, Stefan Beller wrote: This code seems to be underdocumented. I am not a expert in this area of the code, so I hoped Peff would document it if he feels like so. I kind of thought that the explanation in b6e8a3b covered this code. Does it not, or did people not read it? I know that tl;dr is the last thing anyone who has written a clear description of something wants to read, but I fear it applies here. I tried to skim that commit message to get a gist of what the still_interesting variable is supposed to hold and I failed. I think part of the problem was that that commit message doesn't give a specific example early on to motivate the problem and fix[*]. More to the point, someone interested in that specific variable doesn't need to necessarily understand the optimization that motivated it. Instead, they'd want to know what invariants to expect and preserve: what value does it start with, what does its value mean, are there some forbidden values, etc. Is the idea that it represents a commit from the queue which is still interesting, and that it saves us from looping through the queue to find a still-interesting commit as long as mark_parents_uninteresting has not marked this one uninteresting yet? What does it mean when it is NULL? Thanks, Jonathan [*] I.e., what command do I run to get quadratic behavior? The message starts with a diagnosis --- When we are limiting a rev-list traversal due to UNINTERESTING refs, we have to walk down the tips --- without introducing what problem is being diganosed. The problem being solved might have been something like When we call 'git rev-list $commits --not --all' in check_everything_connected after a fetch, if we fetched something much older than most of our refs, and if we have a large number of refs, the operation is slow --- quadratic in the number of refs. This hasn't been a problem in the past because people did not use so many refs, but now as the number of refs in a typical repository grows, it is becoming more noticeable. Even after re-reading the message more carefully, I'm not sure. I assume there was a report motivating the change, which might have been useful for putting the explanation in context for the reader. Alas, git://repo.or.cz/git/trast.git branch notes/gmane doesn't have any annotations for that commit to find the context. The commit message then goes on to explain how the patch solves that problem, but without an example to put that explanation in context, it is hard to follow. What linear search is the explanation talking about? What is the interesting commit we find? I couldn't tell without looking at the code. -- 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] Avoid the need of -- when wildcard pathspec is used
On Wed, Jul 1, 2015 at 1:10 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: When -- is lacking from the command line and a command can take both revs and paths, the idea is if an argument can be seen as both an extended SHA-1 and a path, then -- is required or git refuses to continue. It's currently implemented as: ... Hmph, how does this relate to 28fcc0b7 (pathspec: avoid the need of -- when wildcard is used, 2015-05-02)? A follow-up? Oops, I did it wrong? something else? Arghhh! I vaguely recalled I sent something but I couldn't find it and.. diff --git a/setup.c b/setup.c index 82c0cc2..f7cb93b 100644 --- a/setup.c +++ b/setup.c @@ -143,6 +143,8 @@ int check_filename(const char *prefix, const char *arg) name = arg + 2; } else if (!no_wildcard(arg)) return 1; .. if I looked at the context lines, I should have noticed the change was already here! + else if (!no_wildcard(arg)) + return 1; Seems strange (or expected?) that git cherry-pick just adds this chunk on top anyway.. Puzzling. You already checked if arg has an wildcard and returned with 1 if there is none. The added check looks like a no-op to me. Yeah sorry for the noise. The only value this patch adds is tests (and maybe better commit message, the last one still mentions magic pathspec even though it's not about it). I think we can just drop this. -- Duy -- 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/PATCH] worktree: replace checkout --to with worktree new
Eric Sunshine sunsh...@sunshineco.com writes: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. git checkout -f is to throw-away local changes, which is a very sensible thing to do and I can see why that would be useful, but does --ignore-other-worktrees have the same kind of common-ness? It primarily is a safety measure, and if the user wants to jump around freely to different commits in multiple worktrees, a more sensible thing to do so without getting the nono, you have that branch checked out elsewhere is to detach HEADs in the non-primary worktrees that may want to have the same commit checked out as the current branch of the primary worktree. I would mildly object to make --ignore-other-worktrees more discoverable and moderately object to make that feature more accessible by overloading it into --force. I personally would not mind if we removed --ignore-other-worktrees, but that might be going too far ;-) -- 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 v6 6/7] git-reflog: add create and exists functions
On Tue, 2015-06-30 at 12:48 -0700, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: Alternatives would be strbuf_reset() or declaring and releasing the strbuf within the for-loop scope. Because _reset() just rewinds the .len pointer without deallocating, you would need an extra _release() before it goes out of scope. If it is expected that the strbuf will be reused for a number of times, the length of the string each iteration uses is similar, and you will iterate the loop many times, _reset() each time and _release() to clean-up pattern would save many calls to realloc/free. Yep, that's why I suggested strbuf_reset() as an alternative (and likely would have chosen it myself). OK, then let's do that by squashing this in. builtin/reflog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) I squashed that into my repo on github: https://github.com/dturner-tw/git.git on the branch dturner/pluggable-backends-preamble -- 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 v4 0/4] More helpful 'git status' during 'rebase -i'
Matthieu Moy matthieu@imag.fr writes: This series makes git status provide an output like interactive rebase in progress; onto $ONTO Last commands done (2 commands done): pick $COMMIT2 two_commit exec exit 15 Next commands to do (2 remaining commands): pick $COMMIT3 three_commit pick $COMMIT4 four_commit (use git rebase --edit-todo to view and edit) in addition to the existing output, when ran during an interactive rebase. I'd prefer to see these $COMMITn abbreviated, just like $ONTO. Look what I just got while squashing two adjacent patches ;-) # interactive rebase in progress; onto a04bfc2 # Last commands done (2 commands done): #pick c186b073f46a3298f2e6f63a8c1becb07bedc4f0 rerere: explain what 'want_sp' does to is_cmarker #squash 17c5b40b46c0e171ed49907e6cb91c2a1d7f7113 rerere: drop want_sp parameter from is_cmarker() # Next commands to do (3 remaining commands): #pick 8fc64c4c1024006e71cf0b6c2e3d0ad403f263f8 t4200: rerere a merge with two identical conflicts #pick 094950cdc51599f6fec1b1c0098816888a648bf8 rerere: document internal I/O abstraction # You are currently editing a commit while rebasing branch 'jc/rerere' on 'a04bfc2'. -- 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 v6 6/7] git-reflog: add create and exists functions
David Turner dtur...@twopensource.com writes: On Tue, 2015-06-30 at 12:48 -0700, Junio C Hamano wrote: Eric Sunshine sunsh...@sunshineco.com writes: Alternatives would be strbuf_reset() or declaring and releasing the strbuf within the for-loop scope. Because _reset() just rewinds the .len pointer without deallocating, you would need an extra _release() before it goes out of scope. If it is expected that the strbuf will be reused for a number of times, the length of the string each iteration uses is similar, and you will iterate the loop many times, _reset() each time and _release() to clean-up pattern would save many calls to realloc/free. Yep, that's why I suggested strbuf_reset() as an alternative (and likely would have chosen it myself). OK, then let's do that by squashing this in. builtin/reflog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) I squashed that into my repo on github: I'm already deep into today's integration cycle, so it is unlikely I'd pull that before I push the result out. Please let me know if the resulting tree looks wrong (I only queued it to be squashed, haven't done the squashing two into one yet). Thanks. The 7-patch series with this fixup looks good to me. -- 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/PATCH] worktree: replace checkout --to with worktree new
On Tue, Jun 30, 2015 at 5:23 AM, Duy Nguyen pclo...@gmail.com wrote: On Tue, Jun 30, 2015 at 11:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. I think this is like git checkout -b vs git branch. We pack so many things in 'checkout' that it's a source of both convenience and confusion. I never use git branch to create a new branch [...] --to is another -b in this sense. I too always use git checkout -b, but, like Junio, I don't think this is an apt analogy. git checkout -b is shorthand for two commands git branch and git checkout, whereas git checkout --to is not. git worktree new definitely makes sense (maybe stick with verbs like create, I'm not sure if we have some convention in existing commands), but should we remove git checkout --to? I could do git co -b foo --to bar for example. You can still do that with git worktree new bar -b foo, which is effectively the same as git checkout --to bar -b foo (with s/checkout/worktree/ and s/--to/new/ applied), though perhaps you don't find it as obvious or natural. If we are not sure about --to (I'm not), I think we just remove it now because we can always add it back later. I'm not excited about keeping git checkout --to as an alias for git worktree new, however, removing it now should not harm us since, as you say, it can be added back later if needed. SYNOPSIS +'git worktree new' [-f] path [checkout-options] branch Should we follow clone syntax and put the path (as destination) after branch (source)? Maybe not, because in the clone case, explicit destination is optional, not like this.. Or.. maybe branch could be optional in this case. 'git worktree new' without a branch will create a new branch, named closely after the destination. Existing branch can be specified via an option.. I'm not wedded to this particular argument order, though it does have the advantage that it's clear which options belong to worktree new and which to checkout. As for making branch optional and auto-vivifying a new branch named after path, that's something we can consider later (I think). -- 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/PATCH] worktree: replace checkout --to with worktree new
On 06/30/2015 06:11 PM, Eric Sunshine wrote: On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote: The command git checkout --to path is something of an anachronism, encompassing functionality somewhere between checkout and clone. The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement git checkout --to as git worktree new. [...] Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- This is primarily a code and documentation relocation patch, with minor new code added to builtin/worktree.c. Specifically: * builtin/worktree.c:new() is new. It recognizes a --force option (git worktree new --force path branch) which allows a branch to be checked out in a new worktree even if already checked out in some other worktree (thus, mirroring the functionality of git checkout --ignore-other-worktrees). Speaking of git worktree new --force, should we revisit git checkout --ignore-other-worktrees before it gets set in stone? In particular, I'm wondering if it makes sense to overload git-checkout's existing --force option to encompass the functionality of --ignore-other-worktrees as well. I don't think there would be any semantic conflict by overloading --force, and I do think that --force is more discoverable and more intuitive. I agree with -f subsuming --ignore...: -f/--force should really mean do this if at all possible, not just ignore some checks. Similar to rm -f, etc. Maintaining --ignore-other-worktrees, and making that a configurable option (worktree.ignoreothers??) would allow selectively ignoring just this one issue, perhaps permanently, but not the others -f already overrides. This would make sense if other options were added to ignore other subsets of checks that can block a checkout, probably not otherwise. Mark -- 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
