[L10N] Kickoff of translation for Git 2.14.0 round 1

[Jiang Xin]

Hi,

Git v2.14.0-rc0 has been released, and it's time to start new round of git l10n.
This time there are 30+ updated messages need to be translated since last
update:

l10n: git.pot: v2.14.0 round 1 (34 new, 23 removed)

Generate po/git.pot from v2.14.0-rc0 for git v2.14.0 l10n round 1.

Signed-off-by: Jiang Xin 

You can get it from the usual place:

https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin

Re: [ANNOUNCE] Git v2.14.0-rc0

[Christian Couder]

On Sat, Jul 15, 2017 at 1:17 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Thu, Jul 13 2017, Junio C. Hamano jotted:

>>  * "git send-email" learned to overcome some SMTP server limitation
>>that does not allow many pieces of e-mails to be sent over a single
>>session.
>
> Makes it sound like it does that automatically, also "pieces of e-mails"
> and "sent over" is odd, maybe:
>
> * "git send-email" now has --batch-size and --relogin-delay options
>which can be used to overcome limitations on SMTP servers that have
>limits on how many of e-mails can be sent in a single session.

Maybe s/that have limits on how/that restrict how/

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Junio C Hamano]

Jeff King  writes:

>> There is also this:
>> 
>>   $ git branch
>>   * master
>>   $ git BRANCH
>>   git: 'BRANCH' is not a git command. See 'git --help'.
>>   $ git config alias.branch 'branch -v'
>>   $ git branch
>>   * master
>>   $ git BRANCH
>>   * master 51c785c initial
>
> That is an interesting side effect, especially the latter BRANCH/branch
> one. We usually do not allow overrides of actual git commands, but this
> "fools" that check.

This one is tricky.  An obvious fix would be, because users expect
that their aliases are case insensitive, the rule that forbids and
ignores an alias that masks a real git subcommand should also apply
case insensitively.  But that opens another can of worms.  If "git
Branch" is forbidden to be an alias, it should not just fail but
invoke "git branch", which would mean that Git subcommands should
also be looked up case insensitively!  I am not sure if we want to
go there.

> I agree it's an unexpected fallout. On the other hand, unless you are
> _trying_ to do something funny, I don't think you'd ever hit on this
> behavior. And if you are trying to do something funny, I think this
> behaves in a reasonable and predictable manner.

OK.

Re: [ANNOUNCE] Git v2.14.0-rc0

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 13 2017, Junio C. Hamano jotted:

Proposed improvements for the release notes (is this a good way to
propose RelNotes changes?)

> An early preview release Git v2.14.0-rc0 is now available for
> testing at the usual places.  It is comprised of 675 non-merge
> commits since v2.13.0, contributed by 53 people, 14 of which are
> new faces.
> [...]
> Git 2.14 Release Notes (draft)
> ==
>
> Backward compatibility notes.
>
>  * Use of an empty string as a pathspec element that is used for
>'everything matches' is still warned and Git asks users to use a
>more explicit '.' for that instead.  The hope is that existing
>users will not mind this change, and eventually the warning can be
>turned into a hard error, upgrading the deprecation into removal of
>this (mis)feature.  That is not scheduled to happen in the upcoming
>release (yet).
>
>  * Git now avoids 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").
>We've tried hard to locate such cases and fixed them, but there
>might still be cases that need to be addressed--bug reports are
>greatly appreciated.
>
>  * The experiment to improve the hunk-boundary selection of textual
>diff output has finished, and the "indent heuristics" has now
>become the default.
>
>
> Updates since v2.13
> ---
>
> UI, Workflows & Features
>
>  * The colors in which "git status --short --branch" showed the names
>of the current branch and its remote-tracking branch are now
>configurable.
>
>  * "git clone" learned the "--no-tags" option not to fetch all tags
>initially, and also set up the tagopt not to follow any tags in
>subsequent fetches.
>
>  * "git archive --format=zip" learned to use zip64 extension when
>necessary to go beyond the 4GB limit.
>
>  * "git reset" learned "--recurse-submodules" option.
>
>  * "git diff --submodule=diff" now recurses into nested submodules.
>
>  * "git repack" learned to accept the --threads= option and pass it
>to pack-objects.
>
>  * "git send-email" learned to run sendemail-validate hook to inspect
>and reject a message before sending it out.
>
>  * There is no good reason why "git fetch $there $sha1" should fail
>when the $sha1 names an object at the tip of an advertised ref,
>even when the other side hasn't enabled allowTipSHA1InWant.
>
>  * The recently introduced "[includeIf "gitdir:$dir"] path=..."
>mechanism has further been taught to take symlinks into account.
>The directory "$dir" specified in "gitdir:$dir" may be a symlink to
>a real location, not something that $(getcwd) may return.  In such
>a case, a realpath of "$dir" is compared with the real path of the
>current repository to determine if the contents from the named path
>should be included.

I think this is backwards, comparing the realpath is what we did in
2.13.0, i.e. if ~/work was a symlink to /mnt/storage/wor we'd match
"gitdir:~/work against /mnt/storage/work only (the realpath), now we
also match it against the `pwd` ~/git_tree.

This also makes it sound like now we just do it differently, whereas we
do both now.

I think this may explain it better:

 * The "[includeIf "gitdir:$dir"] path=..." mechanism introduced in
   2.13.0 would canonicalize the path of the gitdir being
   matched.

   Therefore it wouldn't match e.g. "gitdir:~/work/*" against a repo in
   "~/work/main" if ~/work was a symlink to "/mnt/storage/work".

   Now we match both the resolved canonical path and what "pwd" would
   show. The include will happen if either one matches.

>  * Make the "indent" heuristics the default in "diff" and 
> diff.indentHeuristics
>configuration variable an escape hatch for those who do no want it.

It's "diff.indentHeuristic" not "diff.indentHeuristics" (extra "s"). The
"and diff.indentHeuristics configuration variable" reads strangely to
me. Maybe this, and also explain what it's for (the explanation may be
shitty):

  * Make the "indent" heuristics the default in "diff". The
diff.indentHeuristic configuration variable can be set to "false" or
--no-indent-heuristic can be provided on the command-line for those
who do not want it.

The common case affected by the indent heuristic is to make diffs
that deal with editing the middle of repetitive code look nicer,
since they don't seem to e.g. "steal" parts of an earlier function
definition for their own use.

>  * Many commands learned to pay attention to submodule.recurse
>configuration.
>
>  * The convention for a command line is to follow "git cmdname
>--options" with revisions followed by an optional "--"
>disambiguator and then finally pathspecs.  When "--" is not there,
>we make sure early ones are all interpretable as revs (and do not
>look like paths) and later ones are the 

Re: [PATCH v6 02/10] rebase -i: generate the script via rebase--helper

[Stefan Beller]

On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin
 wrote:
> The first step of an interactive rebase is to generate the so-called "todo
> script", to be stored in the state directory as "git-rebase-todo" and to
> be edited by the user.
>
> Originally, we adjusted the output of `git log ` using a simple
> sed script. Over the course of the years, the code became more
> complicated. We now use shell scripting to edit the output of `git log`
> conditionally, depending whether to keep "empty" commits (i.e. commits
> that do not change any files).
>
> On platforms where shell scripting is not native, this can be a serious
> drag. And it opens the door for incompatibilities between platforms when
> it comes to shell scripting or to Unix-y commands.
>
> Let's just re-implement the todo script generation in plain C, using the
> revision machinery directly.
>
> This is substantially faster, improving the speed relative to the
> shell script version of the interactive rebase from 2x to 3x on Windows.

Thanks for working on this


> +int sequencer_make_script(int keep_empty, FILE *out,
> +   int argc, const char **argv)
> +{

> +   init_revisions(, NULL);
> +   revs.verbose_header = 1;
> +   revs.max_parents = 1;
> +   revs.cherry_pick = 1;
> +   revs.limited = 1;
> +   revs.reverse = 1;
> +   revs.right_only = 1;
> +   revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
> +   revs.topo_order = 1;
> +
> +   revs.pretty_given = 1;
> +   git_config_get_string("rebase.instructionFormat", );
> +   if (!format || !*format) {
> +   free(format);
> +   format = xstrdup("%s");
> +   }

https://public-inbox.org/git/xmqqvapqo4i8@gitster.mtv.corp.google.com/

So this is the core part that you and Junio have differing opinions on.

> All of the above feels like inviting unnecessary future breakages by
> knowing too much about the implementation the current version of
> revision.c happens to use.  A more careful implementation would be
> to allocate our own av[] and prepare "--reverse", "--left-right",
> "--cherry-pick", etc. to be parsed by setup_revisions() call we see
> below.  The parsing is not an expensive part of the operation
> anyway, and that way we do not have to worry about one less thing.

Allow me go through each of the options which may help
us finding a consensus (at least it helps me having a more
informed opinion).
List of options used outside of revision.c, which in the ideal
world of Git are parsed in e.g. handle_revision_opt called
from setup_revisions:

.verbose_header
  bisect.c:   opt.verbose_header = 1;
  builtin/commit.c:   rev.verbose_header = 1;
  builtin/log.c:  rev->verbose_header = 1;
  builtin/log.c:  rev.verbose_header = 1;
  builtin/log.c:  rev.verbose_header = 1;

.max_parents
  builtin/log.c:  check_rev.max_parents = 1;
  builtin/log.c:  revs.max_parents = 1;
  builtin/log.c:  rev.max_parents = 1;
  builtin/log.c:  revs.max_parents = 1;

.cherry_pick
  is clean!

.limited
  ref-filter.c:   revs.limited = 1;

.reverse
  seems clean.

.right_only:
.sort_order:
  is clean!

.topo_order:
  builtin/fast-export.c:  revs.topo_order = 1;
  builtin/log.c:  revs.topo_order = 1;

.pretty_given
  builtin/log.c:  if (!rev->show_notes_given && (!rev->pretty_given || w.notes))
  builtin/log.c:  if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {

There are two conflicting messages I get:
* only a few fields seem to be polluted (verbose_header,
  max_parents), much fewer than I thought
* we do use these undocumented ways already,
  but not at the scale that DScho is trying to here.

In the reply to the cover letter I outlined that we may have
a problem with integrating the repository struct when using
string arrays only.

Thanks,
Stefan

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Mike Hommey]

On Fri, Jul 14, 2017 at 09:11:33AM -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Oh, absolutely.
> >
> > Here is another possible test balloon, that may actually be useful
> > as an example.  I think there is a topic in flight that touches this
> > array, unfortunately, so I probably would find another one that is
> > more stable for a real follow-up patch to the one from Peff.
> 
> And here it is.
> 
> As to other things that we currently not allow in our codebase that
> newer compilers can grok, here is what *I* think.  It is *not* meant
> to be an exhaustive "what's new in C99 that is not in C89? what is
> the final verdict on each of them?":
> 
>  - There were occasional cases where we wished if variable-length
>arrays, flexible array members and variadic macros were available
>in our codebase during the course of this project.  We would
>probably want to add a similar test baloon patch for each of
>them to this series that is currently two-patch long.

FWIW, variadic macros have subtle implementation differences that can
cause problems.
For instance, MSVC only supports ##__VA_ARGS__ by way of
ignoring ## somehow, but has the default behavior of dropping the comma
when __VA_ARGS__ is empty, which means , __VA_ARGS__ *without* ## has a
different behavior.
See also 
https://connect.microsoft.com/VisualStudio/feedback/details/380090/variadic-macro-replacement

Mike

[PATCH v2 0/3] Convert grep to recurse in-process

[Brandon Williams]

Changes in v2:
* small style nits fixed.
* Comment describing function contract for repo_read_index
* NEEDSWORK comment in grep to describe the issue with adding the submodule's
  object store as an alternate.
* the_repository->index = _index was removed from setup.c and instead this
  is done as a static initializer.

Brandon Williams (3):
  repo_read_index: don't discard the index
  repository: have the_repository use the_index
  grep: recurse in-process using 'struct repository'

 Documentation/git-grep.txt |   7 -
 builtin/grep.c | 396 ++---
 cache.h|   1 -
 git.c  |   2 +-
 grep.c |  13 --
 grep.h |   1 -
 repository.c   |   6 +-
 repository.h   |   8 +
 setup.c|  12 +-
 9 files changed, 99 insertions(+), 347 deletions(-)

-- 
2.13.2.932.g7449e964c-goog

[PATCH v2 2/3] repository: have the_repository use the_index

[Brandon Williams]

Have the index state which is stored in 'the_repository' be a pointer to
the in-core index 'the_index'.  This makes it easier to begin
transitioning more parts of the code base to operate on a 'struct
repository'.

Signed-off-by: Brandon Williams 
---
 repository.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/repository.c b/repository.c
index 8e60af1d5..c0e0e0e7e 100644
--- a/repository.c
+++ b/repository.c
@@ -4,7 +4,9 @@
 #include "submodule-config.h"
 
 /* The main repository */
-static struct repository the_repo;
+static struct repository the_repo = {
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, 0, 0
+};
 struct repository *the_repository = _repo;
 
 static char *git_path_from_env(const char *envvar, const char *git_dir,
-- 
2.13.2.932.g7449e964c-goog

[PATCH v2 1/3] repo_read_index: don't discard the index

[Brandon Williams]

Have 'repo_read_index()' behave more like the other read_index family of
functions and don't discard the index if it has already been populated
and instead rely on the quick return of read_index_from which has:

  /* istate->initialized covers both .git/index and .git/sharedindex.xxx */
  if (istate->initialized)
return istate->cache_nr;

Signed-off-by: Brandon Williams 
---
 repository.c | 2 --
 repository.h | 8 
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/repository.c b/repository.c
index edca90740..8e60af1d5 100644
--- a/repository.c
+++ b/repository.c
@@ -235,8 +235,6 @@ int repo_read_index(struct repository *repo)
 {
if (!repo->index)
repo->index = xcalloc(1, sizeof(*repo->index));
-   else
-   discard_index(repo->index);
 
return read_index_from(repo->index, repo->index_file);
 }
diff --git a/repository.h b/repository.h
index 417787f3e..7f5e24a0a 100644
--- a/repository.h
+++ b/repository.h
@@ -92,6 +92,14 @@ extern int repo_submodule_init(struct repository *submodule,
   const char *path);
 extern void repo_clear(struct repository *repo);
 
+/*
+ * Populates the repository's index from its index_file, an index struct will
+ * be allocated if needed.
+ *
+ * Return the number of index entries in the populated index or a value less
+ * than zero if an error occured.  If the repository's index has already been
+ * populated then the number of entries will simply be returned.
+ */
 extern int repo_read_index(struct repository *repo);
 
 #endif /* REPOSITORY_H */
-- 
2.13.2.932.g7449e964c-goog

[PATCH v2 3/3] grep: recurse in-process using 'struct repository'

[Brandon Williams]

Convert grep to use 'struct repository' which enables recursing into
submodules to be handled in-process.

Signed-off-by: Brandon Williams 
---
 Documentation/git-grep.txt |   7 -
 builtin/grep.c | 396 ++---
 cache.h|   1 -
 git.c  |   2 +-
 grep.c |  13 --
 grep.h |   1 -
 setup.c|  12 +-
 7 files changed, 88 insertions(+), 344 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 5033483db..720c7850e 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -95,13 +95,6 @@ OPTIONS
 option the prefix of all submodule output will be the name of
the parent project's  object.
 
---parent-basename ::
-   For internal use only.  In order to produce uniform output with the
-   --recurse-submodules option, this option can be used to provide the
-   basename of a parent's  object to a submodule so the submodule
-   can prefix its output with the parent's name rather than the SHA1 of
-   the submodule.
-
 -a::
 --text::
Process binary files as if they were text.
diff --git a/builtin/grep.c b/builtin/grep.c
index fa351c49f..728755d6d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -28,13 +28,7 @@ static char const * const grep_usage[] = {
NULL
 };
 
-static const char *super_prefix;
 static int recurse_submodules;
-static struct argv_array submodule_options = ARGV_ARRAY_INIT;
-static const char *parent_basename;
-
-static int grep_submodule_launch(struct grep_opt *opt,
-const struct grep_source *gs);
 
 #define GREP_NUM_THREADS_DEFAULT 8
 static int num_threads;
@@ -186,10 +180,7 @@ static void *run(void *arg)
break;
 
opt->output_priv = w;
-   if (w->source.type == GREP_SOURCE_SUBMODULE)
-   hit |= grep_submodule_launch(opt, >source);
-   else
-   hit |= grep_source(opt, >source);
+   hit |= grep_source(opt, >source);
grep_source_clear_data(>source);
work_done(w);
}
@@ -327,21 +318,13 @@ static int grep_oid(struct grep_opt *opt, const struct 
object_id *oid,
 {
struct strbuf pathbuf = STRBUF_INIT;
 
-   if (super_prefix) {
-   strbuf_add(, filename, tree_name_len);
-   strbuf_addstr(, super_prefix);
-   strbuf_addstr(, filename + tree_name_len);
+   if (opt->relative && opt->prefix_length) {
+   quote_path_relative(filename + tree_name_len, opt->prefix, 
);
+   strbuf_insert(, 0, filename, tree_name_len);
} else {
strbuf_addstr(, filename);
}
 
-   if (opt->relative && opt->prefix_length) {
-   char *name = strbuf_detach(, NULL);
-   quote_path_relative(name + tree_name_len, opt->prefix, 
);
-   strbuf_insert(, 0, name, tree_name_len);
-   free(name);
-   }
-
 #ifndef NO_PTHREADS
if (num_threads) {
add_work(opt, GREP_SOURCE_OID, pathbuf.buf, path, oid);
@@ -366,15 +349,10 @@ static int grep_file(struct grep_opt *opt, const char 
*filename)
 {
struct strbuf buf = STRBUF_INIT;
 
-   if (super_prefix)
-   strbuf_addstr(, super_prefix);
-   strbuf_addstr(, filename);
-
-   if (opt->relative && opt->prefix_length) {
-   char *name = strbuf_detach(, NULL);
-   quote_path_relative(name, opt->prefix, );
-   free(name);
-   }
+   if (opt->relative && opt->prefix_length)
+   quote_path_relative(filename, opt->prefix, );
+   else
+   strbuf_addstr(, filename);
 
 #ifndef NO_PTHREADS
if (num_threads) {
@@ -421,284 +399,89 @@ static void run_pager(struct grep_opt *opt, const char 
*prefix)
exit(status);
 }
 
-static void compile_submodule_options(const struct grep_opt *opt,
- const char **argv,
- int cached, int untracked,
- int opt_exclude, int use_index,
- int pattern_type_arg)
-{
-   struct grep_pat *pattern;
-
-   if (recurse_submodules)
-   argv_array_push(_options, "--recurse-submodules");
-
-   if (cached)
-   argv_array_push(_options, "--cached");
-   if (!use_index)
-   argv_array_push(_options, "--no-index");
-   if (untracked)
-   argv_array_push(_options, "--untracked");
-   if (opt_exclude > 0)
-   argv_array_push(_options, "--exclude-standard");
-
-   if (opt->invert)
-   argv_array_push(_options, "-v");
-   if (opt->ignore_case)
-   argv_array_push(_options, "-i");
-   if 

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Fri, Jul 14 2017, Junio C. Hamano jotted:
>
>> Junio C Hamano  writes:
>>
>>  - This may be showing I am not just old fashioned but also am
>>ignorant, but I do not see much point in using the following in
>>our codebase (iow, I am not aware of places in the existing code
>>that they can be improved by employing these features):
>>
>>. // comments
>
> [Feel free to ignore this E-Mail and my fascination with C syntax
> trivia]
>
> I wouldn't advocate switching to them on this basis, but since you asked
> for cases where things could be improved with // comments:
>
> The other day I submitted a patch that included this line in a comment:
>
> +* "t/**.sh" and then conclude that there's a directory "t",

Yes, obviously I am very aware of this one.  

The thing is, I had to do this only once in the 10+ year since
project inception.  I find that much more relevant anecdata ;-).

Re: Git Bash Bug

[Johannes Schindelin]

Hi Paul,

On Fri, 14 Jul 2017, Paul Smith wrote:

> On Fri, 2017-07-14 at 22:33 +0200, Johannes Schindelin wrote:
> > > You absolutely have to have /bin and /usr/bin on your PATH,
> > 
> > As Kavita talks about Git Bash, it is probably Git for Windows, for
> > which /bin should not be in the PATH but /mingw64/bin or /mingw32/bin
> > (depending on the architecture).
> 
> I did check this with my Git for Windows installation before posting. 
> Mine is older (2.7.0) though so maybe things have changed:
> 
>   pds@build-win MINGW64 ~
>   $ type -a ls
>   ls is aliased to `ls -F --color=auto --show-control-chars'
>   ls is /usr/bin/ls
>   ls is /bin/ls
>   ls is /usr/bin/ls
>   ls is /usr/bin/ls

Oh wow ;-)

For the record, /bin/ is simply mapped to /usr/bin/. They are not
different directories, really.

> Clearly I have a lot of duplicates on my PATH now that I notice :)

;-)

> I have /mingw64/bin on my PATH as well but looking there it has git,
> gettext, edit, a bunch of DLL's, etc. but it doesn't contain ls or
> other coreutils programs.

Right. The difference between executables in /mingw64/bin/ and in
/usr/bin/ is that the latter executables all implicitly link to
msys-2.0.dll, i.e. the POSIX emulation layer based on Cygwin. As you
probably guessed: the coreutils are all compiled using that POSIX
emulation layer.

Ciao,
Johannes

Re: [RFC PATCH 1/3] promised-blob, fsck: introduce promised blobs

[Jonathan Nieder]

Jeff Hostetler wrote:
> On 7/13/2017 3:39 PM, Jonathan Tan wrote:

>> I know that discussion has shifted to the possibility of not having this
>> list at all, and not sending size information together with the fetch,
>> but going back to this...maybe omitting trees *is* the solution to both
>> the large local list and the large amount of size information needing to
>> be transferred.
>>
>> So the large-blob (e.g. Android) and many-blob (e.g. Windows) cases
>> would look like this:
>>
>>   * Large-blob repositories have no trees omitted and a few blobs
>> omitted, and we have sizes for all of them.
>>   * Many-blob repositories have many trees omitted and either all
>> blobs omitted (and we have size information for them, useful for FUSE
>> or FUSE-like things, for example) or possibly no blobs omitted (for
>> example, if shallow clones are going to be the norm, there won't be
>> many blobs to begin with if trees are omitted).
>
> I'm not sure I understand what you're saying here.  Does omitting a tree
> object change the set of blob sizes we receive?  Are you saying that if
> we omit a tree, then we implicitly omit all the blobs it references and
> don't send size info those blobs?  So that the local list only has
> reachable objects?  So faulting-in a tree would also have to send size
> info for the newly referenced blobs?
>
> Would this make it more similar to a shallow clone (in that none of the
> have_object tests work for items beyond the cut point) ?

Correct.  After the server sends a promise instead of a tree object, the
client has no reason to try to access blobs pointed to by that tree, any
more than it has reason to try to access commits on a branch it has not
fetched.  This means the client does not have to be aware of those blobs
until it fetches the tree and associated blob promises.

[...]
> For the former case, if you just have a few omitted objects, then a
> second round-trip to mget their sizes isn't that much work.

For the client, that is true.  For the server, decreasing the number
of requests even when requests are small and fast can be valuable.

[...]
> I think for the latter, forcing a full promise-list on clone is just
> too much data to send -- data that we likely won't ever need.

What did you think of the suggestion to not send promises for objects
that are only referenced by objects that weren't sent?

[...]
>> What do you think of doing this:
>>   * add a "type" field to the list of promised objects (formerly the list
>> of promised blobs)
>>   * retain mandatory size for blobs
>>   * retain single file containing list of promised objects (I don't feel
>> too strongly about this, but it has a slight simplicity and
>> in-between-GC performance advantage)
>
> The single promise-set is problematic.  I think it will grow too
> large (in our case) and will need all the usual lock juggling
> and merging.
>
> I still prefer my suggestion for a per-packfile promise-set for all
> of the reasons I stated the other day.  This can be computed quickly
> during index-pack, is (nearly) read-only, and doesn't require the
> whole file rewrite lock file.  It also has the benefit of being
> portable -- in that I can also copy the .promise file if I copy the
> .pack and .idx file to another repo.

Okay.

Thanks,
Jonathan

Re: Git Bash Bug

[Paul Smith]

On Fri, 2017-07-14 at 22:33 +0200, Johannes Schindelin wrote:
> > You absolutely have to have /bin and /usr/bin on your PATH,
> 
> As Kavita talks about Git Bash, it is probably Git for Windows, for
> which /bin should not be in the PATH but /mingw64/bin or /mingw32/bin
> (depending on the architecture).

I did check this with my Git for Windows installation before posting. 
Mine is older (2.7.0) though so maybe things have changed:

  pds@build-win MINGW64 ~
  $ type -a ls
  ls is aliased to `ls -F --color=auto --show-control-chars'
  ls is /usr/bin/ls
  ls is /bin/ls
  ls is /usr/bin/ls
  ls is /usr/bin/ls

Clearly I have a lot of duplicates on my PATH now that I notice :)

I have /mingw64/bin on my PATH as well but looking there it has git,
gettext, edit, a bunch of DLL's, etc. but it doesn't contain ls or
other coreutils programs.

Cheers!

Re: [PATCH v6 00/10] The final building block for a faster rebase -i

[Johannes Schindelin]

Hi Stefan,

On Fri, 14 Jul 2017, Stefan Beller wrote:

> On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin
>  wrote:
>
> >  -static int subject2item_cmp(const struct subject2item_entry *a,
> >  -  const struct subject2item_entry *b, const void *key)
> >  +static int subject2item_cmp(const void *fndata,
> 
> This could also be named unused_fndata.

Sure. I simply took the version Junio used when he merged the previous
patch series iteration into `pu`.

The function is short enough, though, that I feel it obvious that the
parameter is unused.

Ciao,
Dscho

Re: Git Bash Bug

[Johannes Schindelin]

Hi,

On Fri, 14 Jul 2017, Paul Smith wrote:

> On Fri, 2017-07-14 at 09:59 -0500, Kavita Desai wrote:
> > What does "echo $PATH" show?
> > /c/Users/Kavita/
> 
> Well, there you go.  That's clearly wrong.
> 
> You absolutely have to have /bin and /usr/bin on your PATH,

As Kavita talks about Git Bash, it is probably Git for Windows, for which
/bin should not be in the PATH but /mingw64/bin or /mingw32/bin (depending
on the architecture).

It may be messed up by some strange setting in the User Environment. Go to
the Windows Explorer, right-click on "This PC", select "Properties", in
the newly-opened window ("Control Panel>All Control Panel Items>System"),
next to the "Computer name, domain, and workgroup settings" select "Change
settings" which brings up the "System Properties" window, in whose
"Advanced" tab is an "Environment Variables" button.

(Or hit the Windows key, type "Edit the system environment variables")

You should find the "Path" variable at least in the System variables. It
is case-insensitive, so if your User variables contain, say, an empty
value for a key "paTH", then the riddle is solved.

Ciao,
Johannes

Re: reftable: new ref storage format

[Jeff King]

On Thu, Jul 13, 2017 at 05:27:44PM -0700, Shawn Pearce wrote:

> > We _could_ consider gzipping individual blocks of
> > a reftable (or any structure that allows you to search to a
> > constant-sized block and do a linear search from there). But given that
> > they're in the same ballpark, I'm happy with whatever ends up the
> > simplest to code and debug. ;)
> 
> This does help to shrink the file, e.g. it drops from 28M to 23M.
> 
> It makes it more CPU costly to access a block, as we have to inflate
> that to walk through the records. It also messes with alignment. When
> you touch a block, that may be straddling two virtual memory pages in
> your kernel/filesystem.
> 
> I'm not sure those penalties are worth the additional 16% reduction in size.

Yeah, I don't really care about a 16% reduction in size. I care much
more about simplicity of implementation and debugging. Using zlib is
kind-of simple to implement. But if you've ever had to debug it (or
figure out what is going on with maybe-corrupted output), it's pretty
nasty.

So I don't mind a more readable custom compression if it's not too
complicated. And especially if it buys us extra performance by being
able to jump around non-sequentially in the block.

-Peff

Re: reftable: new ref storage format

[Jeff King]

On Thu, Jul 13, 2017 at 05:11:52PM -0700, Shawn Pearce wrote:

> Yes, I was hoping for reader atomicity. But I may OK foregoing that if
> the transaction either all goes through, or all fails. A partially
> stuck transaction because the process died in the middle of the commit
> step creates a mess for an administrator to undo. Does she rename
> "foo.lock" to "foo"? Or delete "foo.lock"?

Agreed, there's no real rollback or recovery process. I do think
shooting for reader atomicity is worth doing. Lack of atomicity can
cause odd things to happen with operations like pruning, for example. If
I'm trying to get a list of all of the reachable objects, for example, I
might have to readdir() a bunch of directories (let's forget even that a
single readdir() is not necessarily atomic). If I try to atomically move
"refs/heads/z/foo" to "refs/heads/a/foo" there is a reasonable chance
that a reader may see only the deletion and not the addition.

I don't have any known cases of this biting anyone, but it's somewhat
scary.

> > But I'm not sure if you meant to contrast here a system where we didn't
> > use packed-refs at all (though of course such a system is very much not
> > atomic by the definition above).
> 
> No, I really did mean the current system. Gerrit Code Review servers
> create a lot of references throughout the day. Its easy to accumulate
> a few thousand new loose references in a 24 hour period. Even if you
> have 600k existing refs in packed-refs, you still have 2k new/modified
> refs since last nightly cron ran git gc.

I do think you'd be better served by just calling pack-refs more
frequently, then. Nightly is too infrequent for a busy repo. And under
something like reftables, you'd end up doing the equivalent of a
pack-refs every N updates anyway.

We actually pack refs quite aggressively at GitHub. Way more than I
would consider reasonable, but it's never been a big bottleneck, so I've
never looked into it. We don't do it for every update, but every update
triggers a "consider syncing objects into shared storage" job, which
will pack the refs. So in a hypothetical repo that's constantly updating
we probably pack refs at least once a minute.

But we're generally on low-latency local disks. It sounds like you
emphatically are not.

> > Good goal.  Just to play devil's advocate, the simplest way to do that
> > with the current code would be to gzip packed-refs (and/or store sha1s
> > as binary). That works against the "mmap and binary search" plan,
> > though. :)
> 
> Yes it does. I tried to cover that later under "Alternatives
> considered > bzip". :)

Yeah, sorry, I read and responded to the document a bit out of order. I
agree it's a dead-end. :)

> >> Reference names should be encoded with UTF-8.
> >
> > Don't we usually treat refnames as byte sequences (subject to a few
> > rules, as in check_ref_format())? It seems like the encoding should be
> > out-of-scope for the storage format.
> 
> True that git-core treats them as byte sequences, but JGit treats them as 
> UTF-8.

I think we can probably stick with that, unless the UTF-8ness is really
important? I guess it might matter if it impacts the sorting order.

> Yes. My "near constant time" claim was because I had my head buried
> thinking about disk IO operations when I wrote this, not the algorithm
> that happens on the CPU. One of the applications for reftable is on a
> slower-than-usual storage system, where reading a block of a file
> costs enough milliseconds that it doesn't matter how good or bad the
> CPU algorithm is.

OK, that explains a lot of the decisions better. If you're planning on
evolving this proposal document, I think it would make sense to talk
about that in the objectives section. (I say "if" because I am happy for
the mailing list discussion to serve as a rationale document).

> But you say this yourself below, you can do this using a geometric
> scheme or something to bound the cost of these rewrites such that you
> aren't frequently rewriting the whole database.

Right, but that sounds like math. I wanted you to spoonfeed me the
geometric algorithm (and its bound proof). ;)

> >> Compaction is similar to the update process, but an explicit temporary
> >> file must be used:
> >>
> >> 1. Atomically create `$GIT_DIR/reftable.lock`.
> >> 2. `readdir($GIT_DIR)` to determine the highest suffix ordinal, `n`.
> >> 3. Compute the update transaction (e.g. compare expected values).
> >> 4. Select files from (2) to collapse as part of this transaction.
> >> 5. Create temp file by `mktemp("$GIT_DIR/.reftableXX")`.
> >> 6. Write modified and collapsed references to temp file.
> >> 7. Rename temp file to `reftable.${n + 1}`.
> >> 8. Delete collapsed files `reftable.${n}`, `reftable.${n - 1}`, ...
> >> 9. Delete `reftable.lock`.

I think the "stack" implementation is what makes me most uncomfortable
with this proposal. Atomicity with filesystem operations and especially
readdir() is one of the things I think is most flaky about the 

Re: [RFC PATCH 1/3] promised-blob, fsck: introduce promised blobs

[Jeff Hostetler]

On 7/13/2017 3:39 PM, Jonathan Tan wrote:

On Wed, 12 Jul 2017 13:29:11 -0400
Jeff Hostetler  wrote:


My primary concern is scale and managing the list of objects over time.

My fear is that this list will be quite large.  If we only want to omit
the very large blobs, then maybe not.  But if we want to expand that
scope to also omit other objects (such as a clone synchronized with a
sparse checkout), then that list will get large on large repos.  For
example, on the Windows repo we have (conservatively) 100M+ blobs (and
growing).  Assuming 28 bytes per, gives a 2.8GB list to be manipulated.

If I understand your proposal, newly-omitted blobs would need to be
merged into the promised-blob list after each fetch.  The fetch itself
may not have that many new entries, but inserting them into the existing
list will be slow.  Also, mmap'ing and bsearch'ing will likely have
issues.  And there's likely to be a very expensive step to remove
entries from the list as new blobs are received (or locally created).

In such a "sparse clone", it would be nice to omit unneeded tree objects
in addition to just blobs.   I say that because we are finding with GVFS
on the Windows repo, that even with commits-and-trees-only filtering,
the number of tree objects is overwhelming.


I know that discussion has shifted to the possibility of not having this
list at all, and not sending size information together with the fetch,
but going back to this...maybe omitting trees *is* the solution to both
the large local list and the large amount of size information needing to
be transferred.

So the large-blob (e.g. Android) and many-blob (e.g. Windows) cases
would look like this:

  * Large-blob repositories have no trees omitted and a few blobs
omitted, and we have sizes for all of them.
  * Many-blob repositories have many trees omitted and either all
blobs omitted (and we have size information for them, useful for FUSE
or FUSE-like things, for example) or possibly no blobs omitted (for
example, if shallow clones are going to be the norm, there won't be
many blobs to begin with if trees are omitted).


I'm not sure I understand what you're saying here.  Does omitting a tree
object change the set of blob sizes we receive?  Are you saying that if
we omit a tree, then we implicitly omit all the blobs it references and
don't send size info those blobs?  So that the local list only has
reachable objects?  So faulting-in a tree would also have to send size
info for the newly referenced blobs?

Would this make it more similar to a shallow clone (in that none of the
have_object tests work for items beyond the cut point) ?



This seems better than an intermediate solution for the many-blob
repository case in which we still keep all the trees but also try to
avoid sending and storing as much information about the blobs as
possible, because that doesn't seem to provide us with much savings
(because the trees as a whole are just as large, if not larger, than the
blob information).


Or are you saying that by omitting mostly-unused trees (and sending just
their sizes) we can shrink the packfile payload and therefore afford to
send the full set of blob sizes in the promises payload ?



So I'm also concerned about
limiting the list to just blobs.  If we need to have this list, it
should be able to contain any object.  (Suggesting having an object type
in the entry.)


This makes sense - I'll add it in.


Good.  That will solve some cases where the client is just asking if
we have an object and is it of type x.



I also have to wonder about the need to have a complete list of omitted
blobs up front.  It may be better to just relax the consistency checks
and assume a missing blob is "intentionally missing" rather than
indicating a corruption somewhere.  And then let the client do a later
round-trip to either demand-load the object -or- demand-load the
existence/size info if/when it really matters.

Maybe we should add a verb to your new fetch-blob endpoint to just get
the size of one or more objects to help with this.


If we allow the omission of trees, I don't think the added complexity of
demand-loading sizes is worth it.


As you suggested earlier, we're trying to solve 2 different problems.
The "few omitted very large blobs" and the "way too many unneeded blobs"
problems.

For the former case, if you just have a few omitted objects, then a
second round-trip to mget their sizes isn't that much work.

I think for the latter, forcing a full promise-list on clone is just
too much data to send -- data that we likely won't ever need.  If the
client can compute the promise-set from the received packfile, we can
do the "is an object and type" tests completely locally.  It's only
when the client asks for the size do we need to decide whether to fetch
just the size or fetch the actual object.  Either type of request
doesn't seem like that big of a deal.  But we save a lot of IO
(especially during the initial 

Re: [PATCH v2 00/13] object_id part 9

[Brandon Williams]

On 07/13, brian m. carlson wrote:
> This is the ninth in a series of series to convert Git to use struct
> object_id.  This series converts the remaining callers of get_sha1 and
> friends to take and use struct object_id, and in doing so, renames them
> to get_oid and friends.
> 
> This patch should probably be based Stefan Beller's series, in which case
> patch 9 can be dropped.
> 
> Changes from v1:
> * Restore the check for length in get_sha1_basic.
> * Add a patch converting some uses of 40 to GIT_SHA1_HEXSZ as suggested.  This
>   is a separate patch because I wanted to minimize the non-automated portions 
> of
>   the patch in question.
> 

I've looked through v2 and it looks good to me as nothing crazy stood
out to me.

Thanks!

> brian m. carlson (13):
>   builtin/fsck: convert remaining caller of get_sha1 to object_id
>   builtin/merge-tree: convert remaining caller of get_sha1 to object_id
>   submodule: convert submodule config lookup to use object_id
>   remote: convert struct push_cas to struct object_id
>   sequencer: convert to struct object_id
>   builtin/update_ref: convert to struct object_id
>   bisect: convert bisect_checkout to struct object_id
>   builtin/unpack-file: convert to struct object_id
>   builtin/verify-tag: convert to struct object_id
>   Convert remaining callers of get_sha1 to get_oid.
>   sha1_name: convert get_sha1* to get_oid*
>   sha1_name: convert GET_SHA1* flags to GET_OID*
>   sha1_name: convert uses of 40 to GIT_SHA1_HEXSZ
> 
>  apply.c  |   4 +-
>  archive.c|   2 +-
>  bisect.c |  18 +--
>  builtin/am.c |   6 +-
>  builtin/cat-file.c   |   8 +-
>  builtin/commit-tree.c|   4 +-
>  builtin/commit.c |   8 +-
>  builtin/fsck.c   |   8 +-
>  builtin/grep.c   |   8 +-
>  builtin/log.c|   4 +-
>  builtin/merge-tree.c |   6 +-
>  builtin/receive-pack.c   |   4 +-
>  builtin/replace.c|   4 +-
>  builtin/reset.c  |  10 +-
>  builtin/rev-parse.c  |   8 +-
>  builtin/show-branch.c|   8 +-
>  builtin/submodule--helper.c  |   8 +-
>  builtin/unpack-file.c|  12 +-
>  builtin/update-ref.c |  69 ++-
>  builtin/verify-tag.c |   8 +-
>  cache.h  |  45 
>  commit.c |   4 +-
>  config.c |  12 +-
>  config.h |   4 +-
>  mailmap.c|   6 +-
>  notes.c  |   2 +-
>  refs.c   |   2 +-
>  remote.c |   8 +-
>  remote.h |   2 +-
>  repository.c |   2 +-
>  revision.c   |  16 +--
>  sequencer.c  |  65 +--
>  sha1_name.c  | 240 
> +++
>  submodule-config.c   |  38 +++
>  submodule-config.h   |  12 +-
>  submodule.c  |  32 +++---
>  submodule.h  |   2 +-
>  t/helper/test-submodule-config.c |  10 +-
>  transport-helper.c   |   2 +-
>  39 files changed, 351 insertions(+), 360 deletions(-)
> 

-- 
Brandon Williams

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 14 2017, Junio C. Hamano jotted:

> Junio C Hamano  writes:
>
>  - This may be showing I am not just old fashioned but also am
>ignorant, but I do not see much point in using the following in
>our codebase (iow, I am not aware of places in the existing code
>that they can be improved by employing these features):
>
>. // comments

[Feel free to ignore this E-Mail and my fascination with C syntax
trivia]

I wouldn't advocate switching to them on this basis, but since you asked
for cases where things could be improved with // comments:

The other day I submitted a patch that included this line in a comment:

+* "t/**.sh" and then conclude that there's a directory "t",

Which you fixed up to, before integrating it:

+* "t/" + "**.sh" and then conclude that there's a directory "t",

That was only necessary due to limitations in one of two comment
syntaxes modern C supports.

Well, it wasn't *necessary*, but a compiler warned about the /* there as
a possibly confusing construct, and any compiler would have ended the
comment right there + errored out if it contained "t/**/*.sh".

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Junio C Hamano]

Junio C Hamano  writes:

> Do we need to have a check to detect a buggy compiler that takes the
> syntax but produces an incorrectly initialized array?  I could add a
> test to ensure that HEADER comes out BOLD, etc. (or we may already
> have such a test) and then reorder these lines in this patch, if
> that is the kind of breakage we anticipate.

So here is a lunch-time hack I did to replace the patch I sent
earlier.  I kind of like the ordering of the elements better than
the original, in that it somehow feels more logical, even though I
merely sorted alphabetically ;-).


 builtin/clean.c  | 19 ++-
 t/t7301-clean-interactive.sh | 10 ++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 057fc97fe4..e2bb3c69ed 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -33,15 +33,6 @@ static const char *msg_skip_git_dir = N_("Skipping 
repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
 
-static int clean_use_color = -1;
-static char clean_colors[][COLOR_MAXLEN] = {
-   GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_BOLD_BLUE,/* PROMPT */
-   GIT_COLOR_BOLD, /* HEADER */
-   GIT_COLOR_BOLD_RED, /* HELP */
-   GIT_COLOR_BOLD_RED, /* ERROR */
-};
 enum color_clean {
CLEAN_COLOR_RESET = 0,
CLEAN_COLOR_PLAIN = 1,
@@ -51,6 +42,16 @@ enum color_clean {
CLEAN_COLOR_ERROR = 5
 };
 
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+   [CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
+   [CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
+   [CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
+   [CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
+   [CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
+   [CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
+};
+
 #define MENU_OPTS_SINGLETON01
 #define MENU_OPTS_IMMEDIATE02
 #define MENU_OPTS_LIST_ONLY04
diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh
index 3ae394e934..556e1850e2 100755
--- a/t/t7301-clean-interactive.sh
+++ b/t/t7301-clean-interactive.sh
@@ -472,4 +472,14 @@ test_expect_success 'git clean -id with prefix and path 
(ask)' '
 
 '
 
+test_expect_success 'git clean -i paints the header in HEADER color' '
+   >a.out &&
+   echo q |
+   git -c color.ui=always clean -i |
+   test_decode_color |
+   head -n 1 >header &&
+   # not i18ngrep
+   grep "^" header
+'
+
 test_done

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Junio C Hamano]

Jeff King  writes:

>> +static int clean_use_color = -1;
>> +static char clean_colors[][COLOR_MAXLEN] = {
>> +[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
>> +[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
>> +[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
>> +[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
>> +[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
>> +[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
>> +};
>
> I think this is much nicer to read. I assume if we have a "hole" in our
> numbering that the hole is initialized in the usual static way (a
> COLOR_MAXLEN array full of NULs in this case, I guess)?

I would expect that would be the case.  

Do we need to have a check to detect a buggy compiler that takes the
syntax but produces an incorrectly initialized array?  I could add a
test to ensure that HEADER comes out BOLD, etc. (or we may already
have such a test) and then reorder these lines in this patch, if
that is the kind of breakage we anticipate.

[PATCH] check-ref-format: require a repository for --branch

[Jeff King]

On Fri, Jul 14, 2017 at 02:03:13PM -0400, Jeff King wrote:

> So I think the patch below is probably the right direction.

And here it is with a real commit message, if this is what we want to
do.

I do feel like "check-ref-format --branch" is a misfeature. The
operation seems to fit better with "rev-parse" (which yes, is a kitchen
sink, but it does all sorts of similar resolution operations). I think
"git rev-parse --symbolic-full-name @{-1}" is basically the same thing
(modulo the refs/heads/ shortening).

-- >8 --
Subject: [PATCH] check-ref-format: require a repository for --branch

When the user asks "--branch" to interpret a branch name
like "@{-1}", we have to dig the answer out of the HEAD
reflog. We can obviously only do that if we have a
repository, and indeed, running it outside a repository
causes us to hit a BUG().

We basically have two options:

  1. We can define "@{-N}" outside of a repository as "no
 such branch" and die with "not a valid brach name".

  2. We can just declare that "--branch" must be run inside
 a repository, in which case we die with "not a git
 repository".

The effect is more or less the same for "@{-N}".
Technically one can use "--branch" outside of a repository
as long as you don't use any names that actually need
interpreting. But since intrpreting is the option's
documented purpose, there doesn't seem any point in trying
to resolve vanilla names like "foo" (which we end up just
printing to stdout verbatim).

So let's go with option 2.

Signed-off-by: Jeff King 
---
 builtin/check-ref-format.c  | 3 +--
 t/t1402-check-ref-format.sh | 4 
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450..1e5f9835f 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -39,9 +39,8 @@ static char *collapse_slashes(const char *refname)
 static int check_ref_format_branch(const char *arg)
 {
struct strbuf sb = STRBUF_INIT;
-   int nongit;
 
-   setup_git_directory_gently();
+   setup_git_directory();
if (strbuf_check_branch_ref(, arg))
die("'%s' is not a valid branch name", arg);
printf("%s\n", sb.buf + 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60..1674b061e 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -161,6 +161,10 @@ test_expect_success 'check-ref-format --branch from 
subdir' '
test "$refname" = "$sha1"
 '
 
+test_expect_success 'check-ref-format --branch from non-repo' '
+   test_must_fail nongit git check-ref-format --branch @{-1}
+'
+
 valid_ref_normalized() {
prereq=
case $1 in
-- 
2.14.0.rc0.457.g08326e0ba

Re: bug with check-ref-format outside of repository

[Jeff King]

On Sat, Jul 08, 2017 at 03:17:32AM +0200, Marko Kungla wrote:

> As contrived e.g: if I have in my "workspace" sub directories mostly
> git repositories, but some not and if I exec,
> "for d in */ ; do (cd $d && git check-ref-format --branch @{-1});
> done" then I get 3 possible responses.
> 
> git version: 2.13.0
> 1. Valid branch name
> 2. fatal: '@{-1}' is not a valid branch name
> 3. fatal: BUG: setup_git_env called without repository
> 
> git version 2.13.2.915.ga9c46e097
> 1. Valid branch name
> 2. fatal: '@{-1}' is not a valid branch name
> 3. BUG: environment.c:178: git environment hasn't been setup

Thanks for the report. It's right to return an error, but obviously we
should never hit the BUG() in the lazy-setup code.

I think "check-ref-format --branch" is inherently a repository-only
operation, since its purpose is to look in the reflog with @{-1} and
similar branch-substitutions. Technically you can do:

  $ cd /not/a/git/repo
  $ git check-ref-format --branch 'refs/heads/foo'

but I'm not sure why you would want to. So I think the patch below is
probably the right direction. The other alternative is for
interpret_nth_prior_checkout() to detect the "not in a repo" case and
quietly return "nope, we couldn't find such a reflog entry".

+cc Jonathan, who added the original call in 49cc460d8 (Allow
"check-ref-format --branch" from subdirectory, 2010-08-05). That commit
message doesn't give any indication of why we used the gently form.
Looking back at the original thread[1], I suspect it was mostly out of
conservatism.

[1] https://public-inbox.org/git/20100806033922.GS22369@burratino/

---
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450..1e5f9835f 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -39,9 +39,8 @@ static char *collapse_slashes(const char *refname)
 static int check_ref_format_branch(const char *arg)
 {
struct strbuf sb = STRBUF_INIT;
-   int nongit;
 
-   setup_git_directory_gently();
+   setup_git_directory();
if (strbuf_check_branch_ref(, arg))
die("'%s' is not a valid branch name", arg);
printf("%s\n", sb.buf + 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60..1674b061e 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -161,6 +161,10 @@ test_expect_success 'check-ref-format --branch from 
subdir' '
test "$refname" = "$sha1"
 '
 
+test_expect_success 'check-ref-format --branch from non-repo' '
+   test_must_fail nongit git check-ref-format --branch @{-1}
+'
+
 valid_ref_normalized() {
prereq=
case $1 in

Re: [PATCH] commit & merge: modularize the empty message validator

[Kaartic Sivaraam]

On Thu, 2017-07-13 at 12:23 -0700, Junio C Hamano wrote:
> All good points; if it bothers you that "commit" and "merge" define
> "emptyness" of the buffer differently too much, I think you could
> persuade me to unify them to "the buffer _must_ contain no bytes",
> i.e. not special-casing sign-off lines only "commit".
> 
Intereseting, let me give it a try.

To persuade you with this, I have to convince you that the current
behaviour (special-casing of sign-off lines) is defective and/or
biased. It really is for quite a few reasons,

* Though it's not apparent, it indirectly seems to be hindering
(to some extent) the idea of including the sign-off (or) other
trailers which *can't be modified* by the user.

IOW, the current behaviour seems make the contributors/users
falsely believe (at least to some extent) that git *does* have
trailers for commit messages and thus preventing them from coming
up with ideas that could make "untouchable trailers" a reality.

Thus, consider "the buffer _must_ contain no bytes" hoping this
would initiate a "Butterfly effect" :)


* Looking from an implementation perspective, it's biased in that
it checks only for sign-offs. Making it work in general is
difficult as there's no standard definition for the term
. That's because it varies with respect to usage, I
think. Different people/projects may consider different lines to
be trailer lines. A few examples are,

* Bug:
* Fixes:
* Change-id:
* Helped-by:

Moreover, some people may wish to have commit messages that only
have such trailers (e.g. "Fixes:"). So, it's difficult to do a
generalized implementation that aborts when the message is empty
or consists only of trailers.

Thus, consider "the buffer _must_ contain no bytes" because it's
not easy to define what a  means and special casing
"sign-off" is biased.


* Imagine a hypothetical version of git that aborts when the
 is empty though a  is present. This would
quite possibly instigate controversies as the "hypothetical git"
reduces the "valid commit messages" and would quite possibly
reject a commit message as "empty" (which is uncommunicative)
though a previous version (which did not have this change)
accepted a similar message.

SO, bringing in the Occam's razor, let's choose the option that's
the simplest and makes the fewest assumptions.


Thus, I conclude that the considering a commit message consisting only
of s as empty isn't a good idea and should be dropped.


-- 
Kaartic

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Jeff King]

On Fri, Jul 14, 2017 at 05:26:00PM +, astian wrote:

> FWIW, I don't like 2, I don't like the irregularity in the invocation:

Without quoting, it took me a second to figure out what you meant. But I
think "2" here is the "mental model 2" I mentioned in my earlier email.

>   $ git branch
>   * master
>   $ git BRANCH
>   git: 'BRANCH' is not a git command. See 'git --help'.
>   $ git config alias.br 'branch -v'
>   $ git br
>   * master 51c785c initial
>   $ git BR
>   * master 51c785c initial
> 
> There is also this:
> 
>   $ git branch
>   * master
>   $ git BRANCH
>   git: 'BRANCH' is not a git command. See 'git --help'.
>   $ git config alias.branch 'branch -v'
>   $ git branch
>   * master
>   $ git BRANCH
>   * master 51c785c initial

That is an interesting side effect, especially the latter BRANCH/branch
one. We usually do not allow overrides of actual git commands, but this
"fools" that check.

I agree it's an unexpected fallout. On the other hand, unless you are
_trying_ to do something funny, I don't think you'd ever hit on this
behavior. And if you are trying to do something funny, I think this
behaves in a reasonable and predictable manner.

-Peff

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Jeff King]

On Fri, Jul 14, 2017 at 09:11:33AM -0700, Junio C Hamano wrote:

> As to other things that we currently not allow in our codebase that
> newer compilers can grok, here is what *I* think.  It is *not* meant
> to be an exhaustive "what's new in C99 that is not in C89? what is
> the final verdict on each of them?":
> 
>  - There were occasional cases where we wished if variable-length
>arrays, flexible array members and variadic macros were available
>in our codebase during the course of this project.  We would
>probably want to add a similar test baloon patch for each of
>them to this series that is currently two-patch long.

I think variable-length arrays are potentially dangerous. They're
allocated on the stack, which creates two issues:

  1. You can run out of stack space and segfault, whereas the same
 operation with a heap buffer would be fine. You can say "but this
 VLA will only be used for small things". But then, you can just as
 easily declare a small stack buffer.

  2. My understanding of the recent "Stack Clash" class of
 vulnerabilities[1] is that VLAs make the attacker's job much easier
 (since they can often just send a large input to get you to
 allocate a large stack).

I think variadic macros are a good candidate, though. There have been a
number of times where we've had to sacrifice functionality or
readability in our helper functions. E.g., the case mentioned in
368953912 (add helpers for allocating flex-array structs, 2016-02-22).

The weather-balloon patch for that should be easy, too: just drop the
fallback macros from BUG() or the trace code.

[1] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt

>  - I prefer to keep decl-after-statement out of our codebase.  I
>view it as a big plus in code-readability to be able to see a
>complete list of variables that will be used in a block upfront
>before starting to read the code that uses them.
> 
>  - Corollary to the above, I do not mind to have a variable
>declaration in the initialization clause of a for() statement
>(e.g. "for (int i = 0; i < 4; i++) { ... }"), as the scoping rule
>is very sensible.  Some of our "for()" statements use the value
>of the variable after iteration, for which this new construct
>cannot be used, though.

I agree with both of those points. I think the decl-in-for is nice
exactly because it highlights those cases where the iteration variable's
value is relevant after the loop ends.

>  - This may be showing I am not just old fashioned but also am
>ignorant, but I do not see much point in using the following in
>our codebase (iow, I am not aware of places in the existing code
>that they can be improved by employing these features):
> 
>. // comments
>. restricted pointers
>. static and type qualifiers in parameter array declarators

Agreed, though I think the comment thing is a personal taste issue (just
not my taste).

> +static int clean_use_color = -1;
> +static char clean_colors[][COLOR_MAXLEN] = {
> + [CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
> + [CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
> + [CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
> + [CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
> + [CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
> + [CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
> +};

I think this is much nicer to read. I assume if we have a "hole" in our
numbering that the hole is initialized in the usual static way (a
COLOR_MAXLEN array full of NULs in this case, I guess)?

-Peff

Re: [PATCH v6 00/10] The final building block for a faster rebase -i

[Stefan Beller]

On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin
 wrote:
> This patch series reimplements the expensive pre- and post-processing of
> the todo script in C.
>
> And it concludes the work I did to accelerate rebase -i so far.
>
> I am still unwilling to replace a compile-time safe way to pass the
> options to the revision machinery by the alternative (which I am still
> flabbergasted about) proposed by Junio. This will not change.

There is new input for this discussion, though. :)

https://public-inbox.org/git/20170706202739.6056-1-sbel...@google.com/
was sent out to gauge interest if we want to pull through with removing
all the submodule hack, such as 'add_submodule_odb' in submodule.c
which just adds the submodule objects as an alternate object store, such
that we can do some things in-process (check if a submodule has certain
commits, merge_submodule).

For one of these ('merge_submodule') I would have to add the
'struct repository' as another parameter to pass around to the
diff and revision walking machinery. But the API for these subsystems
is traditionally only operated via an array of strings instead of passing
assigning a member field to a value.

So If I were to follow the "use arrays of strings only to operate the
revision machinery" I would:
* pass a string indicating which repo to use
  (probably the path to git dir?)
* the repository objects are cached, so we can lookup e.g.
  "the_repository" via the correct string.
* use that repo object inside the revision machinery.

That sounds cumbersome and I would prefer to pass in
the repository object directory. So maybe we want to have some
other way opposed to "an array of strings" to operate the revision
machinery.

>  -static int subject2item_cmp(const struct subject2item_entry *a,
>  -  const struct subject2item_entry *b, const void *key)
>  +static int subject2item_cmp(const void *fndata,

This could also be named unused_fndata.
Please see origin/sb/hashmap-cleanup, if that makes sense as well
(have all arguments const void and cast them inside the function, such
that we can avoid the cast to hashmap_cmp_fn in hashmap_init)

Thanks,
Stefan

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[astian]

FWIW, I don't like 2, I don't like the irregularity in the invocation:

  $ git branch
  * master
  $ git BRANCH
  git: 'BRANCH' is not a git command. See 'git --help'.
  $ git config alias.br 'branch -v'
  $ git br
  * master 51c785c initial
  $ git BR
  * master 51c785c initial

There is also this:

  $ git branch
  * master
  $ git BRANCH
  git: 'BRANCH' is not a git command. See 'git --help'.
  $ git config alias.branch 'branch -v'
  $ git branch
  * master
  $ git BRANCH
  * master 51c785c initial

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Stefan Beller]

On Fri, Jul 14, 2017 at 9:11 AM, Junio C Hamano  wrote:

>  - I prefer to keep decl-after-statement out of our codebase.  I
>view it as a big plus in code-readability to be able to see a
>complete list of variables that will be used in a block upfront
>before starting to read the code that uses them.

sounds good to me.

>  - Corollary to the above, I do not mind to have a variable
>declaration in the initialization clause of a for() statement
>(e.g. "for (int i = 0; i < 4; i++) { ... }"), as the scoping rule
>is very sensible.  Some of our "for()" statements use the value
>of the variable after iteration, for which this new construct
>cannot be used, though.

I might send a test balloon for this.

>. // comments

I would think these are useful for two reasons:
(a) these seem to be used widely outside of these old-style project
  (git, gcc, kernel), such that most new contributors need to be told
  to avoid these which adds to the entry burden.
(b) recursive nesting of comments is possible. We may not need this
  in the final patch, but I sure use that in development. To comment out
  a whole function containing multiple comments I have to select all
  lines and prefix them with // currently. If all comments were //, I could
  put /* .. */ around the function, which seems easier. I just realize
  I can use #if 0 .. #endif though.

  (a) may be a concern, (b) not so much from the projects point of view.

>. restricted pointers

That sounds like the ultimate micro optimization, but may be hard
to reason about after a years of churn, so it may become hard to
maintain.

>. static and type qualifiers in parameter array declarators

That sounds equally arcane.

> -- >8 --
> Subject: [PATCH] clean.c: use designated initializer
>
> This is another test balloon to see if we get complaints from people
> whose compilers do not support designated initializer for arrays.

This sounds as if it is applied on top of Jeffs test balloon patch, such
that we do not need to re-iterate the criteria why to do it here, e.g.
this code is always compiled and did not change a lot over the last
couple month, so potentially easy to revert.

Thanks,
Stefan

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Junio C Hamano]

Jeff King  writes:

> I think there are basically two mental models that are reasonable:
>
>   1. Uppercase in key names is treated the same as lowercase. Therefore
>  we must allow "alias.Foo" to match "git foo", but "git Foo" can
>  never have a match (in the current schema).
>
>   2. Keys are case-insensitive, and anything that matches them is
>  considered case-insensitive, too. That means "Foo" and "foo" are
>  identical for these purposes, and you can never have two aliases
>  "Foo" and "foo".
>
> In either mental model, "alias.Foo" for "git foo" must work. But the
> reverse only works in (2).
>
> I think either model is fine. These patches push us into (2).

I've always thought that the promise we give our end users is that
these keys are case insensitive, and that the fact that we downcase
the key before calling config callback is merely an implementation
detail.  That is why I never considered the possibility that (1) can
be a valid mental model.

There are other possible implementations of case insensitivity.  We
could have been upcasing instead before calling config callback and
the users wouldn't have noticed.

So I'd consider that pushing us into (2) is a good thing.

Thanks.

[PATCH] l10n: de.po: update German translation

[Ralf Thielow]

Signed-off-by: Ralf Thielow 
---
 po/de.po | 123 +--
 1 file changed, 64 insertions(+), 59 deletions(-)

diff --git a/po/de.po b/po/de.po
index b9e076f93..f4cb9bf2d 100644
--- a/po/de.po
+++ b/po/de.po
@@ -176,26 +176,26 @@ msgid "git apply: bad git-diff - inconsistent old 
filename on line %d"
 msgstr ""
 "git apply: ungültiges 'git-diff' - Inkonsistenter alter Dateiname in Zeile %d"
 
 #: apply.c:979
 #, c-format
 msgid "git apply: bad git-diff - expected /dev/null on line %d"
 msgstr "git apply: ungültiges 'git-diff' - erwartete /dev/null in Zeile %d"
 
 #: apply.c:1008
-#, fuzzy, c-format
+#, c-format
 msgid "invalid mode on line %d: %s"
-msgstr "Ungültige Identifikationszeile: %s"
+msgstr "Ungültiger Modus in Zeile %d: %s"
 
 #: apply.c:1326
 #, c-format
 msgid "inconsistent header lines %d and %d"
-msgstr ""
+msgstr "Inkonsistente Kopfzeilen %d und %d."
 
 #: apply.c:1498
 #, c-format
 msgid "recount: unexpected line: %.*s"
 msgstr "recount: unerwartete Zeile: %.*s"
 
 #: apply.c:1567
 #, c-format
 msgid "patch fragment without header at line %d: %.*s"
@@ -1528,27 +1528,27 @@ msgstr "LF würde in %s durch CRLF ersetzt werden."
 
 #: date.c:116
 msgid "in the future"
 msgstr "in der Zukunft"
 
 #: date.c:122 date.c:129 date.c:136 date.c:143 date.c:149 date.c:156 date.c:167
 #: date.c:175 date.c:180
 msgid "%"
 msgid_plural "%"
-msgstr[0] ""
-msgstr[1] ""
+msgstr[0] "%"
+msgstr[1] "%"
 
 #. TRANSLATORS: "%s" is " years"
 #: date.c:170
 msgid "%s, %"
 msgid_plural "%s, %"
-msgstr[0] ""
-msgstr[1] ""
+msgstr[0] "%s, %"
+msgstr[1] "%s, %"
 
 #: diffcore-order.c:24
 #, c-format
 msgid "failed to read orderfile '%s'"
 msgstr "Fehler beim Lesen der Reihenfolgedatei '%s'."
 
 #: diffcore-rename.c:536
 msgid "Performing inexact rename detection"
 msgstr "Führe Erkennung für ungenaue Umbenennung aus"
@@ -1890,53 +1890,50 @@ msgid ""
 msgstr ""
 "'%s' scheint ein git-Befehl zu sein, konnte aber\n"
 "nicht ausgeführt werden. Vielleicht ist git-%s fehlerhaft?"
 
 #: help.c:336
 msgid "Uh oh. Your system reports no Git commands at all."
 msgstr "Uh oh. Keine Git-Befehle auf Ihrem System vorhanden."
 
 #: help.c:358
-#, fuzzy, c-format
+#, c-format
 msgid "WARNING: You called a Git command named '%s', which does not exist."
-msgstr ""
-"Warnung: Sie haben den nicht existierenden Git-Befehl '%s' ausgeführt.\n"
-"Setze fort unter der Annahme, dass Sie '%s' gemeint haben."
+msgstr "WARNUNG: Sie haben Git-Befehl '%s' ausgeführt, welcher nicht 
existiert."
 
 #: help.c:363
 #, c-format
 msgid "Continuing under the assumption that you meant '%s'."
-msgstr ""
+msgstr "Setze fort unter der Annahme, dass Sie '%s' meinten."
 
 #: help.c:368
 #, c-format
 msgid "Continuing in %0.1f seconds, assuming that you meant '%s'."
-msgstr ""
+msgstr "Setze in %0.1f Sekunden fort unter der Annahme, dass Sie '%s' meinten."
 
 #: help.c:376
 #, c-format
 msgid "git: '%s' is not a git command. See 'git --help'."
 msgstr "git: '%s' ist kein Git-Befehl. Siehe 'git --help'."
 
 #: help.c:380
 msgid ""
 "\n"
 "The most similar command is"
 msgid_plural ""
 "\n"
 "The most similar commands are"
-msgstr[0] ""
-msgstr[1] ""
+msgstr[0] "\nDer ähnlichste Befehl ist"
+msgstr[1] "\nDie ähnlichsten Befehle sind"
 
 #: help.c:395
-#, fuzzy
 msgid "git version []"
-msgstr "git column []"
+msgstr "git version []"
 
 #: help.c:456
 #, c-format
 msgid "%s: %s - %s"
 msgstr "%s: %s - %s"
 
 #: help.c:460
 msgid ""
 "\n"
@@ -3361,21 +3358,21 @@ msgstr ""
 "Ausführung erfolgreich: %s\n"
 "Aber Änderungen in Index oder Arbeitsverzeichnis verblieben.\n"
 "Committen Sie Ihre Änderungen oder benutzen Sie \"stash\".\n"
 "Führen Sie dann aus:\n"
 "\n"
 "  git rebase --continue\n"
 "\n"
 
 #: sequencer.c:1925
-#, fuzzy, c-format
+#, c-format
 msgid "Applied autostash.\n"
-msgstr "Automatischen Stash angewendet."
+msgstr "Automatischen Stash angewendet.\n"
 
 #: sequencer.c:1937
 #, c-format
 msgid "cannot store %s"
 msgstr "kann %s nicht speichern"
 
 #: sequencer.c:1940 git-rebase.sh:173
 #, c-format
 msgid ""
@@ -3649,21 +3646,21 @@ msgstr "Konnte Eintrag '%s' nicht aus .gitmodules 
entfernen"
 #: submodule.c:126
 msgid "staging updated .gitmodules failed"
 msgstr "Konnte aktualisierte .gitmodules-Datei nicht zum Commit vormerken"
 
 #: submodule.c:165
 msgid "negative values not allowed for submodule.fetchJobs"
 msgstr "Negative Werte für submodule.fetchJobs nicht erlaubt"
 
 #: submodule.c:376
-#, fuzzy, c-format
+#, c-format
 msgid "in unpopulated submodule '%s'"
-msgstr "Überspringe Submodul '%s'"
+msgstr "In nicht ausgechecktem Submodul '%s'."
 
 #: submodule.c:407
 #, c-format
 msgid "Pathspec '%s' is in submodule '%.*s'"
 msgstr "Pfadspezifikation '%s' befindet sich in Submodul '%.*s'"
 
 #: submodule.c:1337
 #, c-format
 msgid "'%s' not recognized as a git repository"
@@ -4330,23 +4327,23 @@ msgstr "neue Commits, "
 #: wt-status.c:374
 msgid "modified content, "
 msgstr 

Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT

[Junio C Hamano]

Junio C Hamano  writes:

> Oh, absolutely.
>
> Here is another possible test balloon, that may actually be useful
> as an example.  I think there is a topic in flight that touches this
> array, unfortunately, so I probably would find another one that is
> more stable for a real follow-up patch to the one from Peff.

And here it is.

As to other things that we currently not allow in our codebase that
newer compilers can grok, here is what *I* think.  It is *not* meant
to be an exhaustive "what's new in C99 that is not in C89? what is
the final verdict on each of them?":

 - There were occasional cases where we wished if variable-length
   arrays, flexible array members and variadic macros were available
   in our codebase during the course of this project.  We would
   probably want to add a similar test baloon patch for each of
   them to this series that is currently two-patch long.

 - I prefer to keep decl-after-statement out of our codebase.  I
   view it as a big plus in code-readability to be able to see a
   complete list of variables that will be used in a block upfront
   before starting to read the code that uses them.

 - Corollary to the above, I do not mind to have a variable
   declaration in the initialization clause of a for() statement
   (e.g. "for (int i = 0; i < 4; i++) { ... }"), as the scoping rule
   is very sensible.  Some of our "for()" statements use the value
   of the variable after iteration, for which this new construct
   cannot be used, though.

 - This may be showing I am not just old fashioned but also am
   ignorant, but I do not see much point in using the following in
   our codebase (iow, I am not aware of places in the existing code
   that they can be improved by employing these features):

   . // comments
   . restricted pointers
   . static and type qualifiers in parameter array declarators



-- >8 --
Subject: [PATCH] clean.c: use designated initializer

This is another test balloon to see if we get complaints from people
whose compilers do not support designated initializer for arrays.

The use of the feature is not all that interesting for cases like
the one this patch touches, where the initialized elements of the
array is dense, but it would be nice if we can use the feature to
initialize an array that has elements initialized to interesting
values only sparsely.

Signed-off-by: Junio C Hamano 
---
 builtin/clean.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 057fc97fe4..858df2c4ee 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -33,15 +33,6 @@ static const char *msg_skip_git_dir = N_("Skipping 
repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
 
-static int clean_use_color = -1;
-static char clean_colors[][COLOR_MAXLEN] = {
-   GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_BOLD_BLUE,/* PROMPT */
-   GIT_COLOR_BOLD, /* HEADER */
-   GIT_COLOR_BOLD_RED, /* HELP */
-   GIT_COLOR_BOLD_RED, /* ERROR */
-};
 enum color_clean {
CLEAN_COLOR_RESET = 0,
CLEAN_COLOR_PLAIN = 1,
@@ -51,6 +42,16 @@ enum color_clean {
CLEAN_COLOR_ERROR = 5
 };
 
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+   [CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
+   [CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
+   [CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
+   [CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
+   [CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
+   [CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
+};
+
 #define MENU_OPTS_SINGLETON01
 #define MENU_OPTS_IMMEDIATE02
 #define MENU_OPTS_LIST_ONLY04
-- 
2.14.0-rc0-148-g5448d1895b

Re: Reducing redundant build at Travis?

[Jeff King]

On Fri, Jul 14, 2017 at 07:54:16AM -0700, Junio C Hamano wrote:

> > The "git test" script[1] uses this strategy with git-notes as the
> > storage, and I've found it quite useful. I don't think we can rely on
> > git-notes, but I think Travis gives us some storage options. Even just a
> > best-effort cache directory would probably be sufficient (this is an
> > optimization, after all).
> 
> We do seem to use some persistence to order prove tests already, but
> I do not think it helps the common case, where my end-of-day push
> pushes out 'maint' and 'v2.13.3' at the same time, because the push
> is made with "git push --follow-tags $there maint master next pu"
> and the new tag happens to be at 'maint'.  It would be nice if
> Travis runs were sequential, but I often observe that it creates
> jobs for these multiple branches and tags pushed at the same time,
> and start running a few of them.

Ah, right, I didn't think about how these are racing. You'd need storage
which allows some kind of atomic operation to "claim" the tree as a
work-in-progress (and anybody who loses the race to get the lock would
have to spin waiting for the winner to tell them the real status).

I don't know if Travis's cache storage is up to that challenge. We could
probably build such a lock on top of third-party storage, but things are
rapidly getting more complex.

-Peff

Re: reftable: new ref storage format

[Shawn Pearce]

On Fri, Jul 14, 2017 at 7:27 AM, Dave Borowitz  wrote:
> On Thu, Jul 13, 2017 at 8:11 PM, Shawn Pearce  wrote:
>> In another (Gerrit Code Review), we disable reflogs for
>> the insane refs/changes/ namespace, as nearly every reference is
>> created once, and never modified.
>
> Apologies for the tangent, but this is not true in the most recent
> Gerrit implementation. We update refs/changes/CD/ABCD/1 and
> refs/changes/CD/ABCD/meta in a single BatchRefUpdate, and we set a
> reflog message on the BatchRefUpdate instance, which updates the
> reflog for all refs in the batch. The reflog message on /meta is
> important, and arguably it's useful to be able to correlate that with
> the reflog on /1.
>
> If you think storing reflogs on patch set refs is going to be a
> problem wrt on-disk storage, we should discuss this offline :)

Reflog storage is a problem for Gerrit. It was a problem in early 2009
when servers had a lot less changes. Its going to be even more of a
problem now. Sounds like we have to support reflogs in reftable, or
something like it.

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Jeff King]

On Fri, Jul 14, 2017 at 08:14:18AM -0700, Junio C Hamano wrote:

> >> It was possible before v2.13.3 to invoke:
> >> 
> >>git config alias.CamelCased 
> >>git CamelCased
> >> 
> >> This regressed (due to a stupid mistake of mine that was not caught in
> >> patch review, sadly) in v2.13.3.
> >
> > Interesting. I don't think this was ever intended to work.
> > ...
> > The patches look obviously correct.
> 
> How can something be "(n)ever intended to work" and yet patches to
> make it work be "obviously correct"? ;-)

You snipped the part where I said "this is probably reasonable to do in
the meantime." :)

My "obviously correct" is only that the patches fulfill that.

> But I think that it is still reasonable for an end user to expect
> that 'git Foo' would trigger that alias.  And that is what was
> recently changed, inadvertently.
> 
> So the problem may need to be explained better in this series, but I
> think the usage was expected to work and the series is fixing a real
> regression.

Sort of. It did not work until it accidentally did work, and now it
accidentally does not work again. So "usage was expected to work" was
certainly not true for Git developers (or at least nobody intentionally
wrote code to make it so). And anybody relying on it started doing so
since v2.2.0.

But like I said, it's probably reasonable to make it work. There's
little harm in doing so.  The only downside I can see is that doing:

  git config alias.foo 
  git Foo

now triggers the alias. That seems like at worst a minor bug, and
possibly even the right thing to do (see below).

> Do we want to promise to keep the following "working"?
> 
> git config alias.Foo 
> git foo
> 
> By designing the system in such a way that an alias is created with
> a two-level name in our system, we are saying that alias names are
> case insensitive to the end users, so I _think_ the above is
> intended to work, and we are effectively promising that it will keep
> working.

Yes, I think we must. Keys are case-insensitive, and you are allowed to
write them in whatever case you like. The more interesting case is the
reverse, that I showed above. I think there are basically two mental
models that are reasonable:

  1. Uppercase in key names is treated the same as lowercase. Therefore
 we must allow "alias.Foo" to match "git foo", but "git Foo" can
 never have a match (in the current schema).

  2. Keys are case-insensitive, and anything that matches them is
 considered case-insensitive, too. That means "Foo" and "foo" are
 identical for these purposes, and you can never have two aliases
 "Foo" and "foo".

In either mental model, "alias.Foo" for "git foo" must work. But the
reverse only works in (2).

I think either model is fine. These patches push us into (2).

-Peff

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Junio C Hamano]

Santiago Torres  writes:

>> Combined with the unknown-ness of the root cause of the issue, I can
>> only say that the patch may be raising an issue worth addressing,
>> but it is too sketchy to tell if it is a right solution or what the
>> exact problem being solved is.
>
> I'll dig into this. This sounds a way more reasonable approach.

Thanks.  Another thing that may help, if it turns out that we do
want to let agent run when it wants to, may be to study and mimick
the way our webserver tests arrange how the servers are killed at
the end of the test.

Re: Regression in v2.13.3, fixed in master: aliases in worktrees

[Junio C Hamano]

Johannes Schindelin  writes:

> The bug is fixed in master already, and I cherry-picked Brandon's config-h
> series that contains the fix into Git for Windows' master branch.

As you can see in Documentation/RelNotes/2.14.0.txt, I actually have
been debating myself if that series should be merged down to 'maint'
as a general bugfix around multi-worktree area for about 3 weeks.

Re: [PATCH v2 1/1] sha1_file: Add support for downloading blobs on demand

[Christian Couder]

On Fri, Jul 14, 2017 at 3:26 PM, Ben Peart  wrote:

> diff --git a/contrib/long-running-read-object/example.pl 
> b/contrib/long-running-read-object/example.pl
> new file mode 100755
> index 00..b8f37f836a
> --- /dev/null
> +++ b/contrib/long-running-read-object/example.pl

[...]

> +sub packet_bin_read {
> +   my $buffer;
> +   my $bytes_read = read STDIN, $buffer, 4;
> +   if ( $bytes_read == 0 ) {
> +
> +   # EOF - Git stopped talking to us!
> +   exit();
> +   }
> +   elsif ( $bytes_read != 4 ) {
> +   die "invalid packet: '$buffer'";
> +   }
> +   my $pkt_size = hex($buffer);
> +   if ( $pkt_size == 0 ) {
> +   return ( 1, "" );
> +   }
> +   elsif ( $pkt_size > 4 ) {
> +   my $content_size = $pkt_size - 4;
> +   $bytes_read = read STDIN, $buffer, $content_size;
> +   if ( $bytes_read != $content_size ) {
> +   die "invalid packet ($content_size bytes expected; 
> $bytes_read bytes read)";
> +   }
> +   return ( 0, $buffer );
> +   }
> +   else {
> +   die "invalid packet size: $pkt_size";
> +   }
> +}
> +
> +sub packet_txt_read {
> +   my ( $res, $buf ) = packet_bin_read();
> +   unless ( $buf =~ s/\n$// ) {
> +   die "A non-binary line MUST be terminated by an LF.";
> +   }
> +   return ( $res, $buf );
> +}
> +
> +sub packet_bin_write {
> +   my $buf = shift;
> +   print STDOUT sprintf( "%04x", length($buf) + 4 );
> +   print STDOUT $buf;
> +   STDOUT->flush();
> +}
> +
> +sub packet_txt_write {
> +   packet_bin_write( $_[0] . "\n" );
> +}
> +
> +sub packet_flush {
> +   print STDOUT sprintf( "%04x", 0 );
> +   STDOUT->flush();
> +}

The above could reuse the refactoring of t0021/rot13-filter.pl into
perl/Git/Packet.pm that is at the beginning of my patch series.

> diff --git a/t/t0410/read-object b/t/t0410/read-object
> new file mode 100755
> index 00..85e997c930
> --- /dev/null
> +++ b/t/t0410/read-object

[...]

> +sub packet_bin_read {
> +   my $buffer;
> +   my $bytes_read = read STDIN, $buffer, 4;
> +   if ( $bytes_read == 0 ) {
> +
> +   # EOF - Git stopped talking to us!
> +   exit();
> +   }
> +   elsif ( $bytes_read != 4 ) {
> +   die "invalid packet: '$buffer'";
> +   }
> +   my $pkt_size = hex($buffer);
> +   if ( $pkt_size == 0 ) {
> +   return ( 1, "" );
> +   }
> +   elsif ( $pkt_size > 4 ) {
> +   my $content_size = $pkt_size - 4;
> +   $bytes_read = read STDIN, $buffer, $content_size;
> +   if ( $bytes_read != $content_size ) {
> +   die "invalid packet ($content_size bytes expected; 
> $bytes_read bytes read)";
> +   }
> +   return ( 0, $buffer );
> +   }
> +   else {
> +   die "invalid packet size: $pkt_size";
> +   }
> +}
> +
> +sub packet_txt_read {
> +   my ( $res, $buf ) = packet_bin_read();
> +   unless ( $buf =~ s/\n$// ) {
> +   die "A non-binary line MUST be terminated by an LF.";
> +   }
> +   return ( $res, $buf );
> +}
> +
> +sub packet_bin_write {
> +   my $buf = shift;
> +   print STDOUT sprintf( "%04x", length($buf) + 4 );
> +   print STDOUT $buf;
> +   STDOUT->flush();
> +}
> +
> +sub packet_txt_write {
> +   packet_bin_write( $_[0] . "\n" );
> +}
> +
> +sub packet_flush {
> +   print STDOUT sprintf( "%04x", 0 );
> +   STDOUT->flush();
> +}

Same thing about the above and perl/Git/Packet.pm.

Otherwise thanks for updating this!

Re: Git Bash Bug

[Paul Smith]

On Fri, 2017-07-14 at 09:59 -0500, Kavita Desai wrote:
> What does "echo $PATH" show?
> /c/Users/Kavita/

Well, there you go.  That's clearly wrong.

You absolutely have to have /bin and /usr/bin on your PATH, _at least_
if you want to be able to run standard UNIX tools.  And most likely
you'll have a ton of other directories on your PATH as well.

I would investigate the shell configuration files etc. and see where
you've messed up resetting your PATH variable.

> What does "type -a ls" show?
> ls is aliased to `ls -F --color=auto --show-control-chars'

Yep.  There is no ls binary found.

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Junio C Hamano]

Jeff King  writes:

> On Fri, Jul 14, 2017 at 10:39:24AM +0200, Johannes Schindelin wrote:
>
>> It was possible before v2.13.3 to invoke:
>> 
>>  git config alias.CamelCased 
>>  git CamelCased
>> 
>> This regressed (due to a stupid mistake of mine that was not caught in
>> patch review, sadly) in v2.13.3.
>
> Interesting. I don't think this was ever intended to work.
> ...
> The patches look obviously correct.

How can something be "(n)ever intended to work" and yet patches to
make it work be "obviously correct"? ;-)

My first/knee-jerk reation to the title of the series also was
"letter cases are not supposed to work in aliases", but that depends
on the definition of "work".  When you add 'alias.Foo', you are not
supposed to be able to make 'git foo' behave differently from that
alias you defined.  In order to make that, which is not supposed to
work, work, we'd need to introduce alias.Foo.commmand, as you said.

But I think that it is still reasonable for an end user to expect
that 'git Foo' would trigger that alias.  And that is what was
recently changed, inadvertently.

So the problem may need to be explained better in this series, but I
think the usage was expected to work and the series is fixing a real
regression.

Do we want to promise to keep the following "working"?

git config alias.Foo 
git foo

By designing the system in such a way that an alias is created with
a two-level name in our system, we are saying that alias names are
case insensitive to the end users, so I _think_ the above is
intended to work, and we are effectively promising that it will keep
working.

It is a different matter if that design decision was sensible,
though.

> As a meta-comment, I find splitting the tests from the fix like this
> makes review more tedious.

I agree.

Re: Git Bash Bug

[Kavita Desai]

Here are the results.

What does "echo $PATH" show?
/c/Users/Kavita/

What does "type -a ls" show?
ls is aliased to `ls -F --color=auto --show-control-chars'



On Fri, Jul 14, 2017 at 9:37 AM, Paul Smith  wrote:
> On Fri, 2017-07-14 at 09:34 -0500, Kavita Desai wrote:
>> Sorry for not being specific. What I meant by not working was that the
>> bash commands are not found.
>> Here is an example
>>
>> $ ls
>> bash: ls: command not found
>
> The most obvious issue is your PATH is wrong.
>
> What does "echo $PATH" show?
>
> What does "type -a ls" show?



-- 
Kavita Desai
UIUC Engineering Physics 2018
Engineering Outreach Society Engineering Open House Chair
UIUC Housing Ike. South Front Desk Clerk

Re: Reducing redundant build at Travis?

[Junio C Hamano]

Jeff King  writes:

> Right, I think the right solution is some amount of peeling. Recognizing
> that the commit sha1 is the same, or better yet, not bothering to retest
> trees which have already been tested.

Yup, I also have a hack to avoid testing a version that is only
different in insignificant way (e.g. the only difference being
GIT-VERSION-GEN or Documentation/RelNotes/*) from an installed one
in the script I use after each integration attempt I do, which I use
a few times a day (that's "Meta/Dothem" if anybody is interested).

> If we had some kind of persistent storage, we could do a quick:
> ...
> The "git test" script[1] uses this strategy with git-notes as the
> storage, and I've found it quite useful. I don't think we can rely on
> git-notes, but I think Travis gives us some storage options. Even just a
> best-effort cache directory would probably be sufficient (this is an
> optimization, after all).

We do seem to use some persistence to order prove tests already, but
I do not think it helps the common case, where my end-of-day push
pushes out 'maint' and 'v2.13.3' at the same time, because the push
is made with "git push --follow-tags $there maint master next pu"
and the new tag happens to be at 'maint'.  It would be nice if
Travis runs were sequential, but I often observe that it creates
jobs for these multiple branches and tags pushed at the same time,
and start running a few of them.

[PATCH v6 08/10] rebase -i: skip unnecessary picks using the rebase--helper

[Johannes Schindelin]

In particular on Windows, where shell scripts are even more expensive
than on MacOSX or Linux, it makes sense to move a loop that forks
Git at least once for every line in the todo list into a builtin.

Note: The original code did not try to skip unnecessary picks of root
commits but punts instead (probably --root was not considered common
enough of a use case to bother optimizing). We do the same, for now.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c   |   6 ++-
 git-rebase--interactive.sh |  41 ++---
 sequencer.c| 107 +
 sequencer.h|   1 +
 4 files changed, 116 insertions(+), 39 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 305adb5b707..057ae7102ff 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -15,7 +15,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
int keep_empty = 0;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
-   CHECK_TODO_LIST
+   CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -32,6 +32,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
OPT_CMDMODE(0, "check-todo-list", ,
N_("check the todo list"), CHECK_TODO_LIST),
+   OPT_CMDMODE(0, "skip-unnecessary-picks", ,
+   N_("skip unnecessary picks"), SKIP_UNNECESSARY_PICKS),
OPT_END()
};
 
@@ -56,5 +58,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!transform_todo_ids(0);
if (command == CHECK_TODO_LIST && argc == 1)
return !!check_todo_list();
+   if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
+   return !!skip_unnecessary_picks();
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c8cad318fa4..af8d7bd77fb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -713,43 +713,6 @@ do_rest () {
done
 }
 
-# skip picking commits whose parents are unchanged
-skip_unnecessary_picks () {
-   fd=3
-   while read -r command rest
-   do
-   # fd=3 means we skip the command
-   case "$fd,$command" in
-   3,pick|3,p)
-   # pick a commit whose parent is current $onto -> skip
-   sha1=${rest%% *}
-   case "$(git rev-parse --verify --quiet "$sha1"^)" in
-   "$onto"*)
-   onto=$sha1
-   ;;
-   *)
-   fd=1
-   ;;
-   esac
-   ;;
-   3,"$comment_char"*|3,)
-   # copy comments
-   ;;
-   *)
-   fd=1
-   ;;
-   esac
-   printf '%s\n' "$command${rest:+ }$rest" >&$fd
-   done <"$todo" >"$todo.new" 3>>"$done" &&
-   mv -f "$todo".new "$todo" &&
-   case "$(peek_next_command)" in
-   squash|s|fixup|f)
-   record_in_rewritten "$onto"
-   ;;
-   esac ||
-   die "$(gettext "Could not skip unnecessary pick commands")"
-}
-
 expand_todo_ids() {
git rebase--helper --expand-ids
 }
@@ -1149,7 +1112,9 @@ git rebase--helper --check-todo-list || {
 
 expand_todo_ids
 
-test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
+test -d "$rewritten" || test -n "$force_rebase" ||
+onto="$(git rebase--helper --skip-unnecessary-picks)" ||
+die "Could not skip unnecessary pick commands"
 
 checkout_onto
 if test -z "$rebase_root" && test ! -d "$rewritten"
diff --git a/sequencer.c b/sequencer.c
index 15107de1e1c..96d43aec764 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2641,3 +2641,110 @@ int check_todo_list(void)
 
return res;
 }
+
+/* skip picking commits whose parents are unchanged */
+int skip_unnecessary_picks(void)
+{
+   const char *todo_file = rebase_path_todo();
+   struct strbuf buf = STRBUF_INIT;
+   struct todo_list todo_list = TODO_LIST_INIT;
+   struct object_id onto_oid, *oid = _oid, *parent_oid;
+   int fd, i;
+
+   if (!read_oneliner(, rebase_path_onto(), 0))
+   return error(_("could not read 'onto'"));
+   if (get_oid(buf.buf, _oid)) {
+   strbuf_release();
+   return error(_("need a HEAD to fixup"));
+   }
+   strbuf_release();
+
+   fd = 

[PATCH v6 09/10] t3415: test fixup with wrapped oneline

[Johannes Schindelin]

The `git commit --fixup` command unwraps wrapped onelines when
constructing the commit message, without wrapping the result.

We need to make sure that `git rebase --autosquash` keeps handling such
cases correctly, in particular since we are about to move the autosquash
handling into the rebase--helper.

Signed-off-by: Johannes Schindelin 
---
 t/t3415-rebase-autosquash.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 6d99f624b62..62cb977e4ec 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -316,4 +316,18 @@ test_expect_success 'extra spaces after fixup!' '
test $base = $parent
 '
 
+test_expect_success 'wrapped original subject' '
+   if test -d .git/rebase-merge; then git rebase --abort; fi &&
+   base=$(git rev-parse HEAD) &&
+   echo "wrapped subject" >wrapped &&
+   git add wrapped &&
+   test_tick &&
+   git commit --allow-empty -m "$(printf "To\nfixup")" &&
+   test_tick &&
+   git commit --allow-empty -m "fixup! To fixup" &&
+   git rebase -i --autosquash --keep-empty HEAD~2 &&
+   parent=$(git rev-parse HEAD^) &&
+   test $base = $parent
+'
+
 test_done
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH v6 10/10] rebase -i: rearrange fixup/squash lines using the rebase--helper

[Johannes Schindelin]

This operation has quadratic complexity, which is especially painful
on Windows, where shell scripts are *already* slow (mainly due to the
overhead of the POSIX emulation layer).

Let's reimplement this with linear complexity (using a hash map to
match the commits' subject lines) for the common case; Sadly, the
fixup/squash feature's design neglected performance considerations,
allowing arbitrary prefixes (read: `fixup! hell` will match the
commit subject `hello world`), which means that we are stuck with
quadratic performance in the worst case.

The reimplemented logic also happens to fix a bug where commented-out
lines (representing empty patches) were dropped by the previous code.

While at it, clarify how the fixup/squash feature works in `git rebase
-i`'s man page.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt |  16 ++--
 builtin/rebase--helper.c |   6 +-
 git-rebase--interactive.sh   |  90 +---
 sequencer.c  | 196 +++
 sequencer.h  |   1 +
 t/t3415-rebase-autosquash.sh |   2 +-
 6 files changed, 213 insertions(+), 98 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 4f6bed61a92..a3c01dfdc8a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -430,13 +430,15 @@ without an explicit `--interactive`.
 --autosquash::
 --no-autosquash::
When the commit log message begins with "squash! ..." (or
-   "fixup! ..."), and there is a commit whose title begins with
-   the same ..., automatically modify the todo list of rebase -i
-   so that the commit marked for squashing comes right after the
-   commit to be modified, and change the action of the moved
-   commit from `pick` to `squash` (or `fixup`).  Ignores subsequent
-   "fixup! " or "squash! " after the first, in case you referred to an
-   earlier fixup/squash with `git commit --fixup/--squash`.
+   "fixup! ..."), and there is already a commit in the todo list that
+   matches the same `...`, automatically modify the todo list of rebase
+   -i so that the commit marked for squashing comes right after the
+   commit to be modified, and change the action of the moved commit
+   from `pick` to `squash` (or `fixup`).  A commit matches the `...` if
+   the commit subject matches, or if the `...` refers to the commit's
+   hash. As a fall-back, partial matches of the commit subject work,
+   too.  The recommended way to create fixup/squash commits is by using
+   the `--fixup`/`--squash` options of linkgit:git-commit[1].
 +
 This option is only valid when the `--interactive` option is used.
 +
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 057ae7102ff..f8519363a39 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -15,7 +15,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
int keep_empty = 0;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
-   CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS
+   CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -34,6 +34,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("check the todo list"), CHECK_TODO_LIST),
OPT_CMDMODE(0, "skip-unnecessary-picks", ,
N_("skip unnecessary picks"), SKIP_UNNECESSARY_PICKS),
+   OPT_CMDMODE(0, "rearrange-squash", ,
+   N_("rearrange fixup/squash lines"), REARRANGE_SQUASH),
OPT_END()
};
 
@@ -60,5 +62,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!check_todo_list();
if (command == SKIP_UNNECESSARY_PICKS && argc == 1)
return !!skip_unnecessary_picks();
+   if (command == REARRANGE_SQUASH && argc == 1)
+   return !!rearrange_squash();
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index af8d7bd77fb..3b0340e7cc9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -721,94 +721,6 @@ collapse_todo_ids() {
git rebase--helper --shorten-ids
 }
 
-# Rearrange the todo list that has both "pick sha1 msg" and
-# "pick sha1 fixup!/squash! msg" appears in it so that the latter
-# comes immediately after the former, and change "pick" to
-# "fixup"/"squash".
-#
-# Note that if the config has specified a custom instruction format
-# each log message will be re-retrieved in order to normalize the
-# autosquash arrangement
-rearrange_squash () {
-   format=$(git config --get 

[PATCH v6 07/10] rebase -i: check for missing commits in the rebase--helper

[Johannes Schindelin]

In particular on Windows, where shell scripts are even more expensive
than on MacOSX or Linux, it makes sense to move a loop that forks
Git at least once for every line in the todo list into a builtin.

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c   |   7 +-
 git-rebase--interactive.sh | 164 ++---
 sequencer.c| 122 +
 sequencer.h|   1 +
 4 files changed, 134 insertions(+), 160 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 791b62901c3..305adb5b707 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -14,7 +14,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct replay_opts opts = REPLAY_OPTS_INIT;
int keep_empty = 0;
enum {
-   CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S
+   CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,
+   CHECK_TODO_LIST
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -29,6 +30,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
OPT_CMDMODE(0, "expand-ids", ,
N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
+   OPT_CMDMODE(0, "check-todo-list", ,
+   N_("check the todo list"), CHECK_TODO_LIST),
OPT_END()
};
 
@@ -51,5 +54,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!transform_todo_ids(1);
if (command == EXPAND_SHA1S && argc == 1)
return !!transform_todo_ids(0);
+   if (command == CHECK_TODO_LIST && argc == 1)
+   return !!check_todo_list();
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d5df02435ae..c8cad318fa4 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -867,96 +867,6 @@ add_exec_commands () {
mv "$1.new" "$1"
 }
 
-# Check if the SHA-1 passed as an argument is a
-# correct one, if not then print $2 in "$todo".badsha
-# $1: the SHA-1 to test
-# $2: the line number of the input
-# $3: the input filename
-check_commit_sha () {
-   badsha=0
-   if test -z "$1"
-   then
-   badsha=1
-   else
-   sha1_verif="$(git rev-parse --verify --quiet $1^{commit})"
-   if test -z "$sha1_verif"
-   then
-   badsha=1
-   fi
-   fi
-
-   if test $badsha -ne 0
-   then
-   line="$(sed -n -e "${2}p" "$3")"
-   warn "$(eval_gettext "\
-Warning: the SHA-1 is missing or isn't a commit in the following line:
- - \$line")"
-   warn
-   fi
-
-   return $badsha
-}
-
-# prints the bad commits and bad commands
-# from the todolist in stdin
-check_bad_cmd_and_sha () {
-   retval=0
-   lineno=0
-   while read -r command rest
-   do
-   lineno=$(( $lineno + 1 ))
-   case $command in
-   "$comment_char"*|''|noop|x|exec)
-   # Doesn't expect a SHA-1
-   ;;
-   "$cr")
-   # Work around CR left by "read" (e.g. with Git for
-   # Windows' Bash).
-   ;;
-   pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
-   if ! check_commit_sha "${rest%%[]*}" "$lineno" 
"$1"
-   then
-   retval=1
-   fi
-   ;;
-   *)
-   line="$(sed -n -e "${lineno}p" "$1")"
-   warn "$(eval_gettext "\
-Warning: the command isn't recognized in the following line:
- - \$line")"
-   warn
-   retval=1
-   ;;
-   esac
-   done <"$1"
-   return $retval
-}
-
-# Print the list of the SHA-1 of the commits
-# from stdin to stdout
-todo_list_to_sha_list () {
-   git stripspace --strip-comments |
-   while read -r command sha1 rest
-   do
-   case $command in
-   "$comment_char"*|''|noop|x|"exec")
-   ;;
-   *)
-   long_sha=$(git rev-list --no-walk "$sha1" 2>/dev/null)
-   printf "%s\n" "$long_sha"
-   ;;
-   esac
-   done
-}
-
-# Use warn for each line in stdin
-warn_lines () {
-   while read -r line
-   do
-   warn " - $line"
-   done
-}
-
 # Switch to the branch in $into and notify it in the 

[PATCH v6 06/10] t3404: relax rebase.missingCommitsCheck tests

[Johannes Schindelin]

These tests were a bit anal about the *exact* warning/error message
printed by git rebase. But those messages are intended for the *end
user*, therefore it does not make sense to test so rigidly for the
*exact* wording.

In the following, we will reimplement the missing commits check in
the sequencer, with slightly different words.

So let's just test for the parts in the warning/error message that
we *really* care about, nothing more, nothing less.

Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh | 22 --
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 37821d24543..3704dbb2ecf 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1249,20 +1249,13 @@ test_expect_success 'rebase -i respects 
rebase.missingCommitsCheck = error' '
test B = $(git cat-file commit HEAD^ | sed -ne \$p)
 '
 
-cat >expect expect 

[PATCH v6 05/10] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

[Johannes Schindelin]

This is crucial to improve performance on Windows, as the speed is now
mostly dominated by the SHA-1 transformation (because it spawns a new
rev-parse process for *every* line, and spawning processes is pretty
slow from Git for Windows' MSYS2 Bash).

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c   | 10 +++-
 git-rebase--interactive.sh | 27 ++
 sequencer.c| 57 ++
 sequencer.h|  2 ++
 4 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 613053276e6..791b62901c3 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -14,7 +14,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct replay_opts opts = REPLAY_OPTS_INIT;
int keep_empty = 0;
enum {
-   CONTINUE = 1, ABORT, MAKE_SCRIPT
+   CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -25,6 +25,10 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
ABORT),
OPT_CMDMODE(0, "make-script", ,
N_("make rebase script"), MAKE_SCRIPT),
+   OPT_CMDMODE(0, "shorten-ids", ,
+   N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
+   OPT_CMDMODE(0, "expand-ids", ,
+   N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
OPT_END()
};
 
@@ -43,5 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!sequencer_remove_state();
if (command == MAKE_SCRIPT && argc > 1)
return !!sequencer_make_script(keep_empty, stdout, argc, argv);
+   if (command == SHORTEN_SHA1S && argc == 1)
+   return !!transform_todo_ids(1);
+   if (command == EXPAND_SHA1S && argc == 1)
+   return !!transform_todo_ids(0);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9d65212b7f1..d5df02435ae 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -750,35 +750,12 @@ skip_unnecessary_picks () {
die "$(gettext "Could not skip unnecessary pick commands")"
 }
 
-transform_todo_ids () {
-   while read -r command rest
-   do
-   case "$command" in
-   "$comment_char"* | exec)
-   # Be careful for oddball commands like 'exec'
-   # that do not have a SHA-1 at the beginning of $rest.
-   ;;
-   *)
-   sha1=$(git rev-parse --verify --quiet "$@" ${rest%%[
 ]*}) &&
-   if test "a$rest" = "a${rest#*[   ]}"
-   then
-   rest=$sha1
-   else
-   rest="$sha1 ${rest#*[]}"
-   fi
-   ;;
-   esac
-   printf '%s\n' "$command${rest:+ }$rest"
-   done <"$todo" >"$todo.new" &&
-   mv -f "$todo.new" "$todo"
-}
-
 expand_todo_ids() {
-   transform_todo_ids
+   git rebase--helper --expand-ids
 }
 
 collapse_todo_ids() {
-   transform_todo_ids --short
+   git rebase--helper --shorten-ids
 }
 
 # Rearrange the todo list that has both "pick sha1 msg" and
diff --git a/sequencer.c b/sequencer.c
index afcb3d00a32..36ed45d9050 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2462,3 +2462,60 @@ int sequencer_make_script(int keep_empty, FILE *out,
strbuf_release();
return 0;
 }
+
+
+int transform_todo_ids(int shorten_ids)
+{
+   const char *todo_file = rebase_path_todo();
+   struct todo_list todo_list = TODO_LIST_INIT;
+   int fd, res, i;
+   FILE *out;
+
+   strbuf_reset(_list.buf);
+   fd = open(todo_file, O_RDONLY);
+   if (fd < 0)
+   return error_errno(_("could not open '%s'"), todo_file);
+   if (strbuf_read(_list.buf, fd, 0) < 0) {
+   close(fd);
+   return error(_("could not read '%s'."), todo_file);
+   }
+   close(fd);
+
+   res = parse_insn_buffer(todo_list.buf.buf, _list);
+   if (res) {
+   todo_list_release(_list);
+   return error(_("unusable todo list: '%s'"), todo_file);
+   }
+
+   out = fopen(todo_file, "w");
+   if (!out) {
+   todo_list_release(_list);
+   return error(_("unable to open '%s' for writing"), todo_file);
+   }
+   for (i = 0; i < todo_list.nr; i++) {
+   struct todo_item *item = todo_list.items + i;
+   int bol = 

[PATCH v6 04/10] rebase -i: do not invent onelines when expanding/collapsing SHA-1s

[Johannes Schindelin]

To avoid problems with short SHA-1s that become non-unique during the
rebase, we rewrite the todo script with short/long SHA-1s before and
after letting the user edit the script. Since SHA-1s are not intuitive
for humans, rebase -i also provides the onelines (commit message
subjects) in the script, purely for the user's convenience.

It is very possible to generate a todo script via different means than
rebase -i and then to let rebase -i run with it; In this case, these
onelines are not required.

And this is where the expand/collapse machinery has a bug: it *expects*
that oneline, and failing to find one reuses the previous SHA-1 as
"oneline".

It was most likely an oversight, and made implementation in the (quite
limiting) shell script language less convoluted. However, we are about
to reimplement performance-critical parts in C (and due to spawning a
git.exe process for every single line of the todo script, the
expansion/collapsing of the SHA-1s *is* performance-hampering on
Windows), therefore let's fix this bug to make cross-validation with the
C version of that functionality possible.

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 93372c62b2e..9d65212b7f1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -760,7 +760,12 @@ transform_todo_ids () {
;;
*)
sha1=$(git rev-parse --verify --quiet "$@" ${rest%%[
 ]*}) &&
-   rest="$sha1 ${rest#*[]}"
+   if test "a$rest" = "a${rest#*[   ]}"
+   then
+   rest=$sha1
+   else
+   rest="$sha1 ${rest#*[]}"
+   fi
;;
esac
printf '%s\n' "$command${rest:+ }$rest"
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH v6 03/10] rebase -i: remove useless indentation

[Johannes Schindelin]

The commands used to be indented, and it is nice to look at, but when we
transform the SHA-1s, the indentation is removed. So let's do away with it.

For the moment, at least: when we will use the upcoming rebase--helper
to transform the SHA-1s, we *will* keep the indentation and can
reintroduce it. Yet, to be able to validate the rebase--helper against
the output of the current shell script version, we need to remove the
extra indentation.

Signed-off-by: Johannes Schindelin 
---
 git-rebase--interactive.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 05766835de1..93372c62b2e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -155,13 +155,13 @@ reschedule_last_action () {
 append_todo_help () {
gettext "
 Commands:
- p, pick = use commit
- r, reword = use commit, but edit the commit message
- e, edit = use commit, but stop for amending
- s, squash = use commit, but meld into previous commit
- f, fixup = like \"squash\", but discard this commit's log message
- x, exec = run command (the rest of the line) using shell
- d, drop = remove commit
+p, pick = use commit
+r, reword = use commit, but edit the commit message
+e, edit = use commit, but stop for amending
+s, squash = use commit, but meld into previous commit
+f, fixup = like \"squash\", but discard this commit's log message
+x, exec = run command (the rest of the line) using shell
+d, drop = remove commit
 
 These lines can be re-ordered; they are executed from top to bottom.
 " | git stripspace --comment-lines >>"$todo"
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH v6 01/10] t3415: verify that an empty instructionFormat is handled as before

[Johannes Schindelin]

An upcoming patch will move the todo list generation into the
rebase--helper. An early version of that patch regressed on an empty
rebase.instructionFormat value (the shell version could not discern
between an empty one and a non-existing one, but the C version used the
empty one as if that was intended to skip the oneline from the `pick
` lines).

Let's verify that this still works as before.

Signed-off-by: Johannes Schindelin 
---
 t/t3415-rebase-autosquash.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 5848949ec37..6d99f624b62 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -271,6 +271,18 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with 
custom inst format' '
test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)
 '
 
+test_expect_success 'autosquash with empty custom instructionFormat' '
+   git reset --hard base &&
+   test_commit empty-instructionFormat-test &&
+   (
+   set_cat_todo_editor &&
+   test_must_fail git -c rebase.instructionFormat= \
+   rebase --autosquash  --force -i HEAD^ >actual &&
+   git log -1 --format="pick %h %s" >expect &&
+   test_cmp expect actual
+   )
+'
+
 set_backup_editor () {
write_script backup-editor.sh <<-\EOF
cp "$1" .git/backup-"$(basename "$1")"
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH v6 02/10] rebase -i: generate the script via rebase--helper

[Johannes Schindelin]

The first step of an interactive rebase is to generate the so-called "todo
script", to be stored in the state directory as "git-rebase-todo" and to
be edited by the user.

Originally, we adjusted the output of `git log ` using a simple
sed script. Over the course of the years, the code became more
complicated. We now use shell scripting to edit the output of `git log`
conditionally, depending whether to keep "empty" commits (i.e. commits
that do not change any files).

On platforms where shell scripting is not native, this can be a serious
drag. And it opens the door for incompatibilities between platforms when
it comes to shell scripting or to Unix-y commands.

Let's just re-implement the todo script generation in plain C, using the
revision machinery directly.

This is substantially faster, improving the speed relative to the
shell script version of the interactive rebase from 2x to 3x on Windows.

Note that the rearrange_squash() function in git-rebase--interactive
relied on the fact that we set the "format" variable to the config setting
rebase.instructionFormat. Relying on a side effect like this is no good,
hence we explicitly perform that assignment (possibly again) in
rearrange_squash().

Signed-off-by: Johannes Schindelin 
---
 builtin/rebase--helper.c   |  8 +++-
 git-rebase--interactive.sh | 44 +
 sequencer.c| 49 ++
 sequencer.h|  3 +++
 4 files changed, 82 insertions(+), 22 deletions(-)

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index c82b4dce683..613053276e6 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,15 +12,19 @@ static const char * const builtin_rebase_helper_usage[] = {
 int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
+   int keep_empty = 0;
enum {
-   CONTINUE = 1, ABORT
+   CONTINUE = 1, ABORT, MAKE_SCRIPT
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
+   OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
ABORT),
+   OPT_CMDMODE(0, "make-script", ,
+   N_("make rebase script"), MAKE_SCRIPT),
OPT_END()
};
 
@@ -37,5 +41,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
return !!sequencer_continue();
if (command == ABORT && argc == 1)
return !!sequencer_remove_state();
+   if (command == MAKE_SCRIPT && argc > 1)
+   return !!sequencer_make_script(keep_empty, stdout, argc, argv);
usage_with_options(builtin_rebase_helper_usage, options);
 }
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 90b1fbe9cf6..05766835de1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -785,6 +785,7 @@ collapse_todo_ids() {
 # each log message will be re-retrieved in order to normalize the
 # autosquash arrangement
 rearrange_squash () {
+   format=$(git config --get rebase.instructionFormat)
# extract fixup!/squash! lines and resolve any referenced sha1's
while read -r pick sha1 message
do
@@ -1210,26 +1211,27 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
 fi
-format=$(git config --get rebase.instructionFormat)
-# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to 
parse
-git rev-list $merges_option --format="%m%H ${format:-%s}" \
-   --reverse --left-right --topo-order \
-   $revisions ${restrict_revision+^$restrict_revision} | \
-   sed -n "s/^>//p" |
-while read -r sha1 rest
-do
-
-   if test -z "$keep_empty" && is_empty_commit $sha1 && ! is_merge_commit 
$sha1
-   then
-   comment_out="$comment_char "
-   else
-   comment_out=
-   fi
+if test t != "$preserve_merges"
+then
+   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+   $revisions ${restrict_revision+^$restrict_revision} >"$todo"
+else
+   format=$(git config --get rebase.instructionFormat)
+   # the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
+   git rev-list $merges_option --format="%m%H ${format:-%s}" \
+   --reverse --left-right --topo-order \
+   $revisions ${restrict_revision+^$restrict_revision} | \
+   sed -n "s/^>//p" |
+   while read -r sha1 rest
+   do
+
+   if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
+   then
+ 

[PATCH v6 00/10] The final building block for a faster rebase -i

[Johannes Schindelin]

This patch series reimplements the expensive pre- and post-processing of
the todo script in C.

And it concludes the work I did to accelerate rebase -i so far.

I am still unwilling to replace a compile-time safe way to pass the
options to the revision machinery by the alternative (which I am still
flabbergasted about) proposed by Junio. This will not change.

I know that we want to concentrate on bug fixes on `master`, but this
patch series will most likely take a couple of months to get there, anyway.
So I may just as well send this iteration now. It's also not like I haven't
contributed any bug fixes lately...

Changes since v5:

- replaced a get_sha1() call by a get_oid() call already.

- adjusted to hashmap API changes


Johannes Schindelin (10):
  t3415: verify that an empty instructionFormat is handled as before
  rebase -i: generate the script via rebase--helper
  rebase -i: remove useless indentation
  rebase -i: do not invent onelines when expanding/collapsing SHA-1s
  rebase -i: also expand/collapse the SHA-1s via the rebase--helper
  t3404: relax rebase.missingCommitsCheck tests
  rebase -i: check for missing commits in the rebase--helper
  rebase -i: skip unnecessary picks using the rebase--helper
  t3415: test fixup with wrapped oneline
  rebase -i: rearrange fixup/squash lines using the rebase--helper

 Documentation/git-rebase.txt  |  16 +-
 builtin/rebase--helper.c  |  29 ++-
 git-rebase--interactive.sh| 373 -
 sequencer.c   | 531 ++
 sequencer.h   |   8 +
 t/t3404-rebase-interactive.sh |  22 +-
 t/t3415-rebase-autosquash.sh  |  28 ++-
 7 files changed, 647 insertions(+), 360 deletions(-)


base-commit: f3da2b79be9565779e4f76dc5812c68e156afdf0
Based-On: rebase--helper at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git rebase--helper
Published-As: https://github.com/dscho/git/releases/tag/rebase-i-extra-v6
Fetch-It-Via: git fetch https://github.com/dscho/git rebase-i-extra-v6

Interdiff vs v5:
 diff --git a/sequencer.c b/sequencer.c
 index 8713cc8d1d5..c54596f9699 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -2654,7 +2654,7 @@ int skip_unnecessary_picks(void)
  
if (!read_oneliner(, rebase_path_onto(), 0))
return error(_("could not read 'onto'"));
 -  if (get_sha1(buf.buf, onto_oid.hash)) {
 +  if (get_oid(buf.buf, _oid)) {
strbuf_release();
return error(_("need a HEAD to fixup"));
}
 @@ -2756,8 +2756,9 @@ struct subject2item_entry {
char subject[FLEX_ARRAY];
  };
  
 -static int subject2item_cmp(const struct subject2item_entry *a,
 -  const struct subject2item_entry *b, const void *key)
 +static int subject2item_cmp(const void *fndata,
 +  const struct subject2item_entry *a,
 +  const struct subject2item_entry *b, const void *key)
  {
return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
  }
 @@ -2802,7 +2803,7 @@ int rearrange_squash(void)
 * be moved to appear after the i'th.
 */
hashmap_init(, (hashmap_cmp_fn) subject2item_cmp,
 -   todo_list.nr);
 +   NULL, todo_list.nr);
ALLOC_ARRAY(next, todo_list.nr);
ALLOC_ARRAY(tail, todo_list.nr);
ALLOC_ARRAY(subjects, todo_list.nr);
-- 
2.13.3.windows.1.13.gaf0c2223da0

Re: Git Bash Bug

[Paul Smith]

On Fri, 2017-07-14 at 09:34 -0500, Kavita Desai wrote:
> Sorry for not being specific. What I meant by not working was that the
> bash commands are not found.
> Here is an example
> 
> $ ls
> bash: ls: command not found

The most obvious issue is your PATH is wrong.

What does "echo $PATH" show?

What does "type -a ls" show?

Regression in v2.13.3, fixed in master: aliases in worktrees

[Johannes Schindelin]

Hi all,

just to let you know that repository-local aliases stopped working in
v2.13.3. This is due to my "aliases via early config" work that made it
into a maintenance release...

The bug is fixed in master already, and I cherry-picked Brandon's config-h
series that contains the fix into Git for Windows' master branch.

I only had to add one add-on patch that falls back to opts->git_dir when
opts->commondir is NULL (no idea whether that can happen in Git's master,
but it can happen in Git for Windows' master at least).

It *may* be worth it to cherry-pick this to `maint`, too, but then, it
looks as if I am the only really serious user of worktrees anyway (given
that auto-gc still corrupts worktrees' reflogs, and it is a known bug for
2 years and counting).

So I dunno. Maybe not worth bothering about in `maint`. Less work for you,
Junio.

Ciao,
Johannes

Re: Git Bash Bug

[Kavita Desai]

Sorry for not being specific. What I meant by not working was that the
bash commands are not found.
Here is an example

$ ls
bash: ls: command not found

Kavita

On Fri, Jul 14, 2017 at 9:28 AM, Johannes Schindelin
 wrote:
> Hi Kavita,
>
> On Fri, 14 Jul 2017, Kavita Desai wrote:
>
>> Originally, when I first had Git Bash everything was working
>> perfectly. Randomly one day, my bash commands were not working.
>
> There are a gazillion ways how commands can "not work". You *need* to be
> more specific here. In the least, a precise pasted output of a failed
> command is necessary.
>
> In addition, a couple of other points of interest would be good to know,
> the issue reporting template at
> https://github.com/git-for-windows/git/issues/new tries to help you
> provide as much important information as you can. Maybe give it a try?
>
> Ciao,
> Johannes



-- 
Kavita Desai
UIUC Engineering Physics 2018
Engineering Outreach Society Engineering Open House Chair
UIUC Housing Ike. South Front Desk Clerk

Re: Git Bash Bug

[Johannes Schindelin]

Hi Kavita,

On Fri, 14 Jul 2017, Kavita Desai wrote:

> Originally, when I first had Git Bash everything was working
> perfectly. Randomly one day, my bash commands were not working.

There are a gazillion ways how commands can "not work". You *need* to be
more specific here. In the least, a precise pasted output of a failed
command is necessary.

In addition, a couple of other points of interest would be good to know,
the issue reporting template at
https://github.com/git-for-windows/git/issues/new tries to help you
provide as much important information as you can. Maybe give it a try?

Ciao,
Johannes

Re: reftable: new ref storage format

[Dave Borowitz]

On Thu, Jul 13, 2017 at 8:11 PM, Shawn Pearce  wrote:
> In another (Gerrit Code Review), we disable reflogs for
> the insane refs/changes/ namespace, as nearly every reference is
> created once, and never modified.

Apologies for the tangent, but this is not true in the most recent
Gerrit implementation. We update refs/changes/CD/ABCD/1 and
refs/changes/CD/ABCD/meta in a single BatchRefUpdate, and we set a
reflog message on the BatchRefUpdate instance, which updates the
reflog for all refs in the batch. The reflog message on /meta is
important, and arguably it's useful to be able to correlate that with
the reflog on /1.

If you think storing reflogs on patch set refs is going to be a
problem wrt on-disk storage, we should discuss this offline :)

Re: What's cooking in git.git (Jul 2017, #04; Thu, 13)

[Santiago Torres]

Hi, Junio. Thanks for replying.

> I postponed it when I saw it the first time to see if anybody
> comments on it, and then it turns out nobody was interested, and it
> remained uninteresting to the list to this day.  
> 

True, that's what I was afraid of, but I wanted to give it some closure.

> Now, after looking at the message again, from the patch description,
> I would believe you that you experienced _some_ trouble when the
> gpg-agent that is auto-spawned by gpg gets left behind (as I do not
> see any hits from "git grep gpg-agent t/", we are not deliberately
> using the agent). 

True, this is what I could gather from asking people over at #gnupg. The
agent spawns a socket for a GNUPGHOME and leaves it open outside of the
home folder (and it caches the inode for the olddir or so).

> However, I could not convince myself that the
> solution is credible.  

I think you're right on this. I'd rather have more people reproduce the
issue (some of my colleagues were able to do so, but we all were running
the latest GPG, vanilla conf) and find the root of the issue too.

> but the current directory of this part is the $TRASH_DIRECTORY,
> which is always created anew from the beginning in a test.  What
> agent process could possibly be running there immedately after
> creating ./gpghome (which happens with "mkdir gpghome &&" without
> "-p" just before the context of this hunk---if the agent was running
> already, the directory would have been there, and mkdir would have
> failed, which would have caused "test_set_prereq GPG" at the end of
> the "&&" cascade to be skipped.  In other words, it is unclear to
> me, and your log message does not sufficiently explain, why this is
> the right solution (or what the exact cause of the symptom is, for
> that matter).

I see. What is causing this (as far as my current understanding goes)
is:

1) First iteration of the tests is run, the .gpghome is created and a
unix socket is created too. The keychain is imported etc. Tests run
normally.

2) The test ends, and the trash directory (along with the .gpghome) are
flushed, but the agent is not aware of this. The socket is still
open.

3) The second iteration of the tests is run, the new .gpghome is
created, but when the keychain fails to import and the agent errors
out with ENOENT. The and-chain fails and test_set_preqreq GPG is
skipped (as you pointed out).

This last bit is apparently a result of the agent trying to be clever
with the paths. 

> 
> Or perhaps the gpg-agent socket is created somewhere outside the
> GNUPGHOME and stays behind even after a previous run of the same
> test finishes and $TRASH_DIRECTORY gets cleared (I am guessing the
> "what the exact cause is" part, as the log message did not explain
> it)?  If that is the case, it makes me wonder if either of the two
> alternative may be a more robust solution: (1) running gpg tests
> telling gpg never to use agent, or (2) telling gpg and gpg-agent to
> use a socket inside GNUPGHOME.

I agree. In hindsight this solution seems rather naive. I'll dig into
the root cause, as well as to try to isolate the issue from a
gpg-version and gpg-config viewpoint.

> After all, "kill"ing agent blindly like the above patch would mean
> you do not know what other party is relying on the proper operation
> of the thing you are killing.  That sounds more like a workaround
> that a solution (unless it is explained with a solid reason why that
> is the right way to run more than one instances of GPG).

I agree. It is probably better to seek the solutions that you suggested
above.

> 
> Perhaps everybody else is running these gpg tests without having to
> worry about gpg-agent already because their environment is more
> vanilla, but you have some configuration or environment that cause
> gpg to use agent, and that is the reason why nobody is interested
> (because they have never seen the symptom)?  It is possible that the
> part of t/test-lib.sh that tries to cleanse environment variables
> and other "external influence" to give us a predictable and
> repeatable test is unaware of such a knob that only some developers
> (including you) have and the rest of us were merely lucky.  Perhaps
> we need to throw GPG_AGENT_INFO SSH_AUTH_SOCK etc. into the list of
> envirionment variables to nuke there?
> 
> Combined with the unknown-ness of the root cause of the issue, I can
> only say that the patch may be raising an issue worth addressing,
> but it is too sketchy to tell if it is a right solution or what the
> exact problem being solved is.

I'll dig into this. This sounds a way more reasonable approach.

Thanks for the feedback!
-Santiago.


signature.asc
Description: PGP signature

Git Bash Bug

[Kavita Desai]

Hello,

Originally, when I first had Git Bash everything was working
perfectly. Randomly one day, my bash commands were not working. I
uninstalled Git Bash an reinstalled it. I have been trying for 3 weeks
to get it to work again. I am working on Windows 10.  I have tried
editing the PATH variables and the PATH environment variables and
nothing seems to be working. Do you have any other suggestions?



Thank you,

Kavita

Re: [PATCH] commit & merge: modularize the empty message validator

[Kaartic Sivaraam]

On Thu, 2017-07-13 at 10:58 -0700, Junio C Hamano wrote:
> I think many people know about and do use the "delete all lines"
> (i.e. ":1,$d" in vi, or \M-< \C-SPC \M-> \C-w in Emacs) to abort out
> of a commit or a merge.  I just do not think it is likely for them
> to leave Sign-off lines and remove everything else, which is more
> work than to delete everything, hence my reaction.
> 
Thanks! Didn't know this before.

Re: Git on macOS shows committed files as untracked

[Andreas Schwab]

On Jul 14 2017, Torsten Bögershausen  wrote:

> (Please no top-posting)
> On 14/07/17 11:45, Elliot Chandler wrote:
>> For what it's worth, the file looks normal in Gentoo GNU/Linux (name
>> appears "ḋἲ╓εﮯ⑏○╓Ӳ" and it seems to work like any other directory).
>>
> Thanks for testing -
> Normal and Normal ;-)
> For me the 6th code point does look strange
> The "box" with 04142F:

This is actually 1244f (CUNEIFORM NUMERIC SIGN ONE BAN2).

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Git on macOS shows committed files as untracked

[Jeff King]

On Fri, Jul 14, 2017 at 02:35:52PM +0200, Torsten Bögershausen wrote:

> (Please no top-posting)
> On 14/07/17 11:45, Elliot Chandler wrote:
> > For what it's worth, the file looks normal in Gentoo GNU/Linux (name
> > appears "ḋἲ╓εﮯ⑏○╓Ӳ" and it seems to work like any other directory).
> > 
> Thanks for testing -
> Normal and Normal ;-)
> For me the 6th code point does look strange
> The "box" with 04142F:
> Code point 4142F is unassigned and is outside any currently defined block 
> range.

Are you sure? I get u+1244f. I don't have a glyph for it in my font, but
at least it's a real unicode codepoint (cuneiform 1).

> So this is not valid Unicode, so we have a problem here under MacOS -
> not much Git can do about it.

I do still suspect this is the root of the problem. There's something
about the string that HFS+ doesn't want to store, and there's nothing we
can do to fix that.

-Peff

[PATCH v2 1/1] sha1_file: Add support for downloading blobs on demand

[Ben Peart]

This patch series enables git to request missing objects when they are
not found in the object store. This is a fault-in model where the
"read-object" sub-process will fetch the missing object and store it in
the object store as a loose, alternate, or pack file. On success, git
will retry the operation and find the requested object.

It utilizes the recent sub-process refactoring to spawn a "read-object"
hook as a sub-process on the first request and then all subsequent
requests are made to that existing sub-process. This significantly
reduces the cost of making multiple request within a single git command.

Signed-off-by: Ben Peart 
---
 Documentation/technical/read-object-protocol.txt | 102 
 cache.h  |   1 +
 config.c |   5 +
 contrib/long-running-read-object/example.pl  | 114 +
 environment.c|   1 +
 sha1_file.c  | 193 ++-
 t/t0410-read-object.sh   |  27 
 t/t0410/read-object  | 114 +
 8 files changed, 550 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/technical/read-object-protocol.txt
 create mode 100755 contrib/long-running-read-object/example.pl
 create mode 100755 t/t0410-read-object.sh
 create mode 100755 t/t0410/read-object

diff --git a/Documentation/technical/read-object-protocol.txt 
b/Documentation/technical/read-object-protocol.txt
new file mode 100644
index 00..a893b46e7c
--- /dev/null
+++ b/Documentation/technical/read-object-protocol.txt
@@ -0,0 +1,102 @@
+Read Object Process
+^^^
+
+The read-object process enables Git to read all missing blobs with a
+single process invocation for the entire life of a single Git command.
+This is achieved by using a packet format (pkt-line, see technical/
+protocol-common.txt) based protocol over standard input and standard
+output as follows. All packets, except for the "*CONTENT" packets and
+the "" flush packet, are considered text and therefore are
+terminated by a LF.
+
+Git starts the process when it encounters the first missing object that
+needs to be retrieved. After the process is started, Git sends a welcome
+message ("git-read-object-client"), a list of supported protocol version
+numbers, and a flush packet. Git expects to read a welcome response
+message ("git-read-object-server"), exactly one protocol version number
+from the previously sent list, and a flush packet. All further
+communication will be based on the selected version.
+
+The remaining protocol description below documents "version=1". Please
+note that "version=42" in the example below does not exist and is only
+there to illustrate how the protocol would look with more than one
+version.
+
+After the version negotiation Git sends a list of all capabilities that
+it supports and a flush packet. Git expects to read a list of desired
+capabilities, which must be a subset of the supported capabilities list,
+and a flush packet as response:
+
+packet: git> git-read-object-client
+packet: git> version=1
+packet: git> version=42
+packet: git> 
+packet: git< git-read-object-server
+packet: git< version=1
+packet: git< 
+packet: git> capability=get
+packet: git> capability=have
+packet: git> capability=put
+packet: git> capability=not-yet-invented
+packet: git> 
+packet: git< capability=get
+packet: git< 
+
+The only supported capability in version 1 is "get".
+
+Afterwards Git sends a list of "key=value" pairs terminated with a flush
+packet. The list will contain at least the command (based on the
+supported capabilities) and the sha1 of the object to retrieve. Please
+note, that the process must not send any response before it received the
+final flush packet.
+
+When the process receives the "get" command, it should make the requested
+object available in the git object store and then return success. Git will
+then check the object store again and this time find it and proceed.
+
+packet: git> command=get
+packet: git> sha1=0a214a649e1b3d5011e14a3dc227753f2bd2be05
+packet: git> 
+
+
+The process is expected to respond with a list of "key=value" pairs
+terminated with a flush packet. If the process does not experience
+problems then the list must contain a "success" status.
+
+packet: git< status=success
+packet: git< 
+
+
+In case the process cannot or does not want to process the content, it
+is expected to respond with an "error" status.
+
+packet: git< status=error
+packet: git< 
+
+
+In case the process cannot or does not want to process the content as
+well as any future content for the lifetime of the Git process, then 

[RFC/PATCH v2 0/1] Add support for downloading blobs on demand

[Ben Peart]

This patch series enables git to request missing objects when they are
not found in the object store. This is a fault-in model where the
"read-object" sub-process will fetch the missing object and store it in
the object store as a loose, alternate, or pack file. On success, git
will retry the operation and find the requested object.

It utilizes the recent sub-process refactoring to spawn a "read-object"
hook as a sub-process on the first request and then all subsequent
requests are made to that existing sub-process. This significantly
reduces the cost of making multiple request within a single git command.

An earlier version [1] of this patch series is pulled into [2] which
enables registering multiple ODB handlers which can be a simple command
or a sub-process. My hope is that this patch series can be completed to
meet the needs of the various efforts where faulting in missing objects
is required.

In the meantime, now that the sub-process refactoring has made it to
master, I�ve refactored this patch series to be as small and focused as
possible so it can be easily incorporated in other patch series where it
makes sense.

[3] has a need for faulting in missing objects and will be reworked to
take advantage of this patch series for its next iteration.

[4] has a similar function that uses a registered command instead of a
sub-process. Spawning a separate process for every missing object only
works if there are very few missing objects.  It does not scale well
when there are many missing objects (especially small objects like
commits and trees).

Patch is available here:
https://github.com/benpeart/git-for-windows/commits/read-object-process


[RFC] Add support for downloading blobs on demand 
[1] https://public-inbox.org/git/20170113155253.1644-1-benpe...@microsoft.com/

[RFC/PATCH v4 00/49] Add initial experimental external ODB support 
[2] https://public-inbox.org/git/20170620075523.26961-1-chrisc...@tuxfamily.org/

[PATCH v2 00/19] WIP object filtering for partial clone
[3] https://public-inbox.org/git/20170713173459.3559-1-...@jeffhostetler.com/

[RFC PATCH 0/3] Partial clone: promised blobs (formerly "missing blobs")
[4] https://public-inbox.org/git/cover.1499800530.git.jonathanta...@google.com/

Ben Peart (1):
  sha1_file: Add support for downloading blobs on demand

 Documentation/technical/read-object-protocol.txt | 102 
 cache.h  |   1 +
 config.c |   5 +
 contrib/long-running-read-object/example.pl  | 114 +
 environment.c|   1 +
 sha1_file.c  | 193 ++-
 t/t0410-read-object.sh   |  27 
 t/t0410/read-object  | 114 +
 8 files changed, 550 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/technical/read-object-protocol.txt
 create mode 100755 contrib/long-running-read-object/example.pl
 create mode 100755 t/t0410-read-object.sh
 create mode 100755 t/t0410/read-object

-- 
2.13.2.windows.1

Re: Git on macOS shows committed files as untracked

[Torsten Bögershausen]

(Please no top-posting)
On 14/07/17 11:45, Elliot Chandler wrote:
For what it's worth, the file looks normal in Gentoo GNU/Linux (name appears 
"ḋἲ╓εﮯ⑏○╓Ӳ" and it seems to work like any other directory).



Thanks for testing -
Normal and Normal ;-)
For me the 6th code point does look strange
The "box" with 04142F:
Code point 4142F is unassigned and is outside any currently defined block range.

So this is not valid Unicode, so we have a problem here under MacOS -
not much Git can do about it.



On Fri, Jul 14, 2017 at 4:41 AM, Torsten Bögershausen > wrote:




On 14/07/17 06:49, Lutz Roeder wrote:

Using precomposeunicode still reproduces the issue:

Repro steps:

1. Download
https://www.dropbox.com/s/0q5pbpqpckwzj7b/gitstatusrepro.zip?dl=0

2. unzip gitstatusrepro.zip && cd gitstatusrepro
3. git reset --hard
4. git -c core.precomposeunicode=true status

On branch master
Untracked files:
(use "git add ..." to include in what will be committed)


A (short) investigation shows that this seems to be invalid unicode,
at least from a MacOSX point of view ?

Unzipping your repo shows this:
  git status -u
   deleted:

"\341\270\213\341\274\262\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\323\262/test.txt"


===
If I run this:
  xx=$(printf

"d\314\207\316\271\314\223\314\200\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\320\243\314\213/")

echo   $xx | iconv -f UTF-8-MAC -t UTF-16 | xxd

iconv: (stdin):1:5: cannot convert
000: feff 1e0b 1f32 2553 03b5 fbaf.2%S
===
So I don't know if we can do something in Git to improve your repo.
How did  you end up with such a directory name ?
And would it be possible to rename it ?

Re: Reducing redundant build at Travis?

[Jeff King]

On Thu, Jul 13, 2017 at 02:53:17PM -0700, Junio C Hamano wrote:

> >> Unfortunately, https://travis-ci.org/git/git/builds/ shows that it
> >> does not care if it spawned a job to build the tip of 'maint' and
> >> another for 'v2.13.3' that point at the same thing.
> >
> > That is indeed suprising and wasteful. Looks like other people
> > did run into the same issue. How about something like this?
> > https://github.com/mockito/mockito/blob/release/2.x/.travis.yml#L26-L29
> 
> That unfortunately is exactly what I wanted to avoid.
> 
> We'd want to test tagged releases, and we'd want to test usual
> updates to integration branches.  It just is that sometimes the tips
> of integration branches happen to be at the tagged release, so I'd
> prefer to always build tags but skip a branch build if it happens to
> be also tagged.  After all, none of the integration branches may
> directly point at a tagged release when the tag is pushed out.

Right, I think the right solution is some amount of peeling. Recognizing
that the commit sha1 is the same, or better yet, not bothering to retest
trees which have already been tested.

We used a hacked-up copy of Jenkins for our in-house CI, and it skips
already-tested trees.  Besides the discussed cases, this also saves time
when pushing noop rebases (e.g., when you just changed commit messages)
and noop merges (e.g., if you already back-merged master to your topic,
tested it, and now do a "merge --no-ff" to merge the topic in). I don't
think either of those are common in our workflows, though.

If we had some kind of persistent storage, we could do a quick:

  tree=$(git rev-parse HEAD^{tree})
  if test "$(get_from_storage status-$tree)" = "good"
  then
echo "Already tested $tree, skipping"
exit 0
  fi
  ... run actual tests ...
  put_into_storage status-$tree good

The "git test" script[1] uses this strategy with git-notes as the
storage, and I've found it quite useful. I don't think we can rely on
git-notes, but I think Travis gives us some storage options. Even just a
best-effort cache directory would probably be sufficient (this is an
optimization, after all).

-Peff

[1] https://github.com/mhagger/git-test

Re: [PATCH 03/15] t: use test_decode_color rather than literal ANSI codes

[Jeff King]

On Thu, Jul 13, 2017 at 12:27:41PM -0700, Junio C Hamano wrote:

> > I agree it's a bit gross. Possibly:
> >
> >   git log --format='%C(auto)%d %s'
> >
> > would be easier for the test to parse (I'm pretty sure that didn't exist
> > back when this test was written).
> 
> Yeah, that may make the test easier to read, too.

I started on that, but the test is actually checking the coloring of the
commit message, too. So switching to a custom format would lose that
part of the test.

-Peff

Re: Git on macOS shows committed files as untracked

[Elliot Chandler]

For what it's worth, the file looks normal in Gentoo GNU/Linux (name
appears "ḋἲ╓εﮯ⑏○╓Ӳ" and it seems to work like any other directory).


On Fri, Jul 14, 2017 at 4:41 AM, Torsten Bögershausen  wrote:
>
>
> On 14/07/17 06:49, Lutz Roeder wrote:
>>
>> Using precomposeunicode still reproduces the issue:
>>
>> Repro steps:
>>
>> 1. Download
>> https://www.dropbox.com/s/0q5pbpqpckwzj7b/gitstatusrepro.zip?dl=0
>> 2. unzip gitstatusrepro.zip && cd gitstatusrepro
>> 3. git reset --hard
>> 4. git -c core.precomposeunicode=true status
>>
>> On branch master
>> Untracked files:
>>(use "git add ..." to include in what will be committed)
>
>
> A (short) investigation shows that this seems to be invalid unicode,
> at least from a MacOSX point of view ?
>
> Unzipping your repo shows this:
>  git status -u
>   deleted:
> "\341\270\213\341\274\262\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\323\262/test.txt"
>
> ===
> If I run this:
>  xx=$(printf
> "d\314\207\316\271\314\223\314\200\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\320\243\314\213/")
>
> echo   $xx | iconv -f UTF-8-MAC -t UTF-16 | xxd
>
> iconv: (stdin):1:5: cannot convert
> 000: feff 1e0b 1f32 2553 03b5 fbaf.2%S
> ===
> So I don't know if we can do something in Git to improve your repo.
> How did  you end up with such a directory name ?
> And would it be possible to rename it ?
>
>

Re: [PATCH 0/2] Fix regression: CamelCased aliases

[Jeff King]

On Fri, Jul 14, 2017 at 10:39:24AM +0200, Johannes Schindelin wrote:

> It was possible before v2.13.3 to invoke:
> 
>   git config alias.CamelCased 
>   git CamelCased
> 
> This regressed (due to a stupid mistake of mine that was not caught in
> patch review, sadly) in v2.13.3.

Interesting. I don't think this was ever intended to work. Prior to
v2.2.0 it did not, and it was "fixed" inadvertently by 111791559
(alias.c: replace `git_config()` with `git_config_get_string()`,
2014-08-07).

It's a known limitation of the alias config scheme that it cannot
distinguish between "camelcase" and "CamelCase" (nor represent commands
with underscores or other characters that are invalid in a keyname). In
the long run we'd want to allow "alias.CamelCase.command" or similar to
make this work correctly.

It probably doesn't hurt anything to make this old ambiguous style work
in the meantime if people are using it and finding it convenient.

> Johannes Schindelin (2):
>   t1300: demonstrate that CamelCased aliases regressed
>   alias: compare alias name *case-insensitively*

The patches look obviously correct.

As a meta-comment, I find splitting the tests from the fix like this
makes review more tedious. The commit messages have to repeat the exact
same reasoning.

The only value I think it brings is that you can confirm in the
first commit that the expect_failure does indeed fail. I guess that has
some value, though I usually do it by running the test against the
version of git built from HEAD^.

-Peff

Darlehensangebot

[MASTHAVEN LOAN FINANCE]

Schönen Tag,

 Brauchen Sie eine finanzielle Unterstützung? Brauchen Sie einen legitimen 
Kredit für Zinsen? Brauchen Sie ein Business-Darlehen? Brauchen Sie ein 
Darlehen, um ein Haus zu kaufen, Auto, zahlen Sie Ihre Rechnungen und Schulden? 
Brauchen Sie Geld, um Probleme zu lösen? Wenn so freundlich ein Darlehen bei 
uns um 1,1% Zinssatz beantragen, reichen unsere Darlehen zwischen 5.000,00 bis 
10.000.000,00 U.S Dollar oder Euro oder Pfund mit einer maximalen Dauer von 15 
Jahren. Bewerben Sie sich jetzt mit den folgenden Angaben und schicken Sie uns 
eine E-Mail an: masthavenlo...@gmail.com

HINWEIS: Lassen Sie sich diese Gelegenheit nicht überlassen. Holen Sie sich ein 
Darlehen, um Ihre finanziellen Probleme zu lösen. Wenn Sie interessiert sind, 
füllen Sie das Darlehensantrag sofort aus und senden Sie es zurück.

INFORMATIONEN BENÖTIGT

Deine Namen:

Adresse: ...
Telefon: ...
Benötigte Menge: ...
Dauer: ...
Beruf: ...
Monatliches Einkommensniveau: ..
Geschlecht: ..
Geburtsdatum: ...
Bundesland: ...
Land: .
Zweck: .

Danke für dein Verständnis

Dringende Antwort von Ihnen jetzt benötigt

Mit freundlichen Grüßen

Mr.Mark Hirt

Re: Git on macOS shows committed files as untracked

[Torsten Bögershausen]

On 14/07/17 06:49, Lutz Roeder wrote:

Using precomposeunicode still reproduces the issue:

Repro steps:

1. Download https://www.dropbox.com/s/0q5pbpqpckwzj7b/gitstatusrepro.zip?dl=0
2. unzip gitstatusrepro.zip && cd gitstatusrepro
3. git reset --hard
4. git -c core.precomposeunicode=true status

On branch master
Untracked files:
   (use "git add ..." to include in what will be committed)


A (short) investigation shows that this seems to be invalid unicode,
at least from a MacOSX point of view ?

Unzipping your repo shows this:
 git status -u
  deleted: 
"\341\270\213\341\274\262\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\323\262/test.txt" 



===
If I run this:
 xx=$(printf 
"d\314\207\316\271\314\223\314\200\342\225\223\316\265\357\256\257\360\222\221\217\342\227\213\342\225\223\320\243\314\213/")


echo   $xx | iconv -f UTF-8-MAC -t UTF-16 | xxd

iconv: (stdin):1:5: cannot convert
000: feff 1e0b 1f32 2553 03b5 fbaf.2%S
===
So I don't know if we can do something in Git to improve your repo.
How did  you end up with such a directory name ?
And would it be possible to rename it ?

[PATCH 2/2] alias: compare alias name *case-insensitively*

[Johannes Schindelin]

It is totally legitimate to add CamelCased aliases, but due to the way
config keys are compared, the case does not matter.

Therefore, we must compare the alias name insensitively to the config
keys.

This fixes a regression introduced by a9bcf6586d1 (alias: use
the early config machinery to expand aliases, 2017-06-14).

Signed-off-by: Johannes Schindelin 
---
 alias.c| 2 +-
 t/t1300-repo-config.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/alias.c b/alias.c
index 39f622e4141..bf146e52632 100644
--- a/alias.c
+++ b/alias.c
@@ -11,7 +11,7 @@ static int config_alias_cb(const char *key, const char 
*value, void *d)
struct config_alias_data *data = d;
const char *p;
 
-   if (skip_prefix(key, "alias.", ) && !strcmp(p, data->alias))
+   if (skip_prefix(key, "alias.", ) && !strcasecmp(p, data->alias))
return git_config_string((const char **)>v, key, value);
 
return 0;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 0df71b84ccf..364a537000b 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1075,7 +1075,7 @@ test_expect_success 'git -c works with aliases of 
builtins' '
test_cmp expect actual
 '
 
-test_expect_failure 'aliases can be CamelCased' '
+test_expect_success 'aliases can be CamelCased' '
test_config alias.CamelCased "rev-parse HEAD" &&
git CamelCased >out &&
git rev-parse HEAD >expect &&
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH 0/2] Fix regression: CamelCased aliases

[Johannes Schindelin]

It was possible before v2.13.3 to invoke:

git config alias.CamelCased 
git CamelCased

This regressed (due to a stupid mistake of mine that was not caught in
patch review, sadly) in v2.13.3.

And this patch series fixes it again, introducing a regression test to
ensure that it does not get broken again.


Johannes Schindelin (2):
  t1300: demonstrate that CamelCased aliases regressed
  alias: compare alias name *case-insensitively*

 alias.c| 2 +-
 t/t1300-repo-config.sh | 7 +++
 2 files changed, 8 insertions(+), 1 deletion(-)


base-commit: f3da2b79be9565779e4f76dc5812c68e156afdf0
Published-As: https://github.com/dscho/git/releases/tag/CamelCased-aliases-v1
Fetch-It-Via: git fetch https://github.com/dscho/git CamelCased-aliases-v1
-- 
2.13.3.windows.1.13.gaf0c2223da0

[PATCH 1/2] t1300: demonstrate that CamelCased aliases regressed

[Johannes Schindelin]

It is totally legitimate to add CamelCased aliases, but due to the way
config keys are compared, the case does not matter.

Except that now it does: the alias name is expected to be all
lower-case. This is a regression introduced by a9bcf6586d1 (alias: use
the early config machinery to expand aliases, 2017-06-14).

Noticed by Alejandro Pauly, diagnosed by Kevin Willford.

Signed-off-by: Johannes Schindelin 
---
 t/t1300-repo-config.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index a37ef042221..0df71b84ccf 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1075,6 +1075,13 @@ test_expect_success 'git -c works with aliases of 
builtins' '
test_cmp expect actual
 '
 
+test_expect_failure 'aliases can be CamelCased' '
+   test_config alias.CamelCased "rev-parse HEAD" &&
+   git CamelCased >out &&
+   git rev-parse HEAD >expect &&
+   test_cmp expect out
+'
+
 test_expect_success 'git -c does not split values on equals' '
echo "value with = in it" >expect &&
git -c core.foo="value with = in it" config core.foo >actual &&
-- 
2.13.3.windows.1.13.gaf0c2223da0

Re: [PATCH v1 0/4] Teach 'run' perf script to read config files

[Jeff King]

On Fri, Jul 14, 2017 at 08:27:53AM +0200, Christian Couder wrote:

> The whole thing seems really complex to me though. And this makes me
> think that people might want to specify different GIT-BUILD-OPTIONS
> and config.mak files to be used when running perf tests, so that the
> results from perf tests can easily be reproduced later even if they
> have changed their build options in their development Git repo in the
> meantime.

I agree with the complexity. The general idea is that your currently
built HEAD is a snapshot in time of options. But that doesn't have to be
so, and laying out the options in a config file does seem like an
improvement.

There is another implicit dependency, though: the set of (and exact
content of) the tests depends on your HEAD, too. So if I do:

  git checkout v2.5.0
  cd t/perf
  ./run v2.0.0 v2.1.0

I might get different results if I replace "v2.5.0" in the first command
with some other version, because the content of the tests will be
different. I'm not sure how to account for that in storing results. Most
of the time the version of the tests you ran is not going to be
interesting. But it can be a source of confusing discrepancies if a test
subtly changed between two runs. It probably happens infrequently enough
that it's not worth worrying about.

> So perhaps the config file should make it possible to specify a
> directory where all the build files (GIT-BUILD-OPTIONS, config.mak,
> config.mak.autogen and config.status) that should be used should be
> taken. And then it could also let people change some variables to
> override what is in those files which is needed to run perf tests with
> different parameters.

That sounds reasonable. I think you could ditch GIT-BUILD-OPTIONS
entirely. It's only needed to pull in GIT_PERF variables that would be
better served by being in the config in the first place.

-Peff

Re: [PATCH v1 0/4] Teach 'run' perf script to read config files

[Christian Couder]

On Thu, Jul 13, 2017 at 10:55 PM, Jeff King  wrote:
> On Thu, Jul 13, 2017 at 08:57:01PM +0200, Christian Couder wrote:
>
>> >> We want to make it possible to store the parameters to the 'run'
>> >> script in a config file. This will make it easier to store, reuse,
>> >> share and compare parameters.
>> >
>> > Because perf-lib is built on test-lib, it already reads
>> > GIT-BUILD-OPTIONS.
>>
>> Actually the 'run' script also sources GIT-BUILD-OPTIONS, so maybe
>> this is not necessary.
>
> Ah, right. The one that comes via perf-lib gets the variables into the
> test scripts themselves. But anything "run" would need itself would come
> from the source it does itself. And that's where GIT_PERF_MAKE_OPTS has
> an effect.
>
>> Also are the variables in GIT-BUILD-OPTIONS exported already?
>
> No, I don't think so. But because both "run" and the scripts themselves
> source them, they're available more or less everywhere, except for
> sub-processes inside the scripts.

Ok, I see.

>> > And the Makefile copies several perf-related values
>> > into it, including GIT_PERF_MAKE_OPTS and GIT_PERF_REPEAT_COUNT. So you
>> > can already do:
>> >
>> >   echo 'GIT_PERF_REPEAT_COUNT = 10' >>config.mak
>> >   echo 'GIT_PERF_MAKE_OPTS = CFLAGS="-O2" DEVELOPER=1' >>config.mak
>> >   make
>>
>> The "make" here might not even be needed as in the 'run' script
>> "config.mak" is copied into the "build/$rev" directory where "make" is
>> run to build the $rev version.
>
> You need it to bake the config into GIT-BUILD-OPTIONS, which is the only
> thing that gets read by "run" and the perf scripts. If you are
> just setting MAKE_OPTS to things that your config.mak already sets, then
> yes, you can skip that one.

Thanks for the explanations.

The whole thing seems really complex to me though. And this makes me
think that people might want to specify different GIT-BUILD-OPTIONS
and config.mak files to be used when running perf tests, so that the
results from perf tests can easily be reproduced later even if they
have changed their build options in their development Git repo in the
meantime.

So perhaps the config file should make it possible to specify a
directory where all the build files (GIT-BUILD-OPTIONS, config.mak,
config.mak.autogen and config.status) that should be used should be
taken. And then it could also let people change some variables to
override what is in those files which is needed to run perf tests with
different parameters.