* Re: [PATCH 1/3] update_unicode.sh: update the uniset repo if it exists
From: Beat Bolli @ 2016-12-12 19:24 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <ca10a51a-0fab-e4a4-8d7d-035673af4c06@web.de>
On 12.12.16 19:12, Torsten Bögershausen wrote:
>
>>> Minor question, especially to the next commit:
>>> Should we make sure to checkout the exact version, which has been tested?
>>> In this case cb97792880625e24a9f581412d03659091a0e54f
>>>
>>> And this is for both a fresh clone and the git pull
>>> needs to be replaced by
>>> git fetch && git checkout cb97792880625e24a9f581412d03659091a0e54f
>>>
>>>
>>> (Which of course is a shell variable)
>>
>> I was actually wondering what the policy was for adding submodules to the Git repo,
>> but then decided against it. Another option would be to fork uniset on GitHub and
>> just let it stay on a working commit.
>>
>> Junio, what's your stance on this?
>>
>> Beat
>
> If I run ./update_unicode.sh on the latest master of https://github.com/depp/uniset.git ,
> commit a5fac4a091857dd5429cc2d, I get a diff in unicode_width.h like this:
>
> -{ 0x0300, 0x036F },
>
> +{ 768, 879 },
>
> IOW, all hex values are printed as decimal values.
> Not a problem for the compiler, but for the human
> to check the unicode tables.
>
> So I think we should "pin" the version of uniset.
That's what patch 3/3 fixes.
^ permalink raw reply
* Re: [RFC PATCH] send-email: allow a custom hook to prevent sending email
From: Stefan Beller @ 2016-12-12 19:35 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git@vger.kernel.org
In-Reply-To: <20161210091333.rhssskrvzjqnzelb@sigill.intra.peff.net>
On Sat, Dec 10, 2016 at 1:13 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Dec 09, 2016 at 12:34:49PM -0800, Stefan Beller wrote:
>
>> My first perl contribution to Git. :)
>
> Yes, I have some style suggestions below. :)
>
>> Marked as RFC to gauge general interest before writing tests and
>> documentation.
>
> It's hard to evaluate without seeing an example of what you'd actually
> want to put into a hook.
>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index da81be40cb..d3ebf666c3 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -815,6 +815,15 @@ if (!$force) {
>> . "Pass --force if you really want to send.\n";
>> }
>> }
>> + my @hook = ( $ENV{GIT_DIR}.'hooks/send-email', $f )
>
> It's awkward to use a list here, when you just end up accessing
> $hook[0]. You can form the list on the fly when you call system(). You
> also probably are missing a trailing "/".
>
> I'm not sure that $GIT_DIR is consistently set; you probably want to
> look at "git rev-parse --git-dir" here. But I think we make a Git
> repository object earlier, and you can just do:
>
> my $hook = join('/', $repo->repo_path(), 'hooks/send-email');
>
> Or you can just do string concatenation:
>
> my $hook = $repo->repo_path() . '/hooks/send-email';
>
> If I were writing repo_path myself, I'd probably allow:
>
> my $hook = $repo->repo_path('hooks/send-email');
>
> like we do with git_path() in the C code. It wouldn't be hard to add.
>
>> + if( -x $hook[0] ) {
>
> Funny whitespace. I think:
>
> if (-x $hook)
>
> would be more usual for us.
>
>> + unless( system( @hook ) == 0 )
>> + {
>> + die "Refusing to send because the patch\n\t$f\n"
>> + . "was refused by the send-email hook."
>> + . "Pass --force if you really want to send.\n";
>> + }
>
> I like "unless" as a trailing one-liner conditional for edge cases,
> like:
>
> do_this($foo) unless $foo < 5;
>
> but I think here it just makes things more Perl-ish than our code base
> usually goes for. Probably:
>
> if (system($hook, $f) != 0) {
> die ...
> }
>
> would be more usual for us (in a more Perl-ish code base I'd probably
> write "system(...) and die ...").
>
> -Peff
Oh, that is quite some feedback on how not to write perl.
Thanks for pointing out how you would do it. My patch was heavily inspired
by git-cvsserver.perl:1720 so maybe that is not the best example to follow.
While trying to figure out the right place, where to put the actual code, I was
wondering about the possible interaction with it, e.g. looking at
output like this
$ git send-email 00* --cc=list --cc=bmwill --cc=duy --to=jch
0000-cover-letter.patch
0001-submodule-use-absolute-path-for-computing-relative-p.patch
0002-submodule-helper-support-super-prefix.patch
0003-test-lib-functions.sh-teach-test_commit-C-dir.patch
0004-worktree-check-if-a-submodule-uses-worktrees.patch
0005-move-connect_work_tree_and_git_dir-to-dir.h.patch
0006-submodule-add-absorb-git-dir-function.patch
(mbox) Adding cc: Stefan Beller <sbeller@google.com> from line 'From:
Stefan Beller <sbeller@google.com>'
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org,
bmwill@google.com,
pclouds@gmail.com,
Stefan Beller <sbeller@google.com>
Subject: [PATCHv7 0/6] submodule absorbgitdirs
Date: Mon, 12 Dec 2016 11:04:29 -0800
Message-Id: <20161212190435.10358-1-sbeller@google.com>
X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
Send this email? ([y]es|[n]o|[q]uit|[a]ll):
---
This is usually where I just say "a" and carry on with life.
The hook would ideally reformat the last lines to be
---
X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
send-email hook thinks it is a bad idea to send out this series:
<output of hook, e.g.
referencing a typo in patch 5.
or a mismatch of patch version numbers>
Send this email? ([y]es|[n]o|[q]uit|[a]ll):
---
So I still need to look around to see the big picture for this
patch to be implemented ideal.
^ permalink raw reply
* [PATCH 0/2] handling alternates paths with colons
From: Jeff King @ 2016-12-12 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161210085133.2pnkz6eqlxoxdckg@sigill.intra.peff.net>
On Sat, Dec 10, 2016 at 03:51:33AM -0500, Jeff King wrote:
> So here's the array of options, as I see it:
>
> 1. Disable quarantine by default; only turn it on if you don't have
> any of the funny corner cases.
>
> 2. Introduce an extra single-item environment variable that gets
> appended to the list of alternates, and side-steps quoting issues.
>
> 3. Introduce a new variable that can hold an alternate list, and
> either teach it reasonable quoting semantics, or give it a
> less-common separator.
>
> 4. Introduce quoting to the existing variable, but make the quoting
> character sufficiently obscure that nobody cares about the lack of
> backwards compatibility.
So I started on (4), but writing the user-facing documentation made me
throw up in my mouth a little. Fortunately that inspired me to come up
with a fifth option: introduce a quoting mechanism that needs a more
obscure signature to kick in, but once kicked-in, uses a less obscure
syntax.
So here are patches that do that. It kicks in only when the first
character of a path is a double-quote, and then expects the usual
C-style quoting. My reasoning is that we wouldn't want to sacrifice
'"' for ':', but it's probably OK if we only care about '"' at the very
beginning of a path. And it's consistent with many other parts of git
which allow optional quoting.
Thoughts?
[1/2]: alternates: accept double-quoted paths
[2/2]: tmp-objdir: quote paths we add to alternates
Documentation/git.txt | 6 ++++++
sha1_file.c | 47 +++++++++++++++++++++++++++++++++++-----------
t/t5547-push-quarantine.sh | 19 +++++++++++++++++++
t/t5615-alternate-env.sh | 18 ++++++++++++++++++
tmp-objdir.c | 18 +++++++++++++++++-
5 files changed, 96 insertions(+), 12 deletions(-)
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] compat: add qsort_s()
From: René Scharfe @ 2016-12-12 19:51 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Git List, Johannes Schindelin
In-Reply-To: <xmqqy3zz8kmq.fsf@gitster.mtv.corp.google.com>
Am 01.12.2016 um 21:22 schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Eh, wait. BSD and Microsoft have paramters reordered in the
>> callback comparison function. I suspect that would not fly very
>> well.
>
> Hmm. We could do it like this, which may not be too bad.
It's kinda cool to have a bespoke compatibility layer for major
platforms, but the more I think about it the less I can answer why we
would want that. Safety, reliability and performance can't be good
reasons -- if our fallback function lacks in these regards then we have
to improve it in any case.
Text size could be a valid reason, but the full function only adds a bit
more than 2KB to the unstripped git binary.
The flip side is we'd build an ifdef maze that's harder to read and a
lot more difficult to test.
What do we get in return for that additional complexity?
> #if APPLE_QSORT_R
> struct apple_qsort_adapter {
> int (*user_cmp)(const void *, const void *, void *);
> void *user_ctx;
> }
>
> static int apple_qsort_adapter_cmp(void *ctx, const void *a, const void *b)
> {
> struct apple_qsort_adapter *wrapper_ctx = ctx;
> return wrapper_ctx->user_cmp(a, b, wrapper_ctx->user_ctx);
> }
> #endif
>
> int git_qsort_s(void *b, size_t n, size_t s,
> int (*cmp)(const void *, const void *, void *), void *ctx)
> {
> if (!n)
> return 0;
> if (!b || !cmp)
> return -1;
> #if GNU_QSORT_R
> qsort_r(b, n, s, cmp, ctx);
> #elif APPLE_QSORT_R
> {
> struct appple_qsort_adapter a = { cmp, ctx };
> qsort_r(b, n, s, &a, appple_qsort_adapter_cmp);
> }
> #endif
Nit: The fallback for non-GNU, non-Apple systems is missing here, but
the idea is illustrated clearly enough.
> return 0;
> }
>
^ permalink raw reply
* [PATCH 1/2] alternates: accept double-quoted paths
From: Jeff King @ 2016-12-12 19:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161212194929.bdcihf7orjabzb2h@sigill.intra.peff.net>
We read lists of alternates from objects/info/alternates
files (delimited by newline), as well as from the
GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable
(delimited by colon or semi-colon, depending on the
platform).
There's no mechanism for quoting the delimiters, so it's
impossible to specify an alternate path that contains a
colon in the environment, or one that contains a newline in
a file. We've lived with that restriction for ages because
both alternates and filenames with colons are relatively
rare, and it's only a problem when the two meet. But since
722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03), which builds on the
alternates system, every push causes the receiver to set
GIT_ALTERNATE_OBJECT_DIRECTORIES internally.
It would be convenient to have some way to quote the
delimiter so that we can represent arbitrary paths.
The simplest thing would be an escape character before a
quoted delimiter (e.g., "\:" as a literal colon). But that
creates a backwards compatibility problem: any path which
uses that escape character is now broken, and we've just
shifted the problem. We could choose an unlikely escape
character (e.g., something from the non-printable ASCII
range), but that's awkward to use.
Instead, let's treat names as unquoted unless they begin
with a double-quote, in which case they are interpreted via
our usual C-stylke quoting rules. This also breaks
backwards-compatibility, but in a smaller way: it only
matters if your file has a double-quote as the very _first_
character in the path (whereas an escape character is a
problem anywhere in the path). It's also consistent with
many other parts of git, which accept either a bare pathname
or a double-quoted one, and the sender can choose to quote
or not as required.
Signed-off-by: Jeff King <peff@peff.net>
---
This also lets you specify paths that start with "#", or ones that
contain newlines in an alternates file, though I doubt anybody really
cares much in practice. I didn't even bother to add them to the test
suite, mostly because of the compatibility hassle (though I guess we
could just mark them with !MINGW and be OK).
Documentation/git.txt | 6 ++++++
sha1_file.c | 47 ++++++++++++++++++++++++++++++++++++-----------
t/t5615-alternate-env.sh | 18 ++++++++++++++++++
3 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index af191c51b..98033302b 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -871,6 +871,12 @@ Git so take care if using a foreign front-end.
specifies a ":" separated (on Windows ";" separated) list
of Git object directories which can be used to search for Git
objects. New objects will not be written to these directories.
++
+ Entries that begin with `"` (double-quote) will be interpreted
+ as C-style quoted paths, removing leading and trailing
+ double-quotes and respecting backslash escapes. E.g., the value
+ `"path-with-\"-and-:-in-it":vanilla-path` has two paths:
+ `path-with-"-and-:-in-it` and `vanilla-path`.
`GIT_DIR`::
If the `GIT_DIR` environment variable is set then it
diff --git a/sha1_file.c b/sha1_file.c
index 9c86d1924..117307185 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -26,6 +26,7 @@
#include "mru.h"
#include "list.h"
#include "mergesort.h"
+#include "quote.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -329,13 +330,40 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
return 0;
}
+static const char *parse_alt_odb_entry(const char *string,
+ int sep,
+ struct strbuf *out)
+{
+ const char *end;
+
+ strbuf_reset(out);
+
+ if (*string == '#') {
+ /* comment; consume up to next separator */
+ end = strchrnul(string, sep);
+ } else if (*string == '"' && !unquote_c_style(out, string, &end)) {
+ /*
+ * quoted path; unquote_c_style has copied the
+ * data for us and set "end". Broken quoting (e.g.,
+ * an entry that doesn't end with a quote) falls
+ * back to the unquoted case below.
+ */
+ } else {
+ /* normal, unquoted path */
+ end = strchrnul(string, sep);
+ strbuf_add(out, string, end - string);
+ }
+
+ if (*end)
+ end++;
+ return end;
+}
+
static void link_alt_odb_entries(const char *alt, int len, int sep,
const char *relative_base, int depth)
{
- struct string_list entries = STRING_LIST_INIT_NODUP;
- char *alt_copy;
- int i;
struct strbuf objdirbuf = STRBUF_INIT;
+ struct strbuf entry = STRBUF_INIT;
if (depth > 5) {
error("%s: ignoring alternate object stores, nesting too deep.",
@@ -348,16 +376,13 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
die("unable to normalize object directory: %s",
objdirbuf.buf);
- alt_copy = xmemdupz(alt, len);
- string_list_split_in_place(&entries, alt_copy, sep, -1);
- for (i = 0; i < entries.nr; i++) {
- const char *entry = entries.items[i].string;
- if (entry[0] == '\0' || entry[0] == '#')
+ while (*alt) {
+ alt = parse_alt_odb_entry(alt, sep, &entry);
+ if (!entry.len)
continue;
- link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
+ link_alt_odb_entry(entry.buf, relative_base, depth, objdirbuf.buf);
}
- string_list_clear(&entries, 0);
- free(alt_copy);
+ strbuf_release(&entry);
strbuf_release(&objdirbuf);
}
diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh
index eec4137ca..69fd8f891 100755
--- a/t/t5615-alternate-env.sh
+++ b/t/t5615-alternate-env.sh
@@ -68,4 +68,22 @@ test_expect_success 'access alternate via relative path (subdir)' '
EOF
'
+# set variables outside test to avoid quote insanity; the \057 is '/',
+# which doesn't need quoting, but just confirms that de-quoting
+# is working.
+quoted='"one.git\057objects"'
+unquoted='two.git/objects'
+test_expect_success 'mix of quoted and unquoted alternates' '
+ check_obj "$quoted:$unquoted" <<-EOF
+ $one blob
+ $two blob
+'
+
+test_expect_success 'broken quoting falls back to interpreting raw' '
+ mv one.git \"one.git &&
+ check_obj \"one.git/objects <<-EOF
+ $one blob
+ EOF
+'
+
test_done
--
2.11.0.203.g6657065
^ permalink raw reply related
* [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Jeff King @ 2016-12-12 19:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161212194929.bdcihf7orjabzb2h@sigill.intra.peff.net>
Commit 722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03) regressed pushes to
repositories with colon (or semi-colon in Windows in them)
because it adds the repository's main object directory to
GIT_ALTERNATE_OBJECT_DIRECTORIES. The receiver interprets
the colon as a delimiter, not as part of the path, and
index-pack is unable to find objects which it needs to
resolve deltas.
The previous commit introduced a quoting mechanism for the
alternates list; let's use it here to cover this case. We'll
avoid quoting when we can, though. This alternate setup is
also used when calling hooks, so it's possible that the user
may call older git implementations which don't understand
the quoting mechanism. By quoting only when necessary, this
setup will continue to work unless the user _also_ has a
repository whose path contains the delimiter.
Signed-off-by: Jeff King <peff@peff.net>
---
Johannes, please let me know if I am wrong about skipping the test on
!MINGW. The appropriate check there would be ";" anyway, but I am not
sure _that_ is allowed in paths, either.
t/t5547-push-quarantine.sh | 19 +++++++++++++++++++
tmp-objdir.c | 18 +++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 1e5d32d06..6275ec807 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -33,4 +33,23 @@ test_expect_success 'rejected objects are removed' '
test_cmp expect actual
'
+# MINGW does not allow colons in pathnames in the first place
+test_expect_success !MINGW 'push to repo path with colon' '
+ # The interesting failure case here is when the
+ # receiving end cannot access its original object directory,
+ # so make it likely for us to generate a delta by having
+ # a non-trivial file with multiple versions.
+
+ test-genrandom foo 4096 >file.bin &&
+ git add file.bin &&
+ git commit -m bin &&
+ git clone --bare . xxx:yyy.git &&
+
+ echo change >>file.bin &&
+ git commit -am change &&
+ # Note that we have to use the full path here, or it gets confused
+ # with the ssh host:path syntax.
+ git push "$PWD/xxx:yyy.git" HEAD
+'
+
test_done
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 64435f23a..b2d9280f1 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -5,6 +5,7 @@
#include "string-list.h"
#include "strbuf.h"
#include "argv-array.h"
+#include "quote.h"
struct tmp_objdir {
struct strbuf path;
@@ -79,12 +80,27 @@ static void remove_tmp_objdir_on_signal(int signo)
*/
static void env_append(struct argv_array *env, const char *key, const char *val)
{
- const char *old = getenv(key);
+ struct strbuf quoted = STRBUF_INIT;
+ const char *old;
+ /*
+ * Avoid quoting if it's not necessary, for maximum compatibility
+ * with older parsers which don't understand the quoting.
+ */
+ if (*val == '"' || strchr(val, PATH_SEP)) {
+ strbuf_addch("ed, '"');
+ quote_c_style(val, "ed, NULL, 1);
+ strbuf_addch("ed, '"');
+ val = quoted.buf;
+ }
+
+ old = getenv(key);
if (!old)
argv_array_pushf(env, "%s=%s", key, val);
else
argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, val);
+
+ strbuf_release("ed);
}
static void env_replace(struct argv_array *env, const char *key, const char *val)
--
2.11.0.203.g6657065
^ permalink raw reply related
* Re: [PATCH 1/3] compat: add qsort_s()
From: Jeff King @ 2016-12-12 19:57 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git List, Johannes Schindelin
In-Reply-To: <b8aa28b1-e645-4cea-cc91-96f62fee6118@web.de>
On Mon, Dec 12, 2016 at 08:51:14PM +0100, René Scharfe wrote:
> It's kinda cool to have a bespoke compatibility layer for major platforms,
> but the more I think about it the less I can answer why we would want that.
> Safety, reliability and performance can't be good reasons -- if our fallback
> function lacks in these regards then we have to improve it in any case.
There may be cases that we don't want to support because of portability
issues. E.g., if your libc has an assembly-optimized qsort() we wouldn't
want to replicate that.
I dunno. I am not that opposed to just saying "forget libc qsort, we
always use our internal one which is consistent, performant, and safe".
But when I suggested something similar for our regex library, I seem to
recall there were complaints.
-Peff
^ permalink raw reply
* [BUG] Git-GUI destroys the author name
From: bitjo @ 2016-12-12 20:05 UTC (permalink / raw)
To: git
Since Git version 2.11, git gui destroys the author name when 'git
commit amend' is executed and the author name contains UTF-8-specific
characters.
This is a known issue from 'Git for Windows' and now a
platform-independent bug.
Please refer
- https://github.com/git-for-windows/git/issues/761
- https://github.com/git-for-windows/git/issues/761#issuecomment-220792200
- https://github.com/git-for-windows/git/issues/761#issuecomment-266519859
From my point of view, this is a severe error since the repostory is
changed incorrectly. Users of git gui will usually not be able to
correct this error with git commands.
^ permalink raw reply
* Re: git add -p with new file
From: Stephan Beyer @ 2016-12-12 20:24 UTC (permalink / raw)
To: Ariel, Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <alpine.DEB.2.11.1612091322290.13185@cherryberry.dsgml.com>
Hi Ariel,
On 12/09/2016 07:26 PM, Ariel wrote:
> On Wed, 7 Dec 2016, Duy Nguyen wrote:
>> We could improve it a bit, suggesting the user to do git add -N. But
>> is there a point of using -p on a new file?
>
> I got into the habit of always using -p as a way of checking my diffs
> before committing,
Not commenting on the main issue here, but if you want to check your new
files with a "git add -p"-like tool, you can do
git add <untracked files>
git reset -p
(this is unstaging, but it's still useful)
Best
Stephan
^ permalink raw reply
* Re: git add -p with new file
From: Stephan Beyer @ 2016-12-12 20:31 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: Ariel, git
In-Reply-To: <20161211130034.ygj5l2gbx33uknlk@sigill.intra.peff.net>
Hi,
On 12/11/2016 02:00 PM, Jeff King wrote:
> On Sat, Dec 10, 2016 at 02:04:33PM -0800, Junio C Hamano wrote:
>> Jeff King <peff@peff.net> writes:
>>> On Fri, Dec 09, 2016 at 01:43:24PM -0500, Ariel wrote:
>>> ...
>>>> But it doesn't have to be that way. You could make add -p identical to add
>>>> without options, except the -p prompts to review diffs first.
>>>
>>> The question is whether you would annoy people using "-p" if you started
>>> including untracked files by default. I agree because it's inherently an
>>> interactive process that we can be looser with backwards compatibility.
>>
>> It might be interactive, but it will be irritating that we suddenly
>> have to see hundreds of lines in an untracked file before we are
>> asked to say "no I do not want to add this file" and have to do so
>> for all the untracked files that happen to match the given pathspec.
>>
>> It might make it less irritating if one of the interactive choices
>> offered in the first prompt were N that tells the command: "No,
>> ignore all the untracked paths", though. I dunno.
>
> Yeah, I agree dumping the contents automatically is annoying. Ariel
> suggested asking twice about each path, which sounds clunky to me. I'd
> probably give a simple question, with an option to dump the contents.
> Like:
>
> $ echo foo >untracked
> $ git add -p
> New file: untracked
> Stage this file [y,n,v,q,a,d,/,e,?]? v <-- user types 'v' for "view"
I am also a "git add -p"-only user (except for new files and merges).
However, I usually keep a lot of untracked files in my repositories.
Files that I do not (git)ignore because I want to see them when I type
"git status".
Hence, the imagination only that "git add -p" starts to ask me for each
untracked file feels like a lot of annoying "n" presses. I could imagine
that it is okay-ish when it asks about the untracked files *after* all
tracked paths have been processed (I guess this has been proposed
before), so that I can safely quit.
I am also not really sure what problem this feature is trying to solve.
If the "problem"(?) is that it should act more like "git add" instead of
"git add -u", for whatever reason, this may be fine (but the
configuration option is a must-have then).
For me, I often had the problem that I simply forgot to add new
files...¹ Still I doubt that one would benefit from such a feature
because either:
- you do not have many untracked files (unlike me). You will see the
untracked file (that should be tracked) in the "git status" output when
you edit the commit message², then you abort-add-commit or
commit-add-amend and everything is fine.
Or:
- you have a lot of untracked files (like me). You won't see the
untracked file (that should be tracked) in the "git status" output (you
ignore it because the list is so long) and you won't see it in the "git
add -p" run because you quit before seeing that file.
So I have mixed feelings...
> I'd also probably add interactive.showUntracked to make the whole thing
> optional (but I think it would be OK to default it to on).
Hm, "interactive.showUntracked" is a confusing name because "git add -i"
(interactive) already handles untracked files.
Best
Stephan
¹ I do not have that problem any more because I got used to add new
files right after saving, in a very early state, and then I simply use
"git add -p"...
² Unless you use git commit -m ... But using "git commit -m" without
"git status" before is user-issue anyway.
^ permalink raw reply
* Re: [PATCHv7 0/6] submodule absorbgitdirs
From: Brandon Williams @ 2016-12-12 20:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, pclouds
In-Reply-To: <20161212190435.10358-1-sbeller@google.com>
On 12/12, Stefan Beller wrote:
> v8:
> * Change the worktree implementation: do not have an internal
> submodule_get_worktrees, and count the number of worktrees, instead
> directly look if there is a worktrees dir and check if there are any files
> in there.
> * reworded one test title slightly.
> * interdiff to v7 (that is queued as origin/sb/submodule-embed-gitdir) below.
>
not important but your cover letter's subject is v7 instead of v8 :)
--
Brandon Williams
^ permalink raw reply
* Re: [PATCHv7 0/6] submodule absorbgitdirs
From: Stefan Beller @ 2016-12-12 20:37 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Duy Nguyen
In-Reply-To: <20161212203528.GA193413@google.com>
On Mon, Dec 12, 2016 at 12:35 PM, Brandon Williams <bmwill@google.com> wrote:
> On 12/12, Stefan Beller wrote:
>> v8:
>> * Change the worktree implementation: do not have an internal
>> submodule_get_worktrees, and count the number of worktrees, instead
>> directly look if there is a worktrees dir and check if there are any files
>> in there.
>> * reworded one test title slightly.
>> * interdiff to v7 (that is queued as origin/sb/submodule-embed-gitdir) below.
>>
>
> not important but your cover letter's subject is v7 instead of v8 :)
>
I realized that, after sending out.
When working on just one series on the mailing list,
I tend to reuse the coverletter and just adapt it, so I
would presume the diff stat is also broken.
^ permalink raw reply
* Re: [PATCH 2/2] tmp-objdir: quote paths we add to alternates
From: Johannes Sixt @ 2016-12-12 20:53 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Klaus Ethgen, git
In-Reply-To: <20161212195355.znqlu44lgnke3ltc@sigill.intra.peff.net>
Am 12.12.2016 um 20:53 schrieb Jeff King:
> Johannes, please let me know if I am wrong about skipping the test on
> !MINGW.
Thanks for being considerate. The !MINGW prerequisite is required for
the test as written.
> The appropriate check there would be ";" anyway, but I am not
> sure _that_ is allowed in paths, either.
That was also my line of thought. I tested earlier today whether a file
name can have ";", and the OS did not reject it bluntly. Let me see
whether I can adjust the test case for Windows.
-- Hannes
^ permalink raw reply
* Re: [BUG] Git-GUI destroys the author name
From: bitjo @ 2016-12-12 20:59 UTC (permalink / raw)
To: git
In-Reply-To: <588ac268-0a1b-6787-e6a1-4164e7e9a784@b-i-t.de>
Addition to the bug
Proposal
https://github.com/git-for-windows/git/commit/30395c645adf21828bbccbdcd3c5c36c39f07050
should be considered and possibly implemented.
But as described in
https://github.com/git-for-windows/git/issues/761#issuecomment-221057828.
And then an implementation without regression is necessary.
^ permalink raw reply
* Re: [PATCHv2 5/7] versioncmp: cope with common part overlapping with prerelease suffix
From: Junio C Hamano @ 2016-12-12 21:27 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git
In-Reply-To: <20161208142401.1329-6-szeder.dev@gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> diff --git a/versioncmp.c b/versioncmp.c
> index a55c23ad5..f86ac562e 100644
> --- a/versioncmp.c
> +++ b/versioncmp.c
> @@ -26,12 +26,15 @@ static int initialized;
>
> /*
> * off is the offset of the first different character in the two strings
> - * s1 and s2. If either s1 or s2 contains a prerelease suffix starting
> - * at that offset, it will be forced to be on top.
> + * s1 and s2. If either s1 or s2 contains a prerelease suffix containing
> + * that offset, then that string will be forced to be on top.
> *
> - * If both s1 and s2 contain a (different) suffix at that position,
> + * If both s1 and s2 contain a (different) suffix around that position,
> * their order is determined by the order of those two suffixes in the
> * configuration.
> + * If any of the strings contains more than one different suffixes around
> + * that position, then that string is sorted according to the contained
> + * suffix which comes first in the configuration.
> *
> * Return non-zero if *diff contains the return value for versioncmp()
> */
> @@ -44,10 +47,21 @@ static int swap_prereleases(const char *s1,
>
> for (i = 0; i < prereleases->nr; i++) {
> const char *suffix = prereleases->items[i].string;
> - if (i1 == -1 && starts_with(s1 + off, suffix))
> - i1 = i;
> - if (i2 == -1 && starts_with(s2 + off, suffix))
> - i2 = i;
> + int j, start, suffix_len = strlen(suffix);
> + if (suffix_len < off)
> + start = off - suffix_len + 1;
> + else
> + start = 0;
Now that this function has to rewind the beginning of the comparison
earlier than the given 'off', it makes me wonder if it still makes
sense for the caller to compute it in the first place.
> + for (j = start; j <= off; j++)
> + if (i1 == -1 && starts_with(s1 + j, suffix)) {
> + i1 = i;
> + break;
> + }
> + for (j = start; j <= off; j++)
> + if (i2 == -1 && starts_with(s2 + j, suffix)) {
> + i2 = i;
> + break;
> + }
> }
> if (i1 == -1 && i2 == -1)
> return 0;
^ permalink raw reply
* What's cooking in git.git (Dec 2016, #02; Mon, 12)
From: Junio C Hamano @ 2016-12-12 21:30 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
Quite a many topics that have been waiting before the most recent
release have now been merged to 'next'; hopefully they will be ready
for 'master' in a week or two.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* jc/lock-report-on-error (2016-12-07) 3 commits
- lockfile: LOCK_REPORT_ON_ERROR
- hold_locked_index(): align error handling with hold_lockfile_for_update()
- wt-status: implement opportunisitc index update correctly
Git 2.11 had a minor regression in "merge --ff-only" that competed
with another process that simultanously attempted to update the
index. We used to explain what went wrong with an error message,
but the new code silently failed. This resurrects the error
message.
Will merge to 'next'.
* nd/shallow-fixup (2016-12-07) 6 commits
- shallow.c: remove useless code
- shallow.c: bit manipulation tweaks
- shallow.c: avoid theoretical pointer wrap-around
- shallow.c: make paint_alloc slightly more robust
- shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory pools
- shallow.c: rename fields in paint_info to better express their purposes
Code cleanup in shallow boundary computation.
Will merge to 'next'.
* sb/sequencer-abort-safety (2016-12-09) 5 commits
- sequencer: remove useless get_dir() function
- sequencer: make sequencer abort safer
- t3510: test that cherry-pick --abort does not unsafely change HEAD
- am: change safe_to_abort()'s not rewinding error into a warning
- am: fix filename in safe_to_abort() error message
Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back
to where cherry-pick started while picking multiple changes, when
the cherry-pick stopped to ask for help from the user, and the user
did "git reset --hard" to a different commit in order to re-attempt
the operation.
Will merge to 'next'.
* kh/tutorial-grammofix (2016-12-09) 4 commits
- doc: omit needless "for"
- doc: make the intent of sentence clearer
- doc: add verb in front of command to run
- doc: add articles (grammar)
Will merge to 'next'.
* da/mergetool-xxdiff-hotkey (2016-12-11) 1 commit
- mergetools: fix xxdiff hotkeys
The way to specify hotkeys to "xxdiff" that is used by "git
mergetool" has been modernized to match recent versions of xxdiff.
Will merge to 'next'.
* jk/difftool-in-subdir (2016-12-11) 4 commits
- difftool: rename variables for consistency
- difftool: chdir as early as possible
- difftool: sanitize $workdir as early as possible
- difftool: fix dir-diff index creation when in a subdirectory
Even though an fix was attempted in Git 2.9.3 days, but running
"git difftool --dir-diff" from a subdirectory never worked. This
has been fixed.
Will merge to 'next'.
* js/mingw-isatty (2016-12-11) 1 commit
(merged to 'next' on 2016-12-12 at 60c1da6676)
+ mingw: intercept isatty() to handle /dev/null as Git expects it
We often decide if a session is interactive by checking if the
standard I/O streams are connected to a TTY, but isatty() emulation
on Windows incorrectly returned true if it is used on NUL (i.e. an
equivalent to /dev/null). This has been fixed.
Will merge to 'master'.
* ew/svn-fixes (2016-12-12) 2 commits
(merged to 'next' on 2016-12-12 at 91ed5b3cf3)
+ git-svn: document useLogAuthor and addAuthorFrom config keys
+ git-svn: allow "0" in SVN path components
Will merge to 'master'.
* lr/doc-fix-cet (2016-12-12) 1 commit
- date-formats.txt: Typo fix
Will merge to 'next'.
* vs/submodule-clone-nested-submodules-alternates (2016-12-12) 1 commit
- submodule--helper: set alternateLocation for cloned submodules
"git clone --reference $there --recurse-submodules $super" has been
taught to guess repositories usable as references for submodules of
$super that are embedded in $there while making a clone of the
superproject borrow objects from $there; extend the mechanism to
also allow submodules of these submodules to borrow repositories
embedded in these clones of the submodules embedded in the clone of
the superproject.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* jc/retire-compaction-heuristics (2016-11-02) 3 commits
- SQUASH???
- SQUASH???
- diff: retire the original experimental "compaction" heuristics
Waiting for a reroll.
* jc/abbrev-autoscale-config (2016-11-01) 1 commit
- config.abbrev: document the new default that auto-scales
Waiting for a reroll.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
- exclude: do not respect symlinks for in-tree .gitignore
- attr: do not respect symlinks for in-tree .gitattributes
- exclude: convert "check_index" into a flags field
- attr: convert "macro_ok" into a flags field
- add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* nd/worktree-move (2016-11-28) 11 commits
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
. copy.c: convert copy_file() to copy_dir_recursively()
. copy.c: style fix
. copy.c: convert bb_(p)error_msg to error(_errno)
. copy.c: delete unused code in copy_file()
. copy.c: import copy_file() from busybox
(this branch uses nd/worktree-list-fixup; is tangled with sb/submodule-embed-gitdir.)
"git worktree" learned move and remove subcommands.
Reported to break builds on Windows.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* ak/lazy-prereq-mktemp (2016-11-29) 1 commit
(merged to 'next' on 2016-12-12 at f346d1f053)
+ t7610: clean up foo.XXXXXX tmpdir
Test code clean-up.
Will merge to 'master'.
* da/mergetool-trust-exit-code (2016-11-29) 2 commits
(merged to 'next' on 2016-12-12 at 28ae202868)
+ mergetools/vimdiff: trust Vim's exit code
+ mergetool: honor mergetool.$tool.trustExitCode for built-in tools
mergetool.<tool>.trustExitCode configuration variable did not apply
to built-in tools, but now it does.
Will merge to 'master'.
* jb/diff-no-index-no-abbrev (2016-12-08) 1 commit
(merged to 'next' on 2016-12-12 at 959981ef50)
+ diff: handle --no-abbrev in no-index case
"git diff --no-index" did not take "--no-abbrev" option.
Will merge to 'master'.
* vk/p4-submit-shelve (2016-11-29) 1 commit
(merged to 'next' on 2016-12-12 at 3fce6df117)
+ git-p4: allow submit to create shelved changelists.
(this branch is used by ld/p4-update-shelve.)
Will merge to 'master'.
* jk/http-walker-limit-redirect-2.9 (2016-12-06) 5 commits
(merged to 'next' on 2016-12-12 at 3e4bcd7bca)
+ http: treat http-alternates like redirects
+ http: make redirects more obvious
+ remote-curl: rename shadowed options variable
+ http: always update the base URL for redirects
+ http: simplify update_url_from_redirect
(this branch is used by jk/http-walker-limit-redirect.)
Transport with dumb http can be fooled into following foreign URLs
that the end user does not intend to, especially with the server
side redirects and http-alternates mechanism, which can lead to
security issues. Tighten the redirection and make it more obvious
to the end user when it happens.
Will merge to 'master'.
* jk/http-walker-limit-redirect (2016-12-06) 2 commits
(merged to 'next' on 2016-12-12 at 8b58025e3a)
+ http-walker: complain about non-404 loose object errors
+ Merge branch 'ew/http-walker' into jk/http-walker-limit-redirect
(this branch uses jk/http-walker-limit-redirect-2.9.)
Update the error messages from the dumb-http client when it fails
to obtain loose objects; we used to give sensible error message
only upon 404 but we now forbid unexpected redirects that needs to
be reported with something sensible.
Will merge to 'master'.
* ah/grammos (2016-12-05) 3 commits
(merged to 'next' on 2016-12-12 at 13ad487b28)
+ clone,fetch: explain the shallow-clone option a little more clearly
+ receive-pack: improve English grammar of denyCurrentBranch message
+ bisect: improve English grammar of not-ancestors message
A few messages have been fixed for their grammatical errors.
Will merge to 'master'.
* ak/commit-only-allow-empty (2016-12-09) 2 commits
(merged to 'next' on 2016-12-12 at 54188ab23c)
+ commit: remove 'Clever' message for --only --amend
+ commit: make --only --allow-empty work without paths
"git commit --allow-empty --only" (no pathspec) with dirty index
ought to be an acceptable way to create a new commit that does not
change any paths, but it was forbidden (perhaps because nobody
needed it).
Will merge to 'master'.
* bb/unicode-9.0 (2016-12-11) 6 commits
- update_unicode.sh: restore hexadecimal output
- update_unicode.sh: remove the plane filters
- update_unicode.sh: update the uniset repo if it exists
- unicode_width.h: update the tables to Unicode 9.0
- update_unicode.sh: strip the plane offsets from the double_width[] table
- update_unicode.sh: automatically download newer definition files
The character width table has been updated to match Unicode 9.0
Will merge to 'next'.
* ld/p4-update-shelve (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 22f6bec94c)
+ git-p4: support updating an existing shelved changelist
(this branch uses vk/p4-submit-shelve.)
Will merge to 'master'.
* ld/p4-worktree (2016-12-05) 1 commit
- git-p4: support secondary working trees managed by "git worktree"
Iffy.
cf. <20161202224319.5385-2-luke@diamand.org>
* ls/p4-empty-file-on-lfs (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 1fce8e037a)
+ git-p4: fix empty file processing for large file system backend GitLFS
"git p4" LFS support was broken when LFS stores an empty blob.
Will merge to 'master'.
* ls/p4-retry-thrice (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 9462e660a8)
+ git-p4: add config to retry p4 commands; retry 3 times by default
Will merge to 'master'.
* ls/t0021-fixup (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at db652e691a)
+ t0021: minor filter process test cleanup
Will merge to 'master'.
* ls/travis-update-p4-and-lfs (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 5496caa048)
+ travis-ci: update P4 to 16.2 and GitLFS to 1.5.2 in Linux build
The default Travis-CI configuration specifies newer P4 and GitLFS.
Will merge to 'master'.
* sb/t3600-cleanup (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at d9996af5e8)
+ t3600: remove useless redirect
Code cleanup.
Will merge to 'master'.
* sb/unpack-trees-grammofix (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 29e536f590)
+ unpack-trees: fix grammar for untracked files in directories
Will merge to 'master'.
* da/difftool-dir-diff-fix (2016-12-08) 1 commit
(merged to 'next' on 2016-12-12 at fd31a92ad6)
+ difftool: fix dir-diff index creation when in a subdirectory
"git difftool --dir-diff" had a minor regression when started from
a subdirectory, which has been fixed.
Will merge to 'master'.
* jk/stash-disable-renames-internally (2016-12-06) 1 commit
(merged to 'next' on 2016-12-12 at e2b97aae68)
+ stash: prefer plumbing over git-diff
When diff.renames configuration is on (and with Git 2.9 and later,
it is enabled by default, which made it worse), "git stash"
misbehaved if a file is removed and another file with a very
similar content is added.
Will merge to 'master'.
* jk/xdiff-drop-xdl-fast-hash (2016-12-06) 1 commit
- xdiff: drop XDL_FAST_HASH
Retire the "fast hash" that had disastrous performance issues in
some corner cases.
Will merge to 'next'.
* ls/filter-process (2016-12-06) 1 commit
(merged to 'next' on 2016-12-12 at 8ed1f9eb02)
+ docs: warn about possible '=' in clean/smudge filter process values
Doc update.
Will merge to 'master'.
* nd/for-each-ref-ignore-case (2016-12-05) 1 commit
(merged to 'next' on 2016-12-12 at 527cc4f275)
+ tag, branch, for-each-ref: add --ignore-case for sorting and filtering
"git branch --list" and friends learned "--ignore-case" option to
optionally sort branches and tags case insensitively.
Will merge to 'master'.
* rj/git-version-gen-do-not-force-abbrev (2016-12-06) 1 commit
(merged to 'next' on 2016-12-12 at e37970c3f5)
+ GIT-VERSION-GEN: do not force abbreviation length used by 'describe'
A minor build update.
Will merge to 'master'.
* jc/renormalize-merge-kill-safer-crlf (2016-12-01) 4 commits
(merged to 'next' on 2016-12-12 at 041b834f81)
+ convert: git cherry-pick -Xrenormalize did not work
+ Merge branch 'tb/t0027-raciness-fix' into jc/renormalize-merge-kill-safer-crlf
+ merge-recursive: handle NULL in add_cacheinfo() correctly
+ cherry-pick: demonstrate a segmentation fault
Fix a corner case in merge-recursive regression that crept in
during 2.10 development cycle.
Will merge to 'master'.
* js/difftool-builtin (2016-11-28) 2 commits
- difftool: implement the functionality in the builtin
- difftool: add a skeleton for the upcoming builtin
Rewrite a scripted porcelain "git difftool" in C.
Under discussion.
* nd/qsort-in-merge-recursive (2016-11-28) 1 commit
(merged to 'next' on 2016-12-12 at e9700f5b93)
+ merge-recursive.c: use string_list_sort instead of qsort
Code simplification.
Will merge to 'master'.
* bw/push-dry-run (2016-11-23) 2 commits
(merged to 'next' on 2016-12-12 at bde7a0f9ae)
+ push: fix --dry-run to not push submodules
+ push: --dry-run updates submodules when --recurse-submodules=on-demand
(this branch uses hv/submodule-not-yet-pushed-fix; is tangled with sb/push-make-submodule-check-the-default.)
"git push --dry-run --recurse-submodule=on-demand" wasn't
"--dry-run" in the submodules.
Will merge to 'master'.
* sb/push-make-submodule-check-the-default (2016-11-29) 2 commits
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
+ submodule add: extend force flag to add existing repos
(this branch uses hv/submodule-not-yet-pushed-fix; is tangled with bw/push-dry-run.)
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will merge to 'master'.
* jk/rev-parse-symbolic-parents-fix (2016-11-16) 1 commit
(merged to 'next' on 2016-12-12 at 6839c1ea28)
+ rev-parse: fix parent shorthands with --symbolic
"git rev-parse --symbolic" failed with a more recent notation like
"HEAD^-1" and "HEAD^!".
Will merge to 'master'.
* nd/worktree-list-fixup (2016-11-28) 5 commits
(merged to 'next' on 2016-12-12 at 1f46421a59)
+ worktree list: keep the list sorted
+ worktree.c: get_worktrees() takes a new flag argument
+ get_worktrees() must return main worktree as first item even on error
+ worktree: reorder an if statement
+ worktree.c: zero new 'struct worktree' on allocation
(this branch is used by nd/worktree-move and sb/submodule-embed-gitdir.)
The output from "git worktree list" was made in readdir() order,
and was unstable.
Will merge to 'master'.
* jk/trailers-placeholder-in-pretty (2016-12-11) 2 commits
(merged to 'next' on 2016-12-12 at 57de4e699a)
+ ref-filter: add support to display trailers as part of contents
+ pretty: add %(trailers) format for displaying trailers of a commit message
(this branch uses jt/use-trailer-api-in-commands.)
In addition to %(subject), %(body), "log --pretty=format:..."
learned a new placeholder %(trailers).
Will merge to 'master'.
* sb/submodule-embed-gitdir (2016-12-09) 6 commits
- submodule: add absorb-git-dir function
- move connect_work_tree_and_git_dir to dir.h
- worktree: have a function to check if worktrees are in use
- test-lib-functions.sh: teach test_commit -C <dir>
- submodule helper: support super prefix
- submodule: use absolute path for computing relative path connecting
(this branch uses nd/worktree-list-fixup; is tangled with nd/worktree-move.)
A new submodule helper "git submodule embedgitdirs" to make it
easier to move embedded .git/ directory for submodules in a
superproject to .git/modules/ (and point the latter with the former
that is turned into a "gitdir:" file) has been added.
Waiting for review to conclude.
cf. <20161208014623.7588-1-sbeller@google.com>
* dt/empty-submodule-in-merge (2016-11-17) 1 commit
(merged to 'next' on 2016-12-12 at 6de2350b2b)
+ submodules: allow empty working-tree dirs in merge/cherry-pick
An empty directory in a working tree that can simply be nuked used
to interfere while merging or cherry-picking a change to create a
submodule directory there, which has been fixed..
Will merge to 'master'.
* bw/grep-recurse-submodules (2016-11-22) 6 commits
- grep: search history of moved submodules
- grep: enable recurse-submodules to work on <tree> objects
- grep: optionally recurse into submodules
- grep: add submodules as a grep source type
- submodules: load gitmodules file from commit sha1
- submodules: add helper functions to determine presence of submodules
"git grep" learns to optionally recurse into submodules
Has anybody else seen t7814 being flakey with this series?
* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
(merged to 'next' on 2016-12-05 at 3ea70d01af)
+ upload-pack: optionally allow fetching any sha1
+ remote-curl: don't hang when a server dies before any output
Originally merged to 'next' on 2016-11-21
When the http server gives an incomplete response to a smart-http
rpc call, it could lead to client waiting for a full response that
will never come. Teach the client side to notice this condition
and abort the transfer.
An improvement counterproposal has failed.
cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
Will cook in 'next'.
* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
(merged to 'next' on 2016-12-05 at 9a2b5bd1a9)
+ doc: mention transfer data leaks in more places
Originally merged to 'next' on 2016-11-16
Doc update on fetching and pushing.
Will cook in 'next'.
* jc/compression-config (2016-11-15) 1 commit
(merged to 'next' on 2016-12-05 at 323769ca07)
+ compression: unify pack.compression configuration parsing
Originally merged to 'next' on 2016-11-23
Compression setting for producing packfiles were spread across
three codepaths, one of which did not honor any configuration.
Unify these so that all of them honor core.compression and
pack.compression variables the same way.
Will cook in 'next'.
* mm/gc-safety-doc (2016-11-16) 1 commit
(merged to 'next' on 2016-12-05 at 031ecc1886)
+ git-gc.txt: expand discussion of races with other processes
Originally merged to 'next' on 2016-11-17
Doc update.
Will cook in 'next'.
* hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
(merged to 'next' on 2016-12-05 at c9d729fca2)
+ submodule_needs_pushing(): explain the behaviour when we cannot answer
+ batch check whether submodule needs pushing into one call
+ serialize collection of refs that contain submodule changes
+ serialize collection of changed submodules
(this branch is used by bw/push-dry-run and sb/push-make-submodule-check-the-default.)
Originally merged to 'next' on 2016-11-21
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
Will cook in 'next'.
* kn/ref-filter-branch-list (2016-12-08) 20 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
- ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
- ref-filter: rename the 'strip' option to 'lstrip'
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
- for-each-ref: do not segv with %(HEAD) on an unborn branch
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled, reviewed, looking good.
* bw/transport-protocol-policy (2016-12-05) 5 commits
- transport: add from_user parameter to is_transport_allowed
- http: create function to get curl allowed protocols
- http: always warn if libcurl version is too old
- transport: add protocol policy config option
- lib-proto-disable: variable name fix
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.
* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
(merged to 'next' on 2016-12-05 at 432f9469a7)
+ fetch: do not redundantly calculate tag refmap
Originally merged to 'next' on 2016-11-16
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.
Will cook in 'next'.
* sb/submodule-config-cleanup (2016-11-22) 3 commits
(merged to 'next' on 2016-12-05 at 658b8764bf)
+ submodule-config: clarify parsing of null_sha1 element
+ submodule-config: rename commit_sha1 to treeish_name
+ submodule config: inline config_from_{name, path}
Originally merged to 'next' on 2016-11-23
Minor code clean-up.
Will cook in 'next'.
* jc/push-default-explicit (2016-10-31) 2 commits
(merged to 'next' on 2016-12-05 at d63f3777af)
+ push: test pushing ambiguously named branches
+ push: do not use potentially ambiguous default refspec
Originally merged to 'next' on 2016-11-01
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.
Will cook in 'next'.
* jt/use-trailer-api-in-commands (2016-11-29) 5 commits
(merged to 'next' on 2016-12-12 at da1f140ad4)
+ sequencer: use trailer's trailer layout
+ trailer: have function to describe trailer layout
+ trailer: avoid unnecessary splitting on lines
+ commit: make ignore_non_trailer take buf/len
+ trailer: be stricter in parsing separators
(this branch is used by jk/trailers-placeholder-in-pretty.)
Commands that operate on a log message and add lines to the trailer
blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
"commit -s", have been taught to use the logic of and share the
code with "git interpret-trailer".
Will merge to 'master'.
* nd/rebase-forget (2016-12-11) 1 commit
(merged to 'next' on 2016-12-12 at 50b5d28af4)
+ rebase: add --quit to cleanup rebase, leave everything else untouched
"git rebase" learned "--forget" option, which allows a user to
remove the metadata left by an earlier "git rebase" that was
manually aborted without using "git rebase --abort".
Will merge to 'master'.
* jc/git-open-cloexec (2016-11-02) 3 commits
- sha1_file: stop opening files with O_NOATIME
- git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
- git_open(): untangle possible NOATIME and CLOEXEC interactions
The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
opens has been simplified.
We may want to drop the tip one.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
Will discard.
This may not be needed, given that update-index has a similar
option.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
Will discard.
The whole premise feels wrong.
* tb/convert-stream-check (2016-10-27) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Will discard.
Retracted.
cf. <20161102071646.GA5094@tb-raspi>
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Waiting for review.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-11-11) 35 commits
- completion: clone can initialize specific submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Waiting for review.
* va/i18n-perl-scripts (2016-11-11) 16 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark composing message for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark status words for translation
- i18n: add--interactive: remove %patch_modes entries
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark patch prompt for translation
- i18n: add--interactive: mark plural strings
- i18n: clean.c: match string with git-add--interactive.perl
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here-documents for translation
- i18n: add--interactive: mark strings for translation
- Git.pm: add subroutines for commenting lines
Porcelain scripts written in Perl are getting internationalized.
Waiting for review.
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-12-05 at fb549caa12)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Originally merged to 'next' on 2016-09-28
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will cook in 'next'.
* sg/fix-versioncmp-with-common-suffix (2016-12-08) 8 commits
- versioncmp: generalize version sort suffix reordering
- squash! versioncmp: use earliest-longest contained suffix to determine sorting order
- versioncmp: use earliest-longest contained suffix to determine sorting order
- versioncmp: cope with common part overlapping with prerelease suffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for review.
cf. <20161208142401.1329-1-szeder.dev@gmail.com>
* jc/pull-rebase-ff (2016-11-29) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
^ permalink raw reply
* Re: [PATCH v2] difftool: fix dir-diff index creation when in a subdirectory
From: Junio C Hamano @ 2016-12-12 21:35 UTC (permalink / raw)
To: David Aguilar; +Cc: Git ML, Frank Becker, Johannes Schindelin, John Keeping
In-Reply-To: <20161207101608.16058-1-davvid@gmail.com>
Thanks; queued and pushed out together with the follow-up series.
^ permalink raw reply
* Re: [PATCH v3 1/4] real_path: resolve symlinks by hand
From: Junio C Hamano @ 2016-12-12 22:19 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <1481566615-75299-2-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> +/* removes the last path component from 'path' except if 'path' is root */
> +static void strip_last_component(struct strbuf *path)
> +{
> + size_t offset = offset_1st_component(path->buf);
> + size_t len = path->len;
> +
> + /* Find start of the last component */
> + while (offset < len && !is_dir_sep(path->buf[len - 1]))
> + len--;
If somebody at a higher level in the callchain has already
normalized path, this is not a problem, but this will behave
"unexpectedly" when path ends with a dir_sep byte (or more).
E.g. for input path "foo/bar/", the above loop runs zero times and
then ...
> + /* Skip sequences of multiple path-separators */
> + while (offset < len && is_dir_sep(path->buf[len - 1]))
> + len--;
... the slash at the end is removed, leaving "foo/bar" in path.
> + strbuf_setlen(path, len);
> +}
> ...
> +/* get (and remove) the next component in 'remaining' and place it in 'next' */
> +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> +{
> + char *start = NULL;
> + char *end = NULL;
> +
> + strbuf_reset(next);
> +
> + /* look for the next component */
> + /* Skip sequences of multiple path-separators */
> + for (start = remaining->buf; is_dir_sep(*start); start++)
> + ; /* nothing */
> + /* Find end of the path component */
> + for (end = start; *end && !is_dir_sep(*end); end++)
> + ; /* nothing */
> +
> + strbuf_add(next, start, end - start);
> + /* remove the component from 'remaining' */
> + strbuf_remove(remaining, 0, end - remaining->buf);
> +}
Unlike the strip_last_component(), I think this one is more
carefully done and avoids getting fooled by //extra//slashes// at
the beginning or at the end, which does help in the correctness of
the loop we see below.
> @@ -58,74 +88,112 @@ static const char *real_path_internal(const char *path, int die_on_error)
> goto error_out;
> }
>
> + strbuf_reset(&resolved);
> +
> + if (is_absolute_path(path)) {
> + /* absolute path; start with only root as being resolved */
> + int offset = offset_1st_component(path);
> + strbuf_add(&resolved, path, offset);
> + strbuf_addstr(&remaining, path + offset);
> + } else {
> + /* relative path; can use CWD as the initial resolved path */
> + if (strbuf_getcwd(&resolved)) {
> + if (die_on_error)
> + die_errno("unable to get current working directory");
> + else
> + goto error_out;
> }
> + strbuf_addstr(&remaining, path);
> + }
>
> + /* Iterate over the remaining path components */
> + while (remaining.len > 0) {
> + get_next_component(&next, &remaining);
> +
> + if (next.len == 0) {
> + continue; /* empty component */
> + } else if (next.len == 1 && !strcmp(next.buf, ".")) {
> + continue; /* '.' component */
> + } else if (next.len == 2 && !strcmp(next.buf, "..")) {
> + /* '..' component; strip the last path component */
> + strip_last_component(&resolved);
Wouldn't this let "resolved" eventually run out of the path
components to strip for a malformed input e.g. "/a/../../b"?
> + ...
> + /*
> + * if there are still remaining components to resolve
> + * then append them to symlink
> + */
> + if (remaining.len) {
> + strbuf_addch(&symlink, '/');
This can add duplicate dir_sep if readlink(2)'ed contents of the
symbolic link already ends with a slash, but I think it (together
with the fact that the code does nothing to normalize what is read
from the symbolic link) probably does not matter, given the way how
get_next_component() is implemented.
> + strbuf_addbuf(&symlink, &remaining);
> + }
> +
> + /*
> + * use the symlink as the remaining components that
> + * need to be resloved
> + */
> + strbuf_swap(&symlink, &remaining);
> + }
> }
>
> + retval = resolved.buf;
> +
> error_out:
> + strbuf_release(&remaining);
> + strbuf_release(&next);
> + strbuf_release(&symlink);
>
> return retval;
> }
^ permalink raw reply
* Re: [PATCH v3 2/4] real_path: convert real_path_internal to strbuf_realpath
From: Junio C Hamano @ 2016-12-12 22:20 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <1481566615-75299-3-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Change the name of real_path_internal to strbuf_realpath. In addition
> push the static strbuf up to its callers and instead take as a
> parameter a pointer to a strbuf to use for the final result.
>
> This change makes strbuf_realpath reentrant.
Yup, this step makes sense.
^ permalink raw reply
* Re: [PATCH v3 3/4] real_path: create real_pathdup
From: Junio C Hamano @ 2016-12-12 22:25 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <1481566615-75299-4-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Create real_pathdup which returns a caller owned string of the resolved
> realpath based on the provide path.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> abspath.c | 13 +++++++++++++
> cache.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/abspath.c b/abspath.c
> index 8c6c76b..79ee310 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -205,6 +205,19 @@ const char *real_path_if_valid(const char *path)
> return strbuf_realpath(&realpath, path, 0);
> }
>
> +char *real_pathdup(const char *path)
> +{
> + struct strbuf realpath = STRBUF_INIT;
> + char *retval = NULL;
> +
> + if (strbuf_realpath(&realpath, path, 0))
> + retval = strbuf_detach(&realpath, NULL);
> +
> + strbuf_release(&realpath);
> +
> + return retval;
> +}
OK. Taken alone, the above makes a reader wonder if it still makes
sense for strbuf_realpath() to return realpath.buf (or NULL upon
error). An obvious alternative is to return 0 on success and -1 on
failure.
But other callers of strbuf_realpath() are mimicking the historical
"give a path in 'const char *', receive a path in 'const char *'
that you have to xstrdup() if you want to keep it" interface, and
this interface may be easier for them. I dunno.
> /*
> * Use this to get an absolute path from a relative one. If you want
> * to resolve links, you should use real_path.
> diff --git a/cache.h b/cache.h
> index 7a81294..e12a5d9 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1068,6 +1068,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error);
> const char *real_path(const char *path);
> const char *real_path_if_valid(const char *path);
> +char *real_pathdup(const char *path);
> const char *absolute_path(const char *path);
> const char *remove_leading_path(const char *in, const char *prefix);
> const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
^ permalink raw reply
* Re: [PATCH v3 4/4] real_path: have callers use real_pathdup and strbuf_realpath
From: Junio C Hamano @ 2016-12-12 22:26 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <1481566615-75299-5-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Migrate callers of real_path() who duplicate the retern value to use
> real_pathdup or strbuf_realpath.
Looks good.
^ permalink raw reply
* Re: [PATCH 0/2] handling alternates paths with colons
From: Junio C Hamano @ 2016-12-12 22:37 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, Klaus Ethgen, git
In-Reply-To: <20161212194929.bdcihf7orjabzb2h@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So here are patches that do that. It kicks in only when the first
> character of a path is a double-quote, and then expects the usual
> C-style quoting.
The quote being per delimited component is what makes this fifth
approach a huge win.
All sane components on a list-valued environment are still honored
and an insane component that has either a colon in the middle or
begins with a double-quote gets quoted. As long as nobody used a
path that begins with a double-quote as an element in such a
list-valued environment (and they cannot be, as using a non-absolute
path as an element does not make much sense), this will be safe, and
a path with a colon didn't work with Git unaware of the new quoting
rule anyway. Nice.
^ permalink raw reply
* Re: [RFC PATCH] send-email: allow a custom hook to prevent sending email
From: Junio C Hamano @ 2016-12-12 22:38 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jeff King, Brandon Williams, git@vger.kernel.org
In-Reply-To: <CAGZ79kZVjsJjreKJgnOH1T-k-zfpfqivxw1gGm_dx4GU4tte2Q@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> $ git send-email 00* --cc=list --cc=bmwill --cc=duy --to=jch
> 0000-cover-letter.patch
> ...
> (mbox) Adding cc: Stefan Beller <sbeller@google.com> from line 'From:
> Stefan Beller <sbeller@google.com>'
>
> From: Stefan Beller <sbeller@google.com>
> To: gitster@pobox.com
> Cc: git@vger.kernel.org,
> bmwill@google.com,
> pclouds@gmail.com,
> Stefan Beller <sbeller@google.com>
> Subject: [PATCHv7 0/6] submodule absorbgitdirs
> Date: Mon, 12 Dec 2016 11:04:29 -0800
> Message-Id: <20161212190435.10358-1-sbeller@google.com>
> X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
>
> Send this email? ([y]es|[n]o|[q]uit|[a]ll):
>
> ---
> This is usually where I just say "a" and carry on with life.
> The hook would ideally reformat the last lines to be
> ---
> X-Mailer: git-send-email 2.11.0.rc2.49.ge1f3b0c.dirty
> send-email hook thinks it is a bad idea to send out this series:
> <output of hook, e.g.
> referencing a typo in patch 5.
> or a mismatch of patch version numbers>
>
> Send this email? ([y]es|[n]o|[q]uit|[a]ll):
> ---
Yup, sounds sensible (the "... thinks it is a bad idea ..." line may
probably not be needed; the hook can say why it is unhappy itself).
Thanks.
^ permalink raw reply
* Re: [PATCH v3 1/4] real_path: resolve symlinks by hand
From: Brandon Williams @ 2016-12-12 22:50 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <xmqqd1gw94f1.fsf@gitster.mtv.corp.google.com>
On 12/12, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > +/* removes the last path component from 'path' except if 'path' is root */
> > +static void strip_last_component(struct strbuf *path)
> > +{
> > + size_t offset = offset_1st_component(path->buf);
> > + size_t len = path->len;
> > +
> > + /* Find start of the last component */
> > + while (offset < len && !is_dir_sep(path->buf[len - 1]))
> > + len--;
>
> If somebody at a higher level in the callchain has already
> normalized path, this is not a problem, but this will behave
> "unexpectedly" when path ends with a dir_sep byte (or more).
>
> E.g. for input path "foo/bar/", the above loop runs zero times and
> then ...
>
> > + /* Skip sequences of multiple path-separators */
> > + while (offset < len && is_dir_sep(path->buf[len - 1]))
> > + len--;
>
> ... the slash at the end is removed, leaving "foo/bar" in path.
>
The way this is currently used I don't believe this scenario can happen
(since input to this shouldn't have trailing slashes), but if others
begin to use this function then yes, that is an implicit assumption. I
think it may be an ok assumption though since this is only called on
"resolved" which is the ouput and needs to be normalized to begin with. To
fix this we could simply add the while loop that strips dir_sep at the
beginning as well as at the end, like so:
/* Skip sequences of multiple path-separators */
while (offset < len && is_dir_sep(path->buf[len - 1]))
len--;
/* Skip sequences of multiple path-separators */
while (offset < len && !is_dir_sep(path->buf[len - 1]))
len--;
/* Skip sequences of multiple path-separators */
while (offset < len && is_dir_sep(path->buf[len - 1]))
len--;
> > + strbuf_setlen(path, len);
> > +}
> > ...
> > +/* get (and remove) the next component in 'remaining' and place it in 'next' */
> > +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> > +{
> > + char *start = NULL;
> > + char *end = NULL;
> > +
> > + strbuf_reset(next);
> > +
> > + /* look for the next component */
> > + /* Skip sequences of multiple path-separators */
> > + for (start = remaining->buf; is_dir_sep(*start); start++)
> > + ; /* nothing */
> > + /* Find end of the path component */
> > + for (end = start; *end && !is_dir_sep(*end); end++)
> > + ; /* nothing */
> > +
> > + strbuf_add(next, start, end - start);
> > + /* remove the component from 'remaining' */
> > + strbuf_remove(remaining, 0, end - remaining->buf);
> > +}
>
> Unlike the strip_last_component(), I think this one is more
> carefully done and avoids getting fooled by //extra//slashes// at
> the beginning or at the end, which does help in the correctness of
> the loop we see below.
>
> > @@ -58,74 +88,112 @@ static const char *real_path_internal(const char *path, int die_on_error)
> > goto error_out;
> > }
> >
> > + strbuf_reset(&resolved);
> > +
> > + if (is_absolute_path(path)) {
> > + /* absolute path; start with only root as being resolved */
> > + int offset = offset_1st_component(path);
> > + strbuf_add(&resolved, path, offset);
> > + strbuf_addstr(&remaining, path + offset);
> > + } else {
> > + /* relative path; can use CWD as the initial resolved path */
> > + if (strbuf_getcwd(&resolved)) {
> > + if (die_on_error)
> > + die_errno("unable to get current working directory");
> > + else
> > + goto error_out;
> > }
> > + strbuf_addstr(&remaining, path);
> > + }
> >
> > + /* Iterate over the remaining path components */
> > + while (remaining.len > 0) {
> > + get_next_component(&next, &remaining);
> > +
> > + if (next.len == 0) {
> > + continue; /* empty component */
> > + } else if (next.len == 1 && !strcmp(next.buf, ".")) {
> > + continue; /* '.' component */
> > + } else if (next.len == 2 && !strcmp(next.buf, "..")) {
> > + /* '..' component; strip the last path component */
> > + strip_last_component(&resolved);
>
> Wouldn't this let "resolved" eventually run out of the path
> components to strip for a malformed input e.g. "/a/../../b"?
>
As I understand it, that path is correct and would resolve to "/b". The
strip_last_component function doesn't allow striping the "1st" component
or the "root" component. So if resolved is "/" and we encounter a ".."
which requires striping of the last component, the result would be "/".
> > + ...
> > + /*
> > + * if there are still remaining components to resolve
> > + * then append them to symlink
> > + */
> > + if (remaining.len) {
> > + strbuf_addch(&symlink, '/');
>
> This can add duplicate dir_sep if readlink(2)'ed contents of the
> symbolic link already ends with a slash, but I think it (together
> with the fact that the code does nothing to normalize what is read
> from the symbolic link) probably does not matter, given the way how
> get_next_component() is implemented.
>
Yes, I think the way get_next_component() is written will account for
non-normalized symlink contents. This way we only have to worry about
normalizing input in one location (maybe two with
strip_last_component()).
> > + strbuf_addbuf(&symlink, &remaining);
> > + }
> > +
> > + /*
> > + * use the symlink as the remaining components that
> > + * need to be resloved
> > + */
> > + strbuf_swap(&symlink, &remaining);
> > + }
> > }
> >
> > + retval = resolved.buf;
> > +
> > error_out:
> > + strbuf_release(&remaining);
> > + strbuf_release(&next);
> > + strbuf_release(&symlink);
> >
> > return retval;
> > }
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v3 1/4] real_path: resolve symlinks by hand
From: Junio C Hamano @ 2016-12-12 23:32 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds
In-Reply-To: <20161212225006.GB193413@google.com>
Brandon Williams <bmwill@google.com> writes:
> On 12/12, Junio C Hamano wrote:
> ...
>> E.g. for input path "foo/bar/", the above loop runs zero times and
>> then ...
>>
>> > + /* Skip sequences of multiple path-separators */
>> > + while (offset < len && is_dir_sep(path->buf[len - 1]))
>> > + len--;
>>
>> ... the slash at the end is removed, leaving "foo/bar" in path.
>
> The way this is currently used I don't believe this scenario can happen
> (since input to this shouldn't have trailing slashes), but if others
> begin to use this function then yes, that is an implicit assumption.
OK.
>> > + /* Iterate over the remaining path components */
>> > + while (remaining.len > 0) {
>> > + get_next_component(&next, &remaining);
>> > +
>> > + if (next.len == 0) {
>> > + continue; /* empty component */
>> > + } else if (next.len == 1 && !strcmp(next.buf, ".")) {
>> > + continue; /* '.' component */
>> > + } else if (next.len == 2 && !strcmp(next.buf, "..")) {
>> > + /* '..' component; strip the last path component */
>> > + strip_last_component(&resolved);
>>
>> Wouldn't this let "resolved" eventually run out of the path
>> components to strip for a malformed input e.g. "/a/../../b"?
>
> As I understand it, that path is correct and would resolve to "/b".
That is OK on the traditional UNIX end.
I am not sure if we want to handle the "//host/share/$remaining" in
this codepath, though. If we do, then this is still an issue (IIRC,
somebody explained that we do not want to step into //host/share/
part when seeing ".."---we'd at least need to leave a NEEDSWORK
comment so that interested parties can later take a look into it.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox