[ANNOUNCE] Git v2.16.0-rc2

[Junio C Hamano]

A release candidate Git v2.16.0-rc2 is now available for testing
at the usual places.  It is comprised of 483 non-merge commits
since v2.15.0, contributed by 80 people, 23 of which are new faces.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/testing/

The following public repositories all have a copy of the
'v2.16.0-rc2' tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.15.0 are as follows.
Welcome to the Git development community!

  Albert Astals Cid, Antoine Beaupré, Damien Marié, Daniel
  Bensoussan, Florian Klink, Gennady Kupava, Guillaume Castagnino,
  Haaris Mehmood, Hans Jerry Illikainen, Ingo Ruhnke, Jakub
  Bereżański, Jean Carlo Machado, Julien Dusser, J Wyman,
  Kevin, Łukasz Stelmach, Marius Paliga, Olga Telezhnaya,
  Rafael Ascensão, Robert Abel, Robert P. J. Day, Shuyu Wei,
  and Wei Shuyu.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Alex Vandiver,
  Anders Kaseorg, Andrey Okoshkin, Ann T Ropea, Beat Bolli,
  Ben Peart, Brandon Williams, brian m. carlson, Carlos Martín
  Nieto, Charles Bailey, Christian Couder, Dave Borowitz, Dennis
  Kaarsemaker, Derrick Stolee, Elijah Newren, Emily Xie, Eric
  Sunshine, Eric Wong, Heiko Voigt, Jacob Keller, Jameson Miller,
  Jean-Noel Avila, Jeff Hostetler, Jeff King, Johannes Schindelin,
  Jonathan Nieder, Jonathan Tan, Junio C Hamano, Kaartic Sivaraam,
  Kevin Daudt, Lars Schneider, Liam Beguin, Luke Diamand, Martin
  Ågren, Michael Haggerty, Nicolas Morey-Chaisemartin, Phil Hord,
  Phillip Wood, Pranit Bauva, Prathamesh Chavan, Ralf Thielow,
  Ramsay Jones, Randall S. Becker, Rasmus Villemoes, René Scharfe,
  Simon Ruderich, Stefan Beller, Steffen Prohaska, Stephan Beyer,
  SZEDER Gábor, Thomas Braun, Thomas Gummerer, Todd Zullinger,
  Torsten Bögershausen, and W. Trevor King.



Git 2.16 Release Notes (draft)
==

Backward compatibility notes and other notable changes.

 * Use of an empty string as a pathspec element that is used for
   'everything matches' is now an error.


Updates since v2.15
---

UI, Workflows & Features

 * An empty string as a pathspec element that means "everything"
   i.e. 'git add ""', is now illegal.  We started this by first
   deprecating and warning a pathspec that has such an element in
   2.11 (Nov 2016).

 * A hook script that is set unexecutable is simply ignored.  Git
   notifies when such a file is ignored, unless the message is
   squelched via advice.ignoredHook configuration.

 * "git pull" has been taught to accept "--[no-]signoff" option and
   pass it down to "git merge".

 * The "--push-option=" option to "git push" now defaults to a
   list of strings configured via push.pushOption variable.

 * "gitweb" checks if a directory is searchable with Perl's "-x"
   operator, which can be enhanced by using "filetest 'access'"
   pragma, which now we do.

 * "git stash save" has been deprecated in favour of "git stash push".

 * The set of paths output from "git status --ignored" was tied
   closely with its "--untracked=" option, but now it can be
   controlled more flexibly.  Most notably, a directory that is
   ignored because it is listed to be ignored in the ignore/exclude
   mechanism can be handled differently from a directory that ends up
   to be ignored only because all files in it are ignored.

 * The remote-helper for talking to MediaWiki has been updated to
   truncate an overlong pagename so that ".mw" suffix can still be
   added.

 * The remote-helper for talking to MediaWiki has been updated to
   work with mediawiki namespaces.

 * The "--format=..." option "git for-each-ref" takes learned to show
   the name of the 'remote' repository and the ref at the remote side
   that is affected for 'upstream' and 'push' via "%(push:remotename)"
   and friends.

 * Doc and message updates to teach users "bisect view" is a synonym
   for "bisect visualize".

 * "git bisect run" that did not specify any command to run used to go
   ahead and treated all commits to be tested as 'good'.  This has
   been corrected by making the command error out.

 * The SubmittingPatches document has been converted to produce an
   HTML version via AsciiDoc/Asciidoctor.

 * We learned to talk to watchman to speed up "git status" and other
   operations that need to see which paths have been modified.

 * The "diff" family of commands learned to ignore differences in
   carriage return at the end of line.

 * Places that know about "sendemail.to", like documentation and shell
   completion (in contrib/) have been taught about "sendemail.tocmd",
   too.

 * "git add --renormalize ." is a new and safer way to 

Re: [PATCH] doc/read-tree: remove obsolete remark

[Jeff King]

On Thu, Jan 11, 2018 at 03:14:07PM -0800, Junio C Hamano wrote:

> >> Doesn't "git read-tree --prefix=previous HEAD^" add paths like
> >> "previous/Documentation/Makefile" to the index, i.e. instead of
> >> forcing you to have the required slash at the end, we give one for
> >> free when it is missing?
> >
> > Yes, I think it does what you'd want with that path. But it would not do
> > what you want by adding "previous-file". Which seems like a gotcha that
> > should be mentioned.
> 
> I am a bit puzzled.  
> 
> Do you mean a user who types "git read-tree --prefix=v1- HEAD^" may
> be expecting to see that the blob object "HEAD^:Makefile" added at
> path "v1-Makefile" etc?

Sorry, I was somewhat turned around in my example, thinking that we were
matching existing entries by prefix here and not putting entries into a
new prefix[1].

But yes, your example hits the point that I think is left unsaid: does
"--prefix=sub" mean the same thing as "--prefix=sub/", or is it a true
string prefix? Reading more carefully, though, we say "under the
directory at " in the earlier part, which is probably
sufficient.

Note that this _is_ different than "git checkout-index --prefix", which
is a strict string prefix (i.e., you can checkout "--prefix=v1-" and get
"v1-Makefile").

-Peff

[1] I was trying to figure out which feature of Git I was confusing it
with, but couldn't find one. I think I may have just been thinking
of checkout-index (which is not about matching existing paths, but
does have the different behavior). Normally matching of existing
paths is done with pathspecs, which I think should all use directory
boundaries for prefix-matching.

Re: [PATCH] doc/read-tree: remove obsolete remark

[Junio C Hamano]

Jeff King  writes:

> On Thu, Jan 11, 2018 at 11:02:04AM -0800, Junio C Hamano wrote:
>
>> >> ---prefix=/::
>> >> +--prefix=::
>> >>   Keep the current index contents, and read the contents
>> >>   of the named tree-ish under the directory at ``.
>> >>   The command will refuse to overwrite entries that already
>> >> - existed in the original index file. Note that the `/`
>> >> - value must end with a slash.
>> >> + existed in the original index file.
>> >
>> > Is it worth mentioning in the new world order that the slash is not
>> > implied? I.e., that you probably do want to say "--prefix=foo/" if you
>> > want the subdirectory "foo", but do not want to match "foobar"?
>> 
>> Doesn't "git read-tree --prefix=previous HEAD^" add paths like
>> "previous/Documentation/Makefile" to the index, i.e. instead of
>> forcing you to have the required slash at the end, we give one for
>> free when it is missing?
>
> Yes, I think it does what you'd want with that path. But it would not do
> what you want by adding "previous-file". Which seems like a gotcha that
> should be mentioned.

I am a bit puzzled.  

Do you mean a user who types "git read-tree --prefix=v1- HEAD^" may
be expecting to see that the blob object "HEAD^:Makefile" added at
path "v1-Makefile" etc?

Re: [PATCH] doc/read-tree: remove obsolete remark

[Jeff King]

On Thu, Jan 11, 2018 at 11:02:04AM -0800, Junio C Hamano wrote:

> >> ---prefix=/::
> >> +--prefix=::
> >>Keep the current index contents, and read the contents
> >>of the named tree-ish under the directory at ``.
> >>The command will refuse to overwrite entries that already
> >> -  existed in the original index file. Note that the `/`
> >> -  value must end with a slash.
> >> +  existed in the original index file.
> >
> > Is it worth mentioning in the new world order that the slash is not
> > implied? I.e., that you probably do want to say "--prefix=foo/" if you
> > want the subdirectory "foo", but do not want to match "foobar"?
> 
> Doesn't "git read-tree --prefix=previous HEAD^" add paths like
> "previous/Documentation/Makefile" to the index, i.e. instead of
> forcing you to have the required slash at the end, we give one for
> free when it is missing?

Yes, I think it does what you'd want with that path. But it would not do
what you want by adding "previous-file". Which seems like a gotcha that
should be mentioned.

-Peff

RE: [BUG] Weird breakages in t1450 #2 on NonStop

[Randall S. Becker]

On January 11, 2018 9:46 AM, I wrote:
> This one has me scratching my head:
> 
> The object file name being reported below in t1450, subtest 2 is corrupt,
but I
> can't figure out why the script might be generating this condition -
there's
> nothing apparent, but it looks like the git commit -m C step is reporting
or
> using a bad name. This breakage was not present in 2.8.5 (now at 7234152
> (2.13.5) and is persistent (i.e. always happens). This is the only test in
all of
> git where I have observed this particular situation.
> Adding set -x to test_commit is unrevealing. The git fsck in this test is
never
> executed because the test_commit fails with a non-zero git commit
> completion code. There is no rn (actual r n 252 252 252 252) in the
objects
> directory - even the 'rn' does not correspond to anything.. I am
suspecting an
> unterminated string that ran into freed memory somewhere, but that's
> speculative.

Does anyone recall fixing this one at or near
dfe46c5ce6e68d682f80f9874f0eb107e9fee797? There was a rewrite of sha1_file.c
including link_alt_odb_entry where I am finding memory corruptions. I think
I'm chasing something that was already fixed some time after 2.13.5 but the
common parent to where I am is pretty far back compared to master.

Thanks,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(2112884442)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.

git gc --auto yelling at users where a repo legitimately has >6700 loose objects

[Ævar Arnfjörð Bjarmason]

I recently disabled gc.auto=0 and my nightly aggressive repack script on
our big monorepo across our infra, relying instead on git gc --auto in
the background to just do its thing.

I didn't want users to wait for git-gc, and I'd written this nightly
cronjob before git-gc learned to detach to the background.

But now I have git-gc on some servers yelling at users on every pull
command:

warning: There are too many unreachable loose objects; run 'git prune' to 
remove them.

The reason is that I have all the values at git's default settings, and
there legitimately are >~6700 loose objects that were created in the
last 2 weeks.

For those rusty on git-gc's defaults, this is what it looks like in this
scenario:

 1. User runs "git pull"
 2. git gc --auto is called, there are >6700 loose objects
 3. it forks into the background, tries to prune and repack, objects
older than gc.pruneExpire (2.weeks.ago) are pruned.
 4. At the end of all this, we check *again* if we have >6700 objects,
if we do we print "run 'git prune'" to .git/gc.log, and will just
emit that error for the next day before trying again, at which point
we unlink the gc.log and retry, see gc.logExpiry.

Right now I've just worked around this by setting gc.pruneExpire to a
lower value (4.days.ago). But there's a larger issue to be addressed
here, and I'm not sure how.

When the warning was added in [1] it didn't know to detach to the
background yet, that came in [2], shortly after came gc.log in [3].

We could add another gc.auto-like limit, which could be set at some
higher value than gc.auto. "Hey if I have more than 6700 loose objects,
prune the <2wks old ones, but if at the end there's still >6700 I don't
want to hear about it unless there's >6700*N".

I thought I'd just add that, but the details of how to pass that message
around get nasty. With that solution we *also* don't want git gc to
start churning in the background once we reach >6700 objects, so we need
something like gc.logExpiry which defers the gc until the next day. We
might need to create .git/gc-waitabit.marker, ew.

More generally, these hard limits seem contrary to what the user cares
about. E.g. I suspect that most of these loose objects come from
branches since deleted in upstream, whose objects could have a different
retention policy.

Or we could say "I want 2 weeks of objects, but if that runs against the
6700 limit just keep the latest 6700/2".

1. a087cc9819 ("git-gc --auto: protect ourselves from accumulated
   cruft", 2007-09-17)
2. 9f673f9477 ("gc: config option for running --auto in background",
   2014-02-08)
3. 329e6e8794 ("gc: save log from daemonized gc --auto and print it next
   time", 2015-09-19)

Dear Friend Please Respond

[james kabore]

From:MR.JAMES KABORE.
Bill and Exchange Manager
Micro Finance Bank Plc
Burkina Faso

Dear Friend,

I know that this mail will come to you as a surprise. I am MR.JAMES
KABORE. and I am  the bill and Exchange manager in a Bank here in my
country .I Hope that you will not expose or betray this trust and
confident that i am about to Repose on you for the mutual benefit of
our both families.

I need your urgent assistance in transferring the sum of $15 million
Immediately to your account. The money has been dormant for years in
our Bank here without any body coming for it.

I want my bank to release the money to you as the nearest person to
our deceased customer the owner Of the account who died a long with
his supposed next of kin in an air Crash since July 2000.I don't want
the money to go into our Bank treasury as an abandoned fund. So this
is the reason why i contacted you, so that the bank can release the
money to you as the nearest person to the deceased customer.

Please i will like you to keep this proposal as a top secret . Upon
receipt of your reply, I will send you full details on how the
transfer will be executed and also note that you will have 40% of the
Above mentioned sum. Acknowledge receipt of this message in acceptance
of my mutual business endeavour by furnishing me with the following
information;

1. Your Full Names and Address...

2. Country... ..

3. Direct Telephone .

4. Occupation ..

Please send me your  information as soon as you reply   I will give
you full details on how you and me can claim the money from our bank.

I am waiting to receive your reply. please replied me with this email address,
james.kabor...@gmail.com


MR.JAMES KABORE
Micro Finance Bank ,
Burkina Faso

Re: [PATCH v2 2/2] submodule: port submodule subcommand 'deinit' from shell to C

[Junio C Hamano]

Prathamesh Chavan  writes:

> + /* remove the submodule work tree (unless the user already did it) */
> + if (is_directory(path)) {
> + struct strbuf sb_rm = STRBUF_INIT;
> + const char *format;
> +
> + /*
> +  * protect submodules containing a .git directory
> +  * NEEDSWORK: instead of dying, automatically call
> +  * absorbgitdirs and (possibly) warn.
> +  */
> + if (is_directory(sub_git_dir))
> + die(_("Submodule work tree '%s' contains a .git "
> +   "directory (use 'rm -rf' if you really want "
> +   "to remove it including all of its history)"),
> + displaypath);
> +
> + if (!(flags & OPT_FORCE)) {
> + struct child_process cp_rm = CHILD_PROCESS_INIT;
> + cp_rm.git_cmd = 1;
> + argv_array_pushl(_rm.args, "rm", "-qn",
> +  path, NULL);
> +
> + if (run_command(_rm))
> + die(_("Submodule work tree '%s' contains local "
> +   "modifications; use '-f' to discard 
> them"),
> +   displaypath);
> + }
> +
> + strbuf_addstr(_rm, path);
> +
> + if (!remove_dir_recursively(_rm, 0))
> + format = _("Cleared directory '%s'\n");
> + else
> + format = _("Could not remove submodule work tree 
> '%s'\n");
> +
> + if (!(flags & OPT_QUIET))
> + printf(format, displaypath);
> +
> + strbuf_release(_rm);
> + }
> +
> + if (mkdir(path, 0777))
> + die_errno(_("could not create empty submodule directory %s"),
> +   displaypath);

If path was a directory (which presumably is the normal case) and
recursive removal fails (i.e. when the code says "Could not remove"),
this mkdir() would also fail with EEXIST.

In such a case, the original code did not die and instead continued
to remove the entries for the submodule from the configuration.
This "rewritten" version dies, leaving the stale configuration for
the submodule we failed to get rid of from the working tree.

I offhand do not know which one of these error case behaviours is
more useful; the user needs to do something (e.g. loosening the perm
in some paths in the submodule that prevented "rm -rf" from working
with "chmod u+w sub/some/path" and removing it manually) to recover
in either case, and cleaning as much as possible by removing the
configuration entries even when this mkdir() fails would probably be
a better behaviour, as long as the command as a whole exits with non
zero status to signal an error.

Re: [PATCH v2 0/2] Incremental rewrite of git-submodules

[Junio C Hamano]

Prathamesh Chavan  writes:

> Changes made to the previous version of the patch series[1]:
>
> * Since later on with certain patches, the number of bit-parameters to
>   be passed to a few functions depend on many parameters, I prefered
>   using a single flag bit.

I am not quite getting what you meant to say here.

> * Memory-leak of the variable 'remote' in the function:
>   print_default_remote() was avoided.

avoided how?  I am not quite getting what you meant to say here.

> * Additional condition were introduced while freeing the variables:
>   sub_origin_url and super_config_url.

As I said, I do not think the change goes into the right direction.

Re: [PATCH v2 1/2] submodule: port submodule subcommand 'sync' from shell to C

[Junio C Hamano]

Prathamesh Chavan  writes:

> + } else {
> + sub_origin_url = xstrdup(sub->url);
> + super_config_url = xstrdup(sub->url);
> + }
> + } else {
> + sub_origin_url = "";
> + super_config_url = "";
> + }
> + ...
> +cleanup:
> + if (strlen(super_config_url))
> + free(super_config_url);
> + if (strlen(sub_origin_url))
> + free(sub_origin_url);

The above is ugly and veriy likely to be wrong; imagine that
sub->url was an empty string to begin with.

Doing xstrdup("") before assigning the constant to *_url would be a
lot more sensible and maintainable solution for things like this.

Re: [PATCH v5 8/9] sequencer: try to commit without forking 'git commit'

[Johannes Schindelin]

Hi Phillip,

On Thu, 11 Jan 2018, Phillip Wood wrote:

> On 10/01/18 22:40, Johannes Schindelin wrote:
> > Hi,
> > 
> > On Wed, 10 Jan 2018, Jonathan Nieder wrote:
> > 
> >> that this causes the prepare-commit-msg hook not to be invoked, which
> >> I think is unintentional.  Should we check for such a hook and take
> >> the slowpath when it is present?
> > 
> > We could also easily recreate the functionality:
> > 
> > if (find_hook("pre-commit")) {
> > struct argv_array hook_env = ARGV_ARRAY_INIT;
> > 
> > argv_array_pushf(_env, "GIT_INDEX_FILE=%s",
> > get_index_file());
> > argv_array_push(_env, "GIT_EDITOR=:");
> > ret = run_hook_le(hook_env.argv, "pre-commit", NULL);
> > argv_array_clear(_env);
> > }
> 
> Thanks Johannes, though it needs to run the 'prepare-commit-msg' hook,
> the current code in master only runs the 'pre-commit' hook when we edit
> the message. I'll send a patch with a test.

Sorry, yes, that's the hook I meant ;-) the quoted text by Jonathan even
mentions it explicitly.

Ciao,
Johannes

Re: [PATCH] Documentation/git-worktree.txt: add missing `

[Junio C Hamano]

Ralf Thielow  writes:

>   of creating a new branch from HEAD, if there exists a tracking
> - branch in exactly one remote matching the basename of `,
> + branch in exactly one remote matching the basename of ``,
>   base the new branch on the remote-tracking branch, and mark
>   the remote-tracking branch as "upstream" from the new branch.

Thanks.

[PATCH v2 2/2] submodule: port submodule subcommand 'deinit' from shell to C

[Prathamesh Chavan]

The same mechanism is used even for porting this submodule
subcommand, as used in the ported subcommands till now.
The function cmd_deinit in split up after porting into four
functions: module_deinit(), for_each_listed_submodule(),
deinit_submodule() and deinit_submodule_cb().

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 147 
 git-submodule.sh|  55 +
 2 files changed, 148 insertions(+), 54 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index eb6f96981..b93e1d50b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -20,6 +20,7 @@
 #define OPT_QUIET (1 << 0)
 #define OPT_CACHED (1 << 1)
 #define OPT_RECURSIVE (1 << 2)
+#define OPT_FORCE (1 << 3)
 
 typedef void (*each_submodule_fn)(const struct cache_entry *list_item,
  void *cb_data);
@@ -911,6 +912,151 @@ static int module_sync(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+struct deinit_cb {
+   const char *prefix;
+   unsigned int flags;
+};
+#define DEINIT_CB_INIT { NULL, 0 }
+
+static void deinit_submodule(const char *path, const char *prefix,
+unsigned int flags)
+{
+   const struct submodule *sub;
+   char *displaypath = NULL;
+   struct child_process cp_config = CHILD_PROCESS_INIT;
+   struct strbuf sb_config = STRBUF_INIT;
+   char *sub_git_dir = xstrfmt("%s/.git", path);
+
+   sub = submodule_from_path(_oid, path);
+
+   if (!sub || !sub->name)
+   goto cleanup;
+
+   displaypath = get_submodule_displaypath(path, prefix);
+
+   /* remove the submodule work tree (unless the user already did it) */
+   if (is_directory(path)) {
+   struct strbuf sb_rm = STRBUF_INIT;
+   const char *format;
+
+   /*
+* protect submodules containing a .git directory
+* NEEDSWORK: instead of dying, automatically call
+* absorbgitdirs and (possibly) warn.
+*/
+   if (is_directory(sub_git_dir))
+   die(_("Submodule work tree '%s' contains a .git "
+ "directory (use 'rm -rf' if you really want "
+ "to remove it including all of its history)"),
+   displaypath);
+
+   if (!(flags & OPT_FORCE)) {
+   struct child_process cp_rm = CHILD_PROCESS_INIT;
+   cp_rm.git_cmd = 1;
+   argv_array_pushl(_rm.args, "rm", "-qn",
+path, NULL);
+
+   if (run_command(_rm))
+   die(_("Submodule work tree '%s' contains local "
+ "modifications; use '-f' to discard 
them"),
+ displaypath);
+   }
+
+   strbuf_addstr(_rm, path);
+
+   if (!remove_dir_recursively(_rm, 0))
+   format = _("Cleared directory '%s'\n");
+   else
+   format = _("Could not remove submodule work tree 
'%s'\n");
+
+   if (!(flags & OPT_QUIET))
+   printf(format, displaypath);
+
+   strbuf_release(_rm);
+   }
+
+   if (mkdir(path, 0777))
+   die_errno(_("could not create empty submodule directory %s"),
+ displaypath);
+
+   cp_config.git_cmd = 1;
+   argv_array_pushl(_config.args, "config", "--get-regexp", NULL);
+   argv_array_pushf(_config.args, "submodule.%s\\.", sub->name);
+
+   /* remove the .git/config entries (unless the user already did it) */
+   if (!capture_command(_config, _config, 0) && sb_config.len) {
+   char *sub_key = xstrfmt("submodule.%s", sub->name);
+   /*
+* remove the whole section so we have a clean state when
+* the user later decides to init this submodule again
+*/
+   git_config_rename_section_in_file(NULL, sub_key, NULL);
+   if (!(flags & OPT_QUIET))
+   printf(_("Submodule '%s' (%s) unregistered for path 
'%s'\n"),
+sub->name, sub->url, displaypath);
+   free(sub_key);
+   }
+
+cleanup:
+   free(displaypath);
+   free(sub_git_dir);
+   strbuf_release(_config);
+}
+
+static void deinit_submodule_cb(const struct cache_entry *list_item,
+   void *cb_data)
+{
+   struct deinit_cb *info = cb_data;
+   deinit_submodule(list_item->name, info->prefix, info->flags);
+}
+
+static int module_deinit(int argc, const char **argv, const char 

[PATCH v2 1/2] submodule: port submodule subcommand 'sync' from shell to C

[Prathamesh Chavan]

Port the submodule subcommand 'sync' from shell to C using the same
mechanism as that used for porting submodule subcommand 'status'.
Hence, here the function cmd_sync() is ported from shell to C.
This is done by introducing four functions: module_sync(),
sync_submodule(), sync_submodule_cb() and print_default_remote().

The function print_default_remote() is introduced for getting
the default remote as stdout.

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 195 
 git-submodule.sh|  57 +
 2 files changed, 196 insertions(+), 56 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a5c4a8a69..eb6f96981 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -50,6 +50,20 @@ static char *get_default_remote(void)
return ret;
 }
 
+static int print_default_remote(int argc, const char **argv, const char 
*prefix)
+{
+   const char *remote;
+
+   if (argc != 1)
+   die(_("submodule--helper print-default-remote takes no 
arguments"));
+
+   remote = get_default_remote();
+   if (remote)
+   printf("%s\n", remote);
+
+   return 0;
+}
+
 static int starts_with_dot_slash(const char *str)
 {
return str[0] == '.' && is_dir_sep(str[1]);
@@ -358,6 +372,25 @@ static void module_list_active(struct module_list *list)
*list = active_modules;
 }
 
+static char *get_up_path(const char *path)
+{
+   int i;
+   struct strbuf sb = STRBUF_INIT;
+
+   for (i = count_slashes(path); i; i--)
+   strbuf_addstr(, "../");
+
+   /*
+* Check if 'path' ends with slash or not
+* for having the same output for dir/sub_dir
+* and dir/sub_dir/
+*/
+   if (!is_dir_sep(path[strlen(path) - 1]))
+   strbuf_addstr(, "../");
+
+   return strbuf_detach(, NULL);
+}
+
 static int module_list(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -718,6 +751,166 @@ static int module_name(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+struct sync_cb {
+   const char *prefix;
+   unsigned int flags;
+};
+
+#define SYNC_CB_INIT { NULL, 0 }
+
+static void sync_submodule(const char *path, const char *prefix,
+  unsigned int flags)
+{
+   const struct submodule *sub;
+   char *remote_key = NULL;
+   char *sub_origin_url, *super_config_url, *displaypath;
+   struct strbuf sb = STRBUF_INIT;
+   struct child_process cp = CHILD_PROCESS_INIT;
+   char *sub_config_path = NULL;
+
+   if (!is_submodule_active(the_repository, path))
+   return;
+
+   sub = submodule_from_path(_oid, path);
+
+   if (sub && sub->url) {
+   if (starts_with_dot_dot_slash(sub->url) ||
+   starts_with_dot_slash(sub->url)) {
+   char *remote_url, *up_path;
+   char *remote = get_default_remote();
+   strbuf_addf(, "remote.%s.url", remote);
+
+   if (git_config_get_string(sb.buf, _url))
+   remote_url = xgetcwd();
+
+   up_path = get_up_path(path);
+   sub_origin_url = relative_url(remote_url, sub->url, 
up_path);
+   super_config_url = relative_url(remote_url, sub->url, 
NULL);
+
+   free(remote);
+   free(up_path);
+   free(remote_url);
+   } else {
+   sub_origin_url = xstrdup(sub->url);
+   super_config_url = xstrdup(sub->url);
+   }
+   } else {
+   sub_origin_url = "";
+   super_config_url = "";
+   }
+
+   displaypath = get_submodule_displaypath(path, prefix);
+
+   if (!(flags & OPT_QUIET))
+   printf(_("Synchronizing submodule url for '%s'\n"),
+displaypath);
+
+   strbuf_reset();
+   strbuf_addf(, "submodule.%s.url", sub->name);
+   if (git_config_set_gently(sb.buf, super_config_url))
+   die(_("failed to register url for submodule path '%s'"),
+ displaypath);
+
+   if (!is_submodule_populated_gently(path, NULL))
+   goto cleanup;
+
+   prepare_submodule_repo_env(_array);
+   cp.git_cmd = 1;
+   cp.dir = path;
+   argv_array_pushl(, "submodule--helper",
+"print-default-remote", NULL);
+
+   strbuf_reset();
+   if (capture_command(, , 0))
+   die(_("failed to get the default remote for submodule '%s'"),
+ path);
+
+   strbuf_strip_suffix(, "\n");
+   remote_key = xstrfmt("remote.%s.url", sb.buf);
+
+   

[PATCH v2 0/2] Incremental rewrite of git-submodules

[Prathamesh Chavan]

Changes made to the previous version of the patch series[1]:

* Since later on with certain patches, the number of bit-parameters to
  be passed to a few functions depend on many parameters, I prefered
  using a single flag bit.

* Memory-leak of the variable 'remote' in the function:
  print_default_remote() was avoided.

* Additional condition were introduced while freeing the variables:
  sub_origin_url and super_config_url.

* print messages and comments in the deinit_submodule function were
  corrected as suggested in previous review of this patch[2].

* Call to the function lstat() for identifying the directory mode was
  avoided and instead 0777 was used. An additional improvement is to be
  made over this patch, but since the improvement can not directly be
  part of the "rewirte in C", the patch would be floated saperately on
  the mailing list.

As before you can find this series at:
https://github.com/pratham-pc/git/commits/patch-series-2

And its build report is available at:
https://travis-ci.org/pratham-pc/git/builds/
Branch: patch-series-2
Build #196

[1]: https://public-inbox.org/git/20180109175703.4793-1-pc44...@gmail.com/ 
[2]: https://public-inbox.org/git/xmqq7esq4tf6@gitster.mtv.corp.google.com/

Prathamesh Chavan (2):
  submodule: port submodule subcommand 'sync' from shell to C
  submodule: port submodule subcommand 'deinit' from shell to C

 builtin/submodule--helper.c | 342 
 git-submodule.sh| 112 +--
 2 files changed, 344 insertions(+), 110 deletions(-)

-- 
2.15.1

hi Git

[rburgstaler]

Good morning Git

https://goo.gl/NUAiB7


rburgstaler

Re: [PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Occasionally submodule code could execute new commands with GIT_DIR set
> to some submodule. GIT_TRACE prints just the command line which makes it
> hard to tell that it's not really executed on this repository.
>
> Print modified env variables (compared to parent environment) in this
> case. Actually only modified or new variables are printed. Variable
> deletion is suppressed because ...

When I read the above, I imagined that you are reducing noise from
the output by showing only modified or new variables, but it appears
that the implementation of concatenate_env() just blindly copies the
variables without checking if they are setting the same value.

I wonder how common it would be to attempt exporting a variable with
the same value, and to attempt unsetting a variable that is not
exported, which might help you reduce the clutter by hiding stuff
that truly do not matter, while keeping what matters (possibly
including the "unset" part).

> +static void concatenate_env(struct strbuf *dst, const char *const *env)
> +{
> + int i;
> +
> + /* Copy into destination buffer. */
> + strbuf_grow(dst, 255);
> + for (i = 0; env[i]; ++i) {
> + /*
> +  * the main interesting is setting new vars

I'll do s/interesting/& part/ while queuing.

> +  * e.g. GIT_DIR, ignore the unsetting to reduce noise.
> +  */
> + if (!strchr(env[i], '='))
> + continue;
> + strbuf_addch(dst, ' ');
> + sq_quote_buf(dst, env[i]);

I think you'd tweak the quoting around here in a later iteration,
based on the distinction between the two:

$ 'GIT_DIR=.git' git foo
$ GIT_DIR='.git' git foo

that was pointed out in a near-by message.

Thanks.

Re: [PATCH] doc/read-tree: remove obsolete remark

[Junio C Hamano]

Jeff King  writes:

> On Tue, Jan 09, 2018 at 04:30:34PM +0100, Andreas G. Schacker wrote:
>
>> Earlier versions of `git read-tree` required the `--prefix` option value
>> to end with a slash. This restriction was eventually lifted without a
>> corresponding amendment to the documentation.
>
> Makes sense.
>
>> ---prefix=/::
>> +--prefix=::
>>  Keep the current index contents, and read the contents
>>  of the named tree-ish under the directory at ``.
>>  The command will refuse to overwrite entries that already
>> -existed in the original index file. Note that the `/`
>> -value must end with a slash.
>> +existed in the original index file.
>
> Is it worth mentioning in the new world order that the slash is not
> implied? I.e., that you probably do want to say "--prefix=foo/" if you
> want the subdirectory "foo", but do not want to match "foobar"?

Doesn't "git read-tree --prefix=previous HEAD^" add paths like
"previous/Documentation/Makefile" to the index, i.e. instead of
forcing you to have the required slash at the end, we give one for
free when it is missing?

Re: [PATCH v2 4/9] object: add clear_commit_marks_all()

[René Scharfe]

Am 10.01.2018 um 08:58 schrieb Jeff King:
> On Mon, Dec 25, 2017 at 06:44:58PM +0100, René Scharfe wrote:
> 
>> Add a function for clearing the commit marks of all in-core commit
>> objects.  It's similar to clear_object_flags(), but more precise, since
>> it leaves the other object types alone.  It still has to iterate through
>> them, though.
> 
> Makes sense.
> 
> Is it worth having:
> 
>void clear_object_flags_from_type(int type, unsigned flags);
> 
> rather than having two near-identical functions? I guess we'd need some
> way of saying "all types" to reimplement clear_object_flags() as a
> wrapper (OBJ_NONE, I guess?).

I don't know if there is a demand.  Perhaps the two callers of
clear_object_flags() should be switched to clear_commit_marks_all()?
They look like they only care about commits as well.  Or is it safe to
stomp over the flags of objects of other types?  Then we'd only need
to keep clear_object_flags()..

> The run-time check is maybe a little bit slower in the middle of a tight
> loop, but I'm not sure it would matter much (I'd actually be curious if
> this approach is faster than the existing traversal code, too).

I don't know how to measure this properly.  With 100 runs each I get
this for the git repo and the silly test program below, which measures
the duration of the respective function call:

   meanstddev method
   --- -- --
   5.89763e+06 613106 clear_commit_marks
   2.72572e+06 507689 clear_commit_marks_all
   1.96582e+06 494753 clear_object_flags

So these are noisy numbers, but kind of in the expected range.

René

---
 Makefile   |  1 +
 t/helper/test-clear-commit-marks.c | 67 ++
 2 files changed, 68 insertions(+)
 create mode 100644 t/helper/test-clear-commit-marks.c

diff --git a/Makefile b/Makefile
index 1a9b23b679..7db2c6ca7f 100644
--- a/Makefile
+++ b/Makefile
@@ -648,6 +648,7 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-clear-commit-marks
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-config
 TEST_PROGRAMS_NEED_X += test-date
diff --git a/t/helper/test-clear-commit-marks.c 
b/t/helper/test-clear-commit-marks.c
new file mode 100644
index 00..296ca0286f
--- /dev/null
+++ b/t/helper/test-clear-commit-marks.c
@@ -0,0 +1,67 @@
+#include "cache.h"
+#include "commit.h"
+#include "revision.h"
+
+static void start(struct timespec *start)
+{
+   clock_gettime(CLOCK_MONOTONIC, start);
+}
+
+static void print_duration(struct timespec *start)
+{
+   struct timespec end;
+   uint64_t d;
+   clock_gettime(CLOCK_MONOTONIC, );
+   d = end.tv_sec - start->tv_sec;
+   d *= 10;
+   d += end.tv_nsec - start->tv_nsec;
+   printf("%lu", d);
+}
+
+int cmd_main(int argc, const char **argv)
+{
+   struct rev_info revs;
+   struct object_id oid;
+   struct commit *commit;
+   struct timespec ts;
+
+   setup_git_directory();
+
+   if (get_oid("HEAD", ))
+   die("No HEAD?");
+   commit = lookup_commit();
+   if (!commit)
+   die("HEAD is not a committish?");
+
+   init_revisions(, NULL);
+   add_pending_object(, >object, "HEAD");
+   if (prepare_revision_walk())
+   die("revision walk setup failed");
+   while (get_revision())
+   ; /* nothing */
+
+   if (argc != 2)
+   return 0;
+
+   if (!strcmp(argv[1], "clear_commit_marks")) {
+   start();
+   clear_commit_marks(commit, ALL_REV_FLAGS);
+   print_duration();
+   }
+
+   if (!strcmp(argv[1], "clear_commit_marks_all")) {
+   start();
+   clear_commit_marks_all(ALL_REV_FLAGS);
+   print_duration();
+   }
+
+   if (!strcmp(argv[1], "clear_object_flags")) {
+   start();
+   clear_object_flags(ALL_REV_FLAGS);
+   print_duration();
+   }
+
+   printf(" %s\n", argv[1]);
+
+   return 0;
+}
-- 
2.15.1

Re: [PATCH v2 5/9] bisect: avoid using the rev_info flag leak_pending

[René Scharfe]

Am 10.01.2018 um 09:07 schrieb Jeff King:
> On Mon, Dec 25, 2017 at 06:45:36PM +0100, René Scharfe wrote:
> 
>> The leak_pending flag is so awkward to use that multiple comments had to
>> be added around each occurrence.  We only use it for remembering the
>> commits whose marks we have to clear after checking if all of the good
>> ones are ancestors of the bad one.  This is easy, though: We need to do
>> that for the bad and good commits, of course.
> 
> Are we sure that our list is the same as what is traversed? I won't be
> surprised if it is true, but it doesn't seem immediately obvious from
> the code:
> 
>> -static int check_ancestors(const char *prefix)
>> +static int check_ancestors(int rev_nr, struct commit **rev, const char 
>> *prefix)
>>   {
> 
> So now we take in a set of objects...
> 
>>  struct rev_info revs;
>> -struct object_array pending_copy;
>>  int res;
>>   
>>  bisect_rev_setup(, prefix, "^%s", "%s", 0);
> 
> But those objects aren't provided here. bisect_rev_setup() puts its own
> set of objects into the pending list...

Yes, namely from the global variables current_bad_oid and good_revs.

>> -/* Save pending objects, so they can be cleaned up later. */
>> -pending_copy = revs.pending;
>> -revs.leak_pending = 1;
>> -
>> -/*
>> - * bisect_common calls prepare_revision_walk right away, which
>> - * (together with .leak_pending = 1) makes us the sole owner of
>> - * the list of pending objects.
>> - */
>>  bisect_common();
>>  res = (revs.commits != NULL);
> 
> And then we traverse, and then...
> 
>>   
>>  /* Clean up objects used, as they will be reused. */
>> -clear_commit_marks_for_object_array(_copy, ALL_REV_FLAGS);
>> -
>> -object_array_clear(_copy);
>> +clear_commit_marks_many(rev_nr, rev, ALL_REV_FLAGS);
> 
> ...this is the first time we look at "rev".

... which is populated by get_bad_and_good_commits() using the global
variables current_bad_oid and good_revs.

> If we already have the list of tips, could we just feed it ourselves to
> bisect_rev_setup (I think that would require us remembering which were
> "good" and "bad", but that doesn't seem like a big deal).

That's done already under the covers.  De-globalizing these variables
would make this visible.

Another way would be to store the bad and good revs in a format that
allows them to be used everywhere, thus avoiding confusing
duplication/conversions.  Commit pointers and arrays thereof should
work everywhere we currently use object_ids and oid_arrays for bad
and good revs, right?

René

Re: [PATCH] run-command.c: print env vars when GIT_TRACE is set

[Johannes Sixt]

Am 11.01.2018 um 11:07 schrieb Jeff King:

The output for a single command is pretty shell-like due to the quoting:

   $ GIT_TRACE=1 ./git upload-pack . >/dev/null
   [...]run_command: 'git-upload-pack' '.'

You could copy and paste that to a shell if you wanted.  And with
environment variables, that remains so:

   $ GIT_TRACE=1 ./git ls-remote https://github.com/git/git >/dev/null
   [...]run_command: 'GIT_DIR=.git' 'git-remote-https' 'https://[...]'


Not quite, though. For variable assignments to be recognized as such, 
the name and equal sign must not be quoted:


  GIT_DIR='.git' 'git-remote-https' 'https://[...]'

-- Hannes

Re: [PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Stefan Beller]

On Thu, Jan 11, 2018 at 9:53 AM, Brandon Williams  wrote:
> On 01/11, Nguyễn Thái Ngọc Duy wrote:
>> Occasionally submodule code could execute new commands with GIT_DIR set
>> to some submodule. GIT_TRACE prints just the command line which makes it
>> hard to tell that it's not really executed on this repository.
>>
>> Print modified env variables (compared to parent environment) in this
>> case. Actually only modified or new variables are printed. Variable
>> deletion is suppressed because they are often used to clean up
>> repo-specific variables that git passes around between processes. When
>> submodule code executes commands on another repo, it clears all these
>> variables, _many_ of these, that make the output really noisy.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  v2 fixes up commit message to clarify about "env delta" and why var
>>  deletion is not printed.
>>
>>  It also keeps child_process tracing in one place/function, this
>>  should make it easier to trace more e.g. cwd and stuff.
>>
>>  Though, Stefan, while i'm not opposed to trace every single setting
>>  in child_process, including variable deletion, cwd and even more, it
>>  may be not that often needed for a "casual" developer.
>>
>>  I suggest we have something like $GIT_TRACE_EXEC instead that could
>>  be super verbose when we need it and leave $GIT_TRACE with a
>>  reasonable subset.

Makes sense. Thanks for working on this!
Code msg look good to me. I agree with Brandon on the
comments grammar to have a missing piece.

Thanks,
Stefan

[PATCH] Documentation/git-worktree.txt: add missing `

[Ralf Thielow]

Signed-off-by: Ralf Thielow 
---
 Documentation/git-worktree.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index f850e8ffb..41585f535 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -118,7 +118,7 @@ OPTIONS
 --[no-]guess-remote::
With `worktree add `, without ``, instead
of creating a new branch from HEAD, if there exists a tracking
-   branch in exactly one remote matching the basename of `,
+   branch in exactly one remote matching the basename of ``,
base the new branch on the remote-tracking branch, and mark
the remote-tracking branch as "upstream" from the new branch.
 +
-- 
2.16.0.rc1.290.gc44db26fe

Re: Unable to de-init stubborn submodule

[Stefan Beller]

On Thu, Jan 11, 2018 at 6:12 AM, Ævar Arnfjörð Bjarmason
 wrote:
> What if we wanted to drop sha1collisiondetection/ as a submodule and
> replace it with a copy of what's now in sha1dc/? I ran into this with
> another project, but here's a way to reproduce it on git.git:
>
> (
> rm -rf /tmp/git &&
> git clone g...@github.com:git/git.git /tmp/git
> cd /tmp/git &&
> git tag nuke-before &&
> git submodule update --init &&

At this point
$ tail -n 3 .git/config
[submodule "sha1collisiondetection"]
active = true
url = https://github.com/cr-marcstevens/sha1collisiondetection.git


> git rm -r .gitmodules sha1collisiondetection &&

no need to delete the .gitmodules here. Git detects you're deleting a submodule
and adjusts the .gitmodules file (it is empty after just "git rm sha1coll...")

> git commit -m"Nuke sha1dc submodule" &&
> cp -Rvp sha1dc sha1collisiondetection &&
> git add sha1collisiondetection &&
> git commit -m"Now it's not a submodule" &&
> git tag nuke-after &&
> git reset --hard nuke-before &&

As bmwill said, you may want to reset with --recurse-submodules here,
"git ls-tree HEAD |grep sha1c" will show commit and "git status" thinks
everything is fine on disk. I have no suspicion to believe otherwise.
But we digress, you chose to not use that flag.

> git submodule update --init && # skip this and the below won't fail
> git reset --hard nuke-after && # Emulate someone doing a pull

$ git reset --hard nuke-after
warning: unable to rmdir 'sha1collisiondetection': Directory not empty
HEAD is now at f683a1b81 Now it's not a submodule
$ git reset --hard nuke-after --recurse-submodules
HEAD is now at f683a1b81 Now it's not a submodule


> git ls-tree HEAD | grep sha1collisiondetection && # OK, shows "tree" 
> not "commit"
> test $(git rev-parse HEAD) == $(git -C sha1collisiondetection/ log -1 
> --pretty=format:%H) && echo OK || echo WTF
> )
>
> This results in a really bizarre state where according to ls-tree
> sha1collisiondetection is a tree at the current commit:
>
> 04 tree 81583289d96bdde4b366c243ab524ea28d895ea5
> sha1collisiondetection
>
> But git still believes there's a submodule there for some reason, and
> shows the log for the upstream sha1collisiondetection project:
>
> git -C sha1collisiondetection/ log -1

(A)
That is because the reset without flag do touch submodules kept the
submodule in place and the -C in this command tells git to cd into that
directory, (which is the submodule, an 'independent' repo) and shows the
log of said repo.

> commit 19d97bf (HEAD, origin/master, origin/HEAD, master)
> Merge: 3f14d1b c93f0b4
> Author: Dan Shumow 
> Date:   Sat Jul 1 12:36:15 2017 -0700
>
> Merge pull request #37 from avar/fixup-pull-request-34
>
> Fix endian detection logic for Sparc, little endian BSD etc.
>
> Doing:
>
> git submodule deinit sha1collisiondetection

Doing this after you reset to nuke-after, there are no submodules from
the superprojects point of view, hence no submodule is touched/modified. :/
(There just happens to be a stray repository at a place where we'd want
to have a tree).

> Does nothing to help, then I thought it might be:
>
> git config -f .git/config -l|grep ^submodule
> submodule.sha1collisiondetection.active=true
> 
> submodule.sha1collisiondetection.url=https://github.com/cr-marcstevens/sha1collisiondetection.git
>
> But running:
>
> git config --remove-section submodule.sha1collisiondetection

This made the submodule not active any more, (note that at the current
tree there
is no submodule to begin with... so what effect to we want here?)

> Doesn't help either, neither does removing the index:
>
> rm .git/index &&
> git reset --hard
>
> If you then do:
>
> rm -rf .git/modules

Getting out the big hammer, eh?

instead of deleting the submodules git repo, remove its worktree:

rm -rf sha1collisiondetection

(This is best done at time (A) above)

>
> You'll get this error:
>
> git -C sha1collisiondetection/ log -1
> fatal: Not a git repository: 
> /tmp/git/sha1collisiondetection/../.git/modules/sha1collisiondetection
>
> But I can't see what's still referencing it.

$ls -la /tmp/git/sha1collisiondetection
...
-rw-r--r--  1 sbeller eng47 Jan 11 10:02 .git
...
$ cat sha1collisiondetection/.git
gitdir: ../.git/modules/sha1collisiondetection

This is a crucial file, telling you there is a repo with the worktree
at the place where
.git lives and the git dir is at the location indicated (which you removed)

> This problem is avoided if, as noted with a comment I skip:
>
> git submodule update --init
>
> But I shouldn't need to remember to de-init a submodule before moving to
> a new commit that doesn't have it, least I end up in some seemingly
> unrecoverable state.
>
> Am I 

[ANNOUNCE] Git London User Group: 16 January 2018

[Edward Thomson]

Git London User Group: 16 January, 2018
===

I'm pleased to announce the formation of the Git London User Group, where
Git users and experts from throughout the UK can get together to share
tips, experience and assistance for using Git successfully.

The first meeting takes place Tuesday, 16 January, 2018 at 19:00.

Agenda
--
Extending Git through Scripting: Charles Bailey

Git is the most popular version control system in use today; it is highly
flexible and supports many different workflows.  One of its strengths is
its openness to scripting.  This talk looks at the basic principles that
support best practice for scripting Git and how to avoid some common
pitfalls.

Building Git Tools with libgit2: Edward Thomson

Edward introduces the libgit2 framework (http://libgit2.org), which is a
portable, implementation of Git as a library.  If you're looking for more
advanced programmatic access to working with Git repositories, libgit2 is
a good option, which is why it's used by many Git servers like GitHub and
VSTS and clients like gmaster and GitKraken.  Edward will introduce libgit2
and some of the language bindings like LibGit2Sharp (for .NET) and Rugged
(for Ruby).

Location

General Assembly.  The Relay Building, 1st floor.
114 Whitechapel High Street London, E1 7PT.

We ask that you please RSVP at http://londongit.org/.

Sponsors

A big thank you to the sponsors of the Git London User Group.  Bloomberg
has been kind enough to sponsor the meeting space for us to use.  Microsoft
has sponsored food for dinner.  And All Things Git (the Podcast about Git)
has sponsored meetup and registration fees.

Thanks also to Henry Kleynhans, the other organizer of the group.

Contact
---
Follow us on Meetup by visiting http://londongit.org/, or on Twitter at
@londongit.

If you're in or around London, we hope that you'll join us next Tuesday!

Sincerely,
Edward Thomson (ethom...@edwardthomson.com)

Re: [PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Brandon Williams]

On 01/11, Nguyễn Thái Ngọc Duy wrote:
> Occasionally submodule code could execute new commands with GIT_DIR set
> to some submodule. GIT_TRACE prints just the command line which makes it
> hard to tell that it's not really executed on this repository.
> 
> Print modified env variables (compared to parent environment) in this
> case. Actually only modified or new variables are printed. Variable
> deletion is suppressed because they are often used to clean up
> repo-specific variables that git passes around between processes. When
> submodule code executes commands on another repo, it clears all these
> variables, _many_ of these, that make the output really noisy.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  v2 fixes up commit message to clarify about "env delta" and why var
>  deletion is not printed.
> 
>  It also keeps child_process tracing in one place/function, this
>  should make it easier to trace more e.g. cwd and stuff.
> 
>  Though, Stefan, while i'm not opposed to trace every single setting
>  in child_process, including variable deletion, cwd and even more, it
>  may be not that often needed for a "casual" developer.
>  
>  I suggest we have something like $GIT_TRACE_EXEC instead that could
>  be super verbose when we need it and leave $GIT_TRACE with a
>  reasonable subset.
> 
>  run-command.c |  3 ++-
>  trace.c   | 39 +++
>  trace.h   |  3 +++
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/run-command.c b/run-command.c
> index 31fc5ea86e..002074b128 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -624,7 +624,8 @@ int start_command(struct child_process *cmd)
>   cmd->err = fderr[0];
>   }
>  
> - trace_argv_printf(cmd->argv, "trace: run_command:");
> + trace_run_command(cmd);
> +
>   fflush(NULL);
>  
>  #ifndef GIT_WINDOWS_NATIVE
> diff --git a/trace.c b/trace.c
> index b7530b51a9..e5e46e2a09 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -23,6 +23,7 @@
>  
>  #include "cache.h"
>  #include "quote.h"
> +#include "run-command.h"
>  
>  struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
>  struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
> @@ -272,6 +273,44 @@ void trace_performance_fl(const char *file, int line, 
> uint64_t nanos,
>  #endif /* HAVE_VARIADIC_MACROS */
>  
>  
> +static void concatenate_env(struct strbuf *dst, const char *const *env)
> +{
> + int i;
> +
> + /* Copy into destination buffer. */
> + strbuf_grow(dst, 255);
> + for (i = 0; env[i]; ++i) {
> + /*
> +  * the main interesting is setting new vars
> +  * e.g. GIT_DIR, ignore the unsetting to reduce noise.
> +  */

Patch looks good to me! Only nit is this comment which reads funny which i
pointed out in v1.

> + if (!strchr(env[i], '='))
> + continue;
> + strbuf_addch(dst, ' ');
> + sq_quote_buf(dst, env[i]);
> + }
> +}
> +
> +void trace_run_command(const struct child_process *cp)
> +{
> + struct strbuf buf = STRBUF_INIT;
> +
> + if (!prepare_trace_line(NULL, 0, _default_key, ))
> + return;
> +
> + strbuf_addf(, "trace: run_command:");
> +
> + /*
> +  * caller is responsible for setting this if the main source
> +  * is actually in cp->env_array
> +  */
> + if (cp->env)
> + concatenate_env(, cp->env);
> +
> + sq_quote_argv(, cp->argv, 0);
> + print_trace_line(_default_key, );
> +}
> +
>  static const char *quote_crnl(const char *path)
>  {
>   static struct strbuf new_path = STRBUF_INIT;
> diff --git a/trace.h b/trace.h
> index 88055abef7..e54c687f26 100644
> --- a/trace.h
> +++ b/trace.h
> @@ -4,6 +4,8 @@
>  #include "git-compat-util.h"
>  #include "strbuf.h"
>  
> +struct child_process;
> +
>  struct trace_key {
>   const char * const key;
>   int fd;
> @@ -17,6 +19,7 @@ extern struct trace_key trace_default_key;
>  extern struct trace_key trace_perf_key;
>  
>  extern void trace_repo_setup(const char *prefix);
> +extern void trace_run_command(const struct child_process *cp);
>  extern int trace_want(struct trace_key *key);
>  extern void trace_disable(struct trace_key *key);
>  extern uint64_t getnanotime(void);
> -- 
> 2.15.1.600.g899a5f85c6
> 

-- 
Brandon Williams

Re: Unable to de-init stubborn submodule

[Brandon Williams]

On 01/11, Ævar Arnfjörð Bjarmason wrote:
> What if we wanted to drop sha1collisiondetection/ as a submodule and
> replace it with a copy of what's now in sha1dc/? I ran into this with
> another project, but here's a way to reproduce it on git.git:
> 
> (
> rm -rf /tmp/git &&
> git clone g...@github.com:git/git.git /tmp/git
> cd /tmp/git &&
> git tag nuke-before &&
> git submodule update --init &&
> git rm -r .gitmodules sha1collisiondetection &&
> git commit -m"Nuke sha1dc submodule" &&
> cp -Rvp sha1dc sha1collisiondetection &&
> git add sha1collisiondetection &&
> git commit -m"Now it's not a submodule" &&
> git tag nuke-after &&
> git reset --hard nuke-before &&
> git submodule update --init && # skip this and the below won't fail
> git reset --hard nuke-after && # Emulate someone doing a pull
> git ls-tree HEAD | grep sha1collisiondetection && # OK, shows "tree" 
> not "commit"
> test $(git rev-parse HEAD) == $(git -C sha1collisiondetection/ log -1 
> --pretty=format:%H) && echo OK || echo WTF
> )
> 
> This results in a really bizarre state where according to ls-tree
> sha1collisiondetection is a tree at the current commit:
> 
> 04 tree 81583289d96bdde4b366c243ab524ea28d895ea5
> sha1collisiondetection
> 
> But git still believes there's a submodule there for some reason, and
> shows the log for the upstream sha1collisiondetection project:
> 
> git -C sha1collisiondetection/ log -1
> commit 19d97bf (HEAD, origin/master, origin/HEAD, master)
> Merge: 3f14d1b c93f0b4
> Author: Dan Shumow 
> Date:   Sat Jul 1 12:36:15 2017 -0700
> 
> Merge pull request #37 from avar/fixup-pull-request-34
> 
> Fix endian detection logic for Sparc, little endian BSD etc.
> 
> Doing:
> 
> git submodule deinit sha1collisiondetection
> 
> Does nothing to help, then I thought it might be:
> 
> git config -f .git/config -l|grep ^submodule
> submodule.sha1collisiondetection.active=true
> 
> submodule.sha1collisiondetection.url=https://github.com/cr-marcstevens/sha1collisiondetection.git
> 
> But running:
> 
> git config --remove-section submodule.sha1collisiondetection
> 
> Doesn't help either, neither does removing the index:
> 
> rm .git/index &&
> git reset --hard
> 
> If you then do:
> 
> rm -rf .git/modules
> 
> You'll get this error:
> 
> git -C sha1collisiondetection/ log -1
> fatal: Not a git repository: 
> /tmp/git/sha1collisiondetection/../.git/modules/sha1collisiondetection
> 
> But I can't see what's still referencing it.
> 
> This problem is avoided if, as noted with a comment I skip:
> 
> git submodule update --init
> 
> But I shouldn't need to remember to de-init a submodule before moving to
> a new commit that doesn't have it, least I end up in some seemingly
> unrecoverable state.
> 
> Am I missing something obvious here?

One thing you could try is adding --recurse-submodule flags to the reset
commands.  IIRC reset ignores submodules unless you specify that flag.

-- 
Brandon Williams

[PATCH] l10n: de.po: translate 72 new messages

[Ralf Thielow]

Translate 72 new messages came from git.pot update in 18a907225 (l10n:
git.pot: v2.16.0 round 1 (64 new, 25 removed)) and 005c62fe4 (l10n:
git.pot: v2.16.0 round 2 (8 new, 4 removed)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 227 +++
 1 file changed, 98 insertions(+), 129 deletions(-)

diff --git a/po/de.po b/po/de.po
index b24b28875..6f04621b9 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1716,7 +1716,7 @@ msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' 
migrieren."
 #: editor.c:61
 #, c-format
 msgid "hint: Waiting for your editor to close the file...%c"
-msgstr ""
+msgstr "Hinweis: Warte auf das Schließen der Datei durch Ihren Editor...%c"
 
 #: entry.c:177
 msgid "Filtering content"
@@ -2087,12 +2087,12 @@ msgstr "Ungültiges Datumsformat: %s"
 
 #: list-objects-filter-options.c:30
 msgid "multiple object filter types cannot be combined"
-msgstr ""
+msgstr "Mehrere Arten von Objekt-Filtern können nicht kombiniert werden."
 
 #: list-objects-filter-options.c:41 list-objects-filter-options.c:68
-#, fuzzy, c-format
+#, c-format
 msgid "invalid filter-spec expression '%s'"
-msgstr "Ungültige Datei: '%s'"
+msgstr "Ungültiger filter-spec Ausdruck '%s'."
 
 #: lockfile.c:151
 #, c-format
@@ -2356,9 +2356,9 @@ msgid "Adding %s"
 msgstr "Füge %s hinzu"
 
 #: merge-recursive.c:1958
-#, fuzzy, c-format
+#, c-format
 msgid "Dirty index: cannot merge (dirty: %s)"
-msgstr "Geänderter Index: kann Patches nicht anwenden (geändert: %s)"
+msgstr "Geänderter Index: kann nicht mergen (geändert: %s)"
 
 #: merge-recursive.c:1962
 msgid "Already up to date!"
@@ -3015,6 +3015,8 @@ msgid ""
 "The '%s' hook was ignored because it's not set as executable.\n"
 "You can disable this warning with `git config advice.ignoredHook false`."
 msgstr ""
+"Der '%s' Hook wurde ignoriert, weil er nicht als ausführbar markiert ist.\n"
+"Sie können diese Warnung mit `git config advice.ignoredHook false` 
deaktivieren."
 
 #: send-pack.c:141
 #, c-format
@@ -3137,14 +3139,12 @@ msgid "%s: Unable to write new index file"
 msgstr "%s: Konnte neue Index-Datei nicht schreiben"
 
 #: sequencer.c:496
-#, fuzzy
 msgid "could not resolve HEAD commit"
-msgstr "Konnte HEAD-Commit nicht auflösen\n"
+msgstr "Konnte HEAD-Commit nicht auflösen."
 
 #: sequencer.c:516
-#, fuzzy
 msgid "unable to update cache tree"
-msgstr "Konnte Cache-Verzeichnis nicht aktualisieren\n"
+msgstr "Konnte Cache-Verzeichnis nicht aktualisieren."
 
 #: sequencer.c:600
 #, c-format
@@ -3178,14 +3178,14 @@ msgstr ""
 "  git rebase --continue\n"
 
 #: sequencer.c:702
-#, fuzzy, c-format
+#, c-format
 msgid "could not parse commit %s"
-msgstr "Konnte Commit %s nicht parsen\n"
+msgstr "Konnte Commit %s nicht parsen."
 
 #: sequencer.c:707
-#, fuzzy, c-format
+#, c-format
 msgid "could not parse parent commit %s"
-msgstr "Konnte Eltern-Commit %s nicht parsen\n"
+msgstr "Konnte Eltern-Commit %s nicht parsen."
 
 #: sequencer.c:836
 #, c-format
@@ -3316,14 +3316,14 @@ msgid "git %s: failed to refresh the index"
 msgstr "git %s: Fehler beim Aktualisieren des Index"
 
 #: sequencer.c:1270
-#, fuzzy, c-format
+#, c-format
 msgid "%s does not accept arguments: '%s'"
-msgstr "%%(body) akzeptiert keine Argumente"
+msgstr "%s akzeptiert keine Argumente: '%s'"
 
 #: sequencer.c:1279
-#, fuzzy, c-format
+#, c-format
 msgid "missing arguments for %s"
-msgstr "Objekt %s fehlt für %s"
+msgstr "Fehlende Argumente für %s."
 
 #: sequencer.c:1322
 #, c-format
@@ -4965,7 +4965,7 @@ msgstr "versionierte Dateien aktualisieren"
 
 #: builtin/add.c:299
 msgid "renormalize EOL of tracked files (implies -u)"
-msgstr ""
+msgstr "erneutes Normalisieren der Zeilenenden von versionierten Dateien 
(impliziert -u)"
 
 #: builtin/add.c:300
 msgid "record only the fact that the path will be added later"
@@ -5507,36 +5507,34 @@ msgstr "git bisect--helper --next-all [--no-checkout]"
 
 #: builtin/bisect--helper.c:13
 msgid "git bisect--helper --write-terms  "
-msgstr ""
+msgstr "git bisect--helper --write-terms  "
 
 #: builtin/bisect--helper.c:14
-#, fuzzy
 msgid "git bisect--helper --bisect-clean-state"
-msgstr "git bisect--helper --next-all [--no-checkout]"
+msgstr "git bisect--helper --bisect-clean-state"
 
 #: builtin/bisect--helper.c:46
-#, fuzzy, c-format
+#, c-format
 msgid "'%s' is not a valid term"
-msgstr "'%s' ist keine gültige Referenz."
+msgstr "'%s' ist kein gültiger Begriff."
 
 #: builtin/bisect--helper.c:50
-#, fuzzy, c-format
+#, c-format
 msgid "can't use the builtin command '%s' as a term"
-msgstr "Kann eingebauten Befehl '$term' nicht als Begriff verwenden"
+msgstr "Kann den eingebauten Befehl '%s' nicht als Begriff verwenden."
 
 #: builtin/bisect--helper.c:60
-#, fuzzy, c-format
+#, c-format
 msgid "can't change the meaning of the term '%s'"
-msgstr "Kann Bedeutung von '$term' nicht ändern."
+msgstr "Kann die Bedeutung von dem Begriff '%s' nicht ändern."
 
 #: builtin/bisect--helper.c:71
 msgid "please 

[BUG] Weird breakages in t1450 #2 on NonStop

[Randall S. Becker]

This one has me scratching my head:

The object file name being reported below in t1450, subtest 2 is corrupt,
but I can't figure out why the script might be generating this condition -
there's nothing apparent, but it looks like the git commit -m C step is
reporting or using a bad name. This breakage was not present in 2.8.5 (now
at 7234152 (2.13.5) and is persistent (i.e. always happens). This is the
only test in all of git where I have observed this particular situation.
Adding set -x to test_commit is unrevealing. The git fsck in this test is
never executed because the test_commit fails with a non-zero git commit
completion code. There is no rn (actual r n 252 252 252 252) in the
objects directory - even the 'rn' does not correspond to anything.. I am
suspecting an unterminated string that ran into freed memory somewhere, but
that's speculative. 

Does anyone have a perspective on this - was it subsequently fixed?

Thanks,
Randall

Initialized empty Git repository in /home/git/git/t/trash
directory.t1450-fsck/another/.git/
error: object directory /home/git/git/t/trash
directory.t1450-fsck/another/.git/objects/rn does not exist; check
.git/objects/info/alternates.
[master (root-commit) 1aac250] C
 Author: A U Thor 
 1 file changed, 1 insertion(+)
 create mode 100644 fileC
error: object directory /home/git/git/t/trash
directory.t1450-fsck/another/.git/objects/rn does not exist; check
.git/objects/info/alternates.
--- empty   2018-01-11 13:57:30 +
+++ actual  2018-01-11 13:57:40 +
@@ -0,0 +1 @@
+error: object directory /home/git/git/t/trash
directory.t1450-fsck/another/.git/objects/rn does not exist; check
.git/objects/info/alternates.
not ok 2 - loose objects borrowed from alternate are not missing
#
#   mkdir another &&
#   (
#   cd another &&
#   git init &&
#   echo ../../../.git/objects
>.git/objects/info/alternates &&
#   test_commit C fileC one &&
#   git fsck --no-dangling >../actual 2>&1
#   ) &&
#   test_cmp empty actual
#

Ls -l of the objects directory:
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 .
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 ..
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 13
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 56
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 bd
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 c9
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 f7
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 info
drwxrwxr-x 1 randall ITUGLIB 4096 Jan 11 08:26 pack
from find:
./13
./13/4756353796a5439d93586be27999eea3807a34
./56
./56/26abf0f72e58d7a153368ba57db4c673c0e171
./bd
./bd/04fbdc74c1ad468ee1cc86d49860490ab3e6c7
./c9
./c9/145d6720f85544cc4bb6009a2e541660aa156b
./c9/176b0dd1a95c80ad8de21784b1eeffd3681f49
./f7
./f7/19efd430d52bcfc8566a43b2eb655688d38871
./info
./pack

Unable to de-init stubborn submodule

[Ævar Arnfjörð Bjarmason]

What if we wanted to drop sha1collisiondetection/ as a submodule and
replace it with a copy of what's now in sha1dc/? I ran into this with
another project, but here's a way to reproduce it on git.git:

(
rm -rf /tmp/git &&
git clone g...@github.com:git/git.git /tmp/git
cd /tmp/git &&
git tag nuke-before &&
git submodule update --init &&
git rm -r .gitmodules sha1collisiondetection &&
git commit -m"Nuke sha1dc submodule" &&
cp -Rvp sha1dc sha1collisiondetection &&
git add sha1collisiondetection &&
git commit -m"Now it's not a submodule" &&
git tag nuke-after &&
git reset --hard nuke-before &&
git submodule update --init && # skip this and the below won't fail
git reset --hard nuke-after && # Emulate someone doing a pull
git ls-tree HEAD | grep sha1collisiondetection && # OK, shows "tree" 
not "commit"
test $(git rev-parse HEAD) == $(git -C sha1collisiondetection/ log -1 
--pretty=format:%H) && echo OK || echo WTF
)

This results in a really bizarre state where according to ls-tree
sha1collisiondetection is a tree at the current commit:

04 tree 81583289d96bdde4b366c243ab524ea28d895ea5
sha1collisiondetection

But git still believes there's a submodule there for some reason, and
shows the log for the upstream sha1collisiondetection project:

git -C sha1collisiondetection/ log -1
commit 19d97bf (HEAD, origin/master, origin/HEAD, master)
Merge: 3f14d1b c93f0b4
Author: Dan Shumow 
Date:   Sat Jul 1 12:36:15 2017 -0700

Merge pull request #37 from avar/fixup-pull-request-34

Fix endian detection logic for Sparc, little endian BSD etc.

Doing:

git submodule deinit sha1collisiondetection

Does nothing to help, then I thought it might be:

git config -f .git/config -l|grep ^submodule
submodule.sha1collisiondetection.active=true

submodule.sha1collisiondetection.url=https://github.com/cr-marcstevens/sha1collisiondetection.git

But running:

git config --remove-section submodule.sha1collisiondetection

Doesn't help either, neither does removing the index:

rm .git/index &&
git reset --hard

If you then do:

rm -rf .git/modules

You'll get this error:

git -C sha1collisiondetection/ log -1
fatal: Not a git repository: 
/tmp/git/sha1collisiondetection/../.git/modules/sha1collisiondetection

But I can't see what's still referencing it.

This problem is avoided if, as noted with a comment I skip:

git submodule update --init

But I shouldn't need to remember to de-init a submodule before moving to
a new commit that doesn't have it, least I end up in some seemingly
unrecoverable state.

Am I missing something obvious here?

RE: [PATCH] Replaced read with xread in transport-helper.c to fix SSIZE_MAX overun in t5509

[Randall S. Becker]

On January 11, 2018 1:31 AM Jeff King wrote"
> On Thu, Jan 11, 2018 at 12:40:05AM -0500, Randall S. Becker wrote:
> > diff --git a/transport-helper.c b/transport-helper.c index
> > 3640804..68a4e30 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -1202,7 +1202,7 @@ static int udt_do_read(struct
> unidirectional_transfer *t)
> > return 0;   /* No space for more. */
> >
> > transfer_debug("%s is readable", t->src_name);
> > -   bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
> > +   bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE -
> > + t->bufuse);
> > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
> > errno != EINTR) {
> > error_errno("read(%s) failed", t->src_name);
> 
> After this patch, I don't think we can ever see any of those errno values
> again, as xread() will automatically retry in such a case.
> 
> I think that's OK. In the code before your patch, udt_do_read() would return
> 0 in such a case, giving the caller the opportunity to do something besides
> simply retry the read. But the only caller is udt_copy_task_routine(), which
> would just loop anyway.  It may be worth mentioning that in the commit
> message.
> 
> So your patch is OK.  But we should probably clean up on top, like the patch
> below (on top of yours; though note your patch was whitespace corrupted;
> the tabs were converted to spaces).
> 
> -- >8 --
> Subject: [PATCH] transport-helper: drop read/write errno checks
> 
> Since we use xread() and xwrite() here, EINTR, EAGAIN, and EWOULDBLOCK
> retries are already handled for us, and we will never see these errno values
> ourselves. We can drop these conditions entirely, making the code easier to
> follow.
> 
> Signed-off-by: Jeff King 
> ---
>  transport-helper.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/transport-helper.c b/transport-helper.c index
> d48be722a5..fc49567ac4 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -1208,8 +1208,7 @@ static int udt_do_read(struct
> unidirectional_transfer *t)
> 
>   transfer_debug("%s is readable", t->src_name);
>   bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
> - if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
> - errno != EINTR) {
> + if (bytes < 0) {
>   error_errno("read(%s) failed", t->src_name);
>   return -1;
>   } else if (bytes == 0) {
> @@ -1236,7 +1235,7 @@ static int udt_do_write(struct
> unidirectional_transfer *t)
> 
>   transfer_debug("%s is writable", t->dest_name);
>   bytes = xwrite(t->dest, t->buf, t->bufuse);
> - if (bytes < 0 && errno != EWOULDBLOCK) {
> + if (bytes < 0) {
>   error_errno("write(%s) failed", t->dest_name);
>   return -1;
>   } else if (bytes > 0) {

I'm sorry about the spaces. Still trying to get my mailer fixed so that I can 
get there directly from git.

Thanks for the approval and subsequent.
Cheers,
Randall

RE: [PATCH] Replaced read with xread in transport-helper.c to fix SSIZE_MAX overun in t5509

[Randall S. Becker]

On January 11, 2018 1:21 AM , Jeff King wrote:
> On Thu, Jan 11, 2018 at 12:40:05AM -0500, Randall S. Becker wrote:
> > This fix was needed on HPE NonStop NSE where SSIZE_MAX is less than
> > BUFFERSIZE resulting in EINVAL. The call to read in transport-helper.c
> > was the only place outside of wrapper.c.
> 
> For my own curiosity, what is SSIZE_MAX on your platform? BUFFERSIZE is
> only 64k. Do you really have 16-bit size_t?
> 
> I wondered if you would also need to set MAX_IO_SIZE, but it looks like we
> default it to SSIZE_MAX.

size_t is 32 or 64 depending on the memory model of how a program is compiled. 
SSIZE_MAX in limits.h is 53284, which is a message system limit. There was a 
previous fix associated with this size limit came from our team (commit 
a983e6ac58094a3b2466ad3be13049ce213f9fc3).

Cheers,
Randall

Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]

[Jeff King]

On Thu, Jan 11, 2018 at 12:39:28PM +0100, SZEDER Gábor wrote:

> > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
> > index 9e4e694d93..09ad4d8878 100755
> > --- a/t/t3900-i18n-commit.sh
> > +++ b/t/t3900-i18n-commit.sh
> > @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '
> > test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
> >  '
> >  
> > -test_expect_success 'UTF-8 invalid characters refused' '
> 
> Note that the test snippet started right after that last single quote,
> i.e. it started with a newline.
> 
> > -   test_when_finished "rm -f \"$HOME/stderr $HOME/invalid\"" &&
> > +test_expect_success 'UTF-8 invalid characters refused' - <<\EOT
> > +   test_when_finished 'rm -f "$HOME/stderr $HOME/invalid"' &&
> 
> And now it starts at the beginning of this line, i.e. without that
> leading neline.  This change leads to the following when run with '-v':

Yeah, I noticed that, too. It would be easy enough to add the extra
newline ourselves when printing the verbose output (quite a few of our
older tests don't start with a newline already, so it would be improving
them, too).

Alternatively, I considered adding a whole new helper function that
would skip the need to say "-" as the test body. And then it would
always know we were reading from the here-doc.

> > +   # command substitution eats trailing whitespace, so we add
> > +   # and then remove a non-whitespace character.
> > +   eval "$1=\$(cat; printf x)"
> > +   eval "$1=\${$1%x}"
> > +   fi
> > +}
> 
> Command substitutions don't eat trailing whitespaces in general, only
> trailing newlines.  POSIX:

Yeah, I wondered about that, but didn't bother digging since the
solution is the same either way.

> How about this alternative (also adding the missing leading newline
> mentioned above):
> 
> + eval "$1='
> +'\$(cat)'
> +'"
> 
> The indentation is yuck, but overall perhaps a bit less hacky...

I wrote something very similar at first, before finding the printf trick
on Stack Overflow. I thought the indentation on what I wrote was too
ugly. :)

I don't have a strong preference, and certainly if one is more portable
than the other, we should choose that.

The main point of my email was just to say "do people even like the
concept/direction?"

-Peff

read test snippet from stdin [was: [PATCH] t3900: add some more quotes]

[SZEDER Gábor]

> I've often wondered if
> our tests would be more readable taking the snippet over stdin.
> Something like:
> 
> diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
> index 9e4e694d93..09ad4d8878 100755
> --- a/t/t3900-i18n-commit.sh
> +++ b/t/t3900-i18n-commit.sh
> @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '
>   test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
>  '
>  
> -test_expect_success 'UTF-8 invalid characters refused' '

Note that the test snippet started right after that last single quote,
i.e. it started with a newline.

> - test_when_finished "rm -f \"$HOME/stderr $HOME/invalid\"" &&
> +test_expect_success 'UTF-8 invalid characters refused' - <<\EOT
> + test_when_finished 'rm -f "$HOME/stderr $HOME/invalid"' &&

And now it starts at the beginning of this line, i.e. without that
leading neline.  This change leads to the following when run with '-v':

  expecting success:test_when_finished 'rm -f "$HOME/stderr $HOME/invalid"' 
&&
echo "UTF-8 characters" >F &&
printf "Commit message\n\nInvalid surrogate:\355\240\200\n" \
>"$HOME/invalid" &&
git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
test_i18ngrep "did not conform" "$HOME"/stderr

Notice how the "expecting success" and the first line of the test ended
up in the same line.  I find this more annoying than the lack of empty
line between the colored and indented test code and the uncolored and
unindented test output.

>   echo "UTF-8 characters" >F &&
>   printf "Commit message\n\nInvalid surrogate:\355\240\200\n" \
>   >"$HOME/invalid" &&
>   git commit -a -F "$HOME/invalid" 2>"$HOME"/stderr &&
>   test_i18ngrep "did not conform" "$HOME"/stderr
> -'
> +EOT
>  
>  test_expect_success 'UTF-8 overlong sequences rejected' '
>   test_when_finished "rm -f \"$HOME/stderr $HOME/invalid\"" &&
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 1701fe2a06..be8a47d304 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -391,11 +391,32 @@ test_verify_prereq () {
>   error "bug in the test script: '$test_prereq' does not look like a 
> prereq"
>  }
>  
> +# Read from stdin into the variable given in $1.
> +test_read_to_eof () {
> + # Bash's "read" is sufficiently flexible that we can skip the extra
> + # process.
> + if test -n "$BASH_VERSION"
> + then
> + # 64k should be enough for anyone...
> + read -N 65536 -r "$1"
> + else
> + # command substitution eats trailing whitespace, so we add
> + # and then remove a non-whitespace character.
> + eval "$1=\$(cat; printf x)"
> + eval "$1=\${$1%x}"
> + fi
> +}

Command substitutions don't eat trailing whitespaces in general, only
trailing newlines.  POSIX:

  The shell shall expand the command substitution by executing command
  in a subshell environment (see Shell Execution Environment) and
  replacing the command substitution (the text of command plus the
  enclosing "$()" or backquotes) with the standard output of the
  command, removing sequences of one or more s at the end of
  the substitution.

Bash and dash conform to this.

How about this alternative (also adding the missing leading newline
mentioned above):

+   eval "$1='
+'\$(cat)'
+'"

The indentation is yuck, but overall perhaps a bit less hacky...

Re: [PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Duy Nguyen]

On Thu, Jan 11, 2018 at 4:47 PM, Nguyễn Thái Ngọc Duy  wrote:
>  Though, Stefan, while i'm not opposed to trace every single setting
>  in child_process, including variable deletion, cwd and even more, it

Another thing I forgot to add, s/ and even more/, redirection&/. At
some point I think I was checking the git-pack-objects command from
GIT_TRACE and didn't realize it was taking input from stdin (I was
naive :D). At least on linux we could even take advantage of
/proc//fd to show path names and stuff in addition to plain file
descriptors.

>  may be not that often needed for a "casual" developer.
>
>  I suggest we have something like $GIT_TRACE_EXEC instead that could
>  be super verbose when we need it and leave $GIT_TRACE with a
>  reasonable subset.
-- 
Duy

Re: [PATCH] run-command.c: print env vars when GIT_TRACE is set

[Duy Nguyen]

On Thu, Jan 11, 2018 at 5:07 PM, Jeff King  wrote:
> On Wed, Jan 10, 2018 at 05:48:35PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> Occasionally submodule code could execute new commands with GIT_DIR set
>> to some submodule. GIT_TRACE prints just the command line which makes it
>> hard to tell that it's not really executed on this repository.
>>
>> Print env variables in this case. Note that the code deliberately ignore
>> variables unsetting because there are so many of them (to keep git
>> environment clean for the next process) and really hard to read.
>
> I like this, and I'm pretty sure it would have helped me debug at least
> once in the past. I did notice one funny thing, though I think it was
> largely there before.
>
> The output for a single command is pretty shell-like due to the quoting:
>
>   $ GIT_TRACE=1 ./git upload-pack . >/dev/null
>   [...]run_command: 'git-upload-pack' '.'
>
> You could copy and paste that to a shell if you wanted.  And with
> environment variables, that remains so:
>
>   $ GIT_TRACE=1 ./git ls-remote https://github.com/git/git >/dev/null
>   [...]run_command: 'GIT_DIR=.git' 'git-remote-https' 'https://[...]'
>
> But if we're actually running a command via the shell, it all gets
> quoted as one argument:
>
>   $ GIT_TRACE=1 GIT_PAGER='foo | bar' ./git log
>   [...]run_command: 'LV=-c' 'foo | bar'
>
> which is kind of weird, as that's not something that can be run in a
> shell. This isn't introduced by your patch at all, but I noticed it more
> because of the shell-like environment variable output.

I think you just found an argument to change my "meh" with regards to
quoting to "need to fix" because I also often copy/paste these
commands. I thought about it and assumed shells would still recognize
'name=value' assignments without actually testing it.

> We actually used to print a separate:
>
>   exec: /bin/sh -c 'foo | bar'
>
> line when we invoked a shell, which would arguably be the right place to
> show the env variables for such a case. But that went away with
> 3967e25be1 (run-command: prepare command before forking, 2017-04-19).
>
> I think it might be helpful if we added back in "/bin/sh -c" to the
> run_command line when "use_shell" is in effect (and when we're not doing
> our "skip the shell" trickery).  But that's totally orthogonal to your
> patch.
>
> And anyway, it's just tracing output, so I don't think it's incredibly
> important either way. It was just something I noticed while looking at
> your patch's output.

Noted. I might do it if it's not super complex.
-- 
Duy

Re: [PATCH/RFC] add--interactive: ignore all internal submodule changes

[Duy Nguyen]

On Thu, Jan 11, 2018 at 2:47 AM, Stefan Beller  wrote:
> On Wed, Jan 10, 2018 at 3:06 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> For 'add -i' and 'add -p' the only action we can take on a dirty
>> submodule entry (from the superproject perspective) is its SHA-1. The
>> content changes inside do not matter, at least until interactive add has
>> --recurse-submodules or something.
>>
>> Ignore all dirty changes to reduce the questions 'add -i' and 'add -p'
>> throw at the user when submodules are dirty.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  $DAYJOB started to use submodules and this annoys me so much when I
>>  use 'git add -p'. I'm neither very familiar with add--interactive nor
>>  submodules code but this seems to work. Hopefully it's a correct
>>  change.
>
> I would think this fixes your problem and it looks correct.
>
> However I wonder about some subtle detail:
> the "dirty" setting will ignore anything inside the submodule, and
> only pay attention to the delta in gitlinks between HEAD and index.

Wait, why does diff-files, the command about worktree and index, look
at HEAD? Testing, testing... no I think it still works as expected

> ~/w/git/temp/z $ git ls-files  --stage foo
16 41521690bee4b76ad108a403b79415f8591a5592 0   foo
> ~/w/git/temp/z $ git -C foo rev-parse HEAD
3bc15b2e78ec3a5c5ea27715f20adaa2669446b1
> ~/w/git/temp/z $ ../git diff --ignore-submodules=dirty foo
diff --git a/foo b/foo
index 4152169..3bc15b2 16
--- a/foo
+++ b/foo
@@ -1 +1 @@
-Subproject commit 41521690bee4b76ad108a403b79415f8591a5592
+Subproject commit 3bc15b2e78ec3a5c5ea27715f20adaa2669446b1
> ~/w/git/temp/z $ ../git diff-files --ignore-submodules=dirty foo
:16 16 41521690bee4b76ad108a403b79415f8591a5592
 M  foo

If I reset foo/.git/HEAD back to 4152169... then diff-files
--ignore..=dirty returns empty. So I think it does check submodule's
HEAD.

> Maybe we'd want to have a mode "dirty-except-submodule-HEAD",
> which would ignore all submodule worktree changes, but if its HEAD
> is different than the gitlink in the superproject index or HEAD, such that
> checking out a different revision inside the submodule is not lost
> when staging things in the superproject for a new commit?
-- 
Duy

Re: [PATCH] doc/read-tree: remove obsolete remark

[Jeff King]

On Tue, Jan 09, 2018 at 04:30:34PM +0100, Andreas G. Schacker wrote:

> Earlier versions of `git read-tree` required the `--prefix` option value
> to end with a slash. This restriction was eventually lifted without a
> corresponding amendment to the documentation.

Makes sense.

> ---prefix=/::
> +--prefix=::
>   Keep the current index contents, and read the contents
>   of the named tree-ish under the directory at ``.
>   The command will refuse to overwrite entries that already
> - existed in the original index file. Note that the `/`
> - value must end with a slash.
> + existed in the original index file.

Is it worth mentioning in the new world order that the slash is not
implied? I.e., that you probably do want to say "--prefix=foo/" if you
want the subdirectory "foo", but do not want to match "foobar"?

-Peff

Re: [PATCH v5 8/9] sequencer: try to commit without forking 'git commit'

[Phillip Wood]

On 10/01/18 22:40, Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 10 Jan 2018, Jonathan Nieder wrote:
> 
>> Phillip Wood wrote:
>>
>>> From: Phillip Wood 
>>>
>>> If the commit message does not need to be edited then create the
>>> commit without forking 'git commit'. Taking the best time of ten runs
>>> with a warm cache this reduces the time taken to cherry-pick 10
>>> commits by 27% (from 282ms to 204ms), and the time taken by 'git
>>> rebase --continue' to pick 10 commits by 45% (from 386ms to 212ms) on
>>> my computer running linux. Some of greater saving for rebase is
>>> because it no longer wastes time creating the commit summary just to
>>> throw it away.
>>
>> Neat!  Dmitry Torokhov (cc-ed) noticed[1]

Thanks for reporting and bisecting this Dmitry. When I was preparing
this series I checked to see if it needed to run the 'pre-commit' hook
but missed the 'prepare-commit-msg' hook.

> that this causes the
>> prepare-commit-msg hook not to be invoked, which I think is
>> unintentional.  Should we check for such a hook and take the slowpath
>> when it is present?
> 
> We could also easily recreate the functionality:
> 
>   if (find_hook("pre-commit")) {
>   struct argv_array hook_env = ARGV_ARRAY_INIT;
> 
>   argv_array_pushf(_env, "GIT_INDEX_FILE=%s",
>   get_index_file());
>   argv_array_push(_env, "GIT_EDITOR=:");
>   ret = run_hook_le(hook_env.argv, "pre-commit", NULL);
>   argv_array_clear(_env);
>   }

Thanks Johannes, though it needs to run the 'prepare-commit-msg' hook,
the current code in master only runs the 'pre-commit' hook when we edit
the message. I'll send a patch with a test.

Best Wishes

Phillip

> (This assumes that the in-process try_to_commit() is only called if the
> commit message is not to be edited interactively, which is currently the
> case.)
> 
> Ciao,
> Dscho
> 

Re: [PATCH] run-command.c: print env vars when GIT_TRACE is set

[Jeff King]

On Wed, Jan 10, 2018 at 05:48:35PM +0700, Nguyễn Thái Ngọc Duy wrote:

> Occasionally submodule code could execute new commands with GIT_DIR set
> to some submodule. GIT_TRACE prints just the command line which makes it
> hard to tell that it's not really executed on this repository.
> 
> Print env variables in this case. Note that the code deliberately ignore
> variables unsetting because there are so many of them (to keep git
> environment clean for the next process) and really hard to read.

I like this, and I'm pretty sure it would have helped me debug at least
once in the past. I did notice one funny thing, though I think it was
largely there before.

The output for a single command is pretty shell-like due to the quoting:

  $ GIT_TRACE=1 ./git upload-pack . >/dev/null
  [...]run_command: 'git-upload-pack' '.'

You could copy and paste that to a shell if you wanted.  And with
environment variables, that remains so:

  $ GIT_TRACE=1 ./git ls-remote https://github.com/git/git >/dev/null
  [...]run_command: 'GIT_DIR=.git' 'git-remote-https' 'https://[...]'

But if we're actually running a command via the shell, it all gets
quoted as one argument:

  $ GIT_TRACE=1 GIT_PAGER='foo | bar' ./git log
  [...]run_command: 'LV=-c' 'foo | bar'

which is kind of weird, as that's not something that can be run in a
shell. This isn't introduced by your patch at all, but I noticed it more
because of the shell-like environment variable output.

We actually used to print a separate:

  exec: /bin/sh -c 'foo | bar'

line when we invoked a shell, which would arguably be the right place to
show the env variables for such a case. But that went away with
3967e25be1 (run-command: prepare command before forking, 2017-04-19).

I think it might be helpful if we added back in "/bin/sh -c" to the
run_command line when "use_shell" is in effect (and when we're not doing
our "skip the shell" trickery).  But that's totally orthogonal to your
patch.

And anyway, it's just tracing output, so I don't think it's incredibly
important either way. It was just something I noticed while looking at
your patch's output.

-Peff

[PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Nguyễn Thái Ngọc Duy]

Occasionally submodule code could execute new commands with GIT_DIR set
to some submodule. GIT_TRACE prints just the command line which makes it
hard to tell that it's not really executed on this repository.

Print modified env variables (compared to parent environment) in this
case. Actually only modified or new variables are printed. Variable
deletion is suppressed because they are often used to clean up
repo-specific variables that git passes around between processes. When
submodule code executes commands on another repo, it clears all these
variables, _many_ of these, that make the output really noisy.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 v2 fixes up commit message to clarify about "env delta" and why var
 deletion is not printed.

 It also keeps child_process tracing in one place/function, this
 should make it easier to trace more e.g. cwd and stuff.

 Though, Stefan, while i'm not opposed to trace every single setting
 in child_process, including variable deletion, cwd and even more, it
 may be not that often needed for a "casual" developer.
 
 I suggest we have something like $GIT_TRACE_EXEC instead that could
 be super verbose when we need it and leave $GIT_TRACE with a
 reasonable subset.

 run-command.c |  3 ++-
 trace.c   | 39 +++
 trace.h   |  3 +++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 31fc5ea86e..002074b128 100644
--- a/run-command.c
+++ b/run-command.c
@@ -624,7 +624,8 @@ int start_command(struct child_process *cmd)
cmd->err = fderr[0];
}
 
-   trace_argv_printf(cmd->argv, "trace: run_command:");
+   trace_run_command(cmd);
+
fflush(NULL);
 
 #ifndef GIT_WINDOWS_NATIVE
diff --git a/trace.c b/trace.c
index b7530b51a9..e5e46e2a09 100644
--- a/trace.c
+++ b/trace.c
@@ -23,6 +23,7 @@
 
 #include "cache.h"
 #include "quote.h"
+#include "run-command.h"
 
 struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
 struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
@@ -272,6 +273,44 @@ void trace_performance_fl(const char *file, int line, 
uint64_t nanos,
 #endif /* HAVE_VARIADIC_MACROS */
 
 
+static void concatenate_env(struct strbuf *dst, const char *const *env)
+{
+   int i;
+
+   /* Copy into destination buffer. */
+   strbuf_grow(dst, 255);
+   for (i = 0; env[i]; ++i) {
+   /*
+* the main interesting is setting new vars
+* e.g. GIT_DIR, ignore the unsetting to reduce noise.
+*/
+   if (!strchr(env[i], '='))
+   continue;
+   strbuf_addch(dst, ' ');
+   sq_quote_buf(dst, env[i]);
+   }
+}
+
+void trace_run_command(const struct child_process *cp)
+{
+   struct strbuf buf = STRBUF_INIT;
+
+   if (!prepare_trace_line(NULL, 0, _default_key, ))
+   return;
+
+   strbuf_addf(, "trace: run_command:");
+
+   /*
+* caller is responsible for setting this if the main source
+* is actually in cp->env_array
+*/
+   if (cp->env)
+   concatenate_env(, cp->env);
+
+   sq_quote_argv(, cp->argv, 0);
+   print_trace_line(_default_key, );
+}
+
 static const char *quote_crnl(const char *path)
 {
static struct strbuf new_path = STRBUF_INIT;
diff --git a/trace.h b/trace.h
index 88055abef7..e54c687f26 100644
--- a/trace.h
+++ b/trace.h
@@ -4,6 +4,8 @@
 #include "git-compat-util.h"
 #include "strbuf.h"
 
+struct child_process;
+
 struct trace_key {
const char * const key;
int fd;
@@ -17,6 +19,7 @@ extern struct trace_key trace_default_key;
 extern struct trace_key trace_perf_key;
 
 extern void trace_repo_setup(const char *prefix);
+extern void trace_run_command(const struct child_process *cp);
 extern int trace_want(struct trace_key *key);
 extern void trace_disable(struct trace_key *key);
 extern uint64_t getnanotime(void);
-- 
2.15.1.600.g899a5f85c6

Re: [PATCH v4 0/4] Add --no-ahead-behind to status

[Jeff King]

On Wed, Jan 10, 2018 at 12:22:10PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > To be clear, which approach are we talking about? I think there are
> > three options:
> >
> >   1. The user tells us not to bother computing real ahead/behind values.
> >  We always say "same" or "not the same".
> >
> >   2. The user tells us not to bother computing ahead/behind values
> >  with more effort than N. After traversing N commits without getting
> >  an answer, we say "same" or "not the same". But we may sometimes
> >  give a real answer if we found it within N.
> >
> >   3. The user tells us not to spend more effort than N. After traversing
> >  N commits we try to make some partial statement based on
> >  generations (or commit timestamps as a proxy for them).
> >
> > I agree that (3) is probably not going to be useful enough in the
> > general case to merit the implementation effort and confusion. But is
> > there anything wrong with (2)?
> 
> I agree (3) would not be all that interesting.  Offhand I do not see
> a problem with (2).  I think with "real" in your "sometimes give a
> real answer" you meant to say that we limit our answers to just one
> three ("same", "not the same", "ahead/behind by exactly N/M") and I
> think it is a good choice that is easy to explain.

Yes, exactly. That's a better way of saying it.

-Peff

Re: [PATCH] t3900: add some more quotes

[Jeff King]

On Wed, Jan 10, 2018 at 01:31:22PM -0800, Junio C Hamano wrote:

> > +# Read from stdin into the variable given in $1.
> > +test_read_to_eof () {
> > +   # Bash's "read" is sufficiently flexible that we can skip the extra
> > +   # process.
> > +   if test -n "$BASH_VERSION"
> > +   then
> > +   # 64k should be enough for anyone...
> > +   read -N 65536 -r "$1"
> > +   else
> > +   # command substitution eats trailing whitespace, so we add
> > +   # and then remove a non-whitespace character.
> > +   eval "$1=\$(cat; printf x)"
> > +   eval "$1=\${$1%x}"
> > +   fi
> > +}
> 
> Yuck.  Hacky but nice.
> 
> I think this will make it easier to read but I suspect here text
> would use temporary files and may slow things down a bit?  I do not
> know if it is even measurable, though.

Yeah, since I was able to contain the horrible-ness in this function, I
think the overall readability/portability is probably OK. My main
concern was that it would be slower (hence the bash hackery). I applied
the patch below on top to see what kind of impact we could measure
across the whole suite:

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index be8a47d304..6f2e6e7560 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -441,6 +441,15 @@ test_expect_success () {
then
test_read_to_eof test_block
set -- "$1" "$test_block"
+   else
+   # this is obviously a dumb thing to do, but it's just
+   # a performance-testing hack.
+   test_read_to_eof test_block <

Re: [PATCH v4 8/7] wildmatch test: skip file creation tests on Windows proper

[Duy Nguyen]

On Thu, Jan 11, 2018 at 3:24 AM, Johannes Schindelin
 wrote:
>> diff --git a/Makefile b/Makefile
>> index 2a81ae22e9..567387b558 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -644,6 +644,7 @@ X =
>>
>>  PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
>>
>> +TEST_PROGRAMS_NEED_X += test-3071-wildmatch
>
> I guess I can always work on unifying those gazillion of test executables
> into a single one later.

Oh yeah. I did notice your remark about disk consumption but this was
a quick hack that I would not bother with it. For the record I'm
slightly bothered with many test programs too, not due to disk size
but because it increases link time (disk i/o probably also plays part
in that). This may help another thing... at the end of the mail

>> +static struct match_input match_tests[] = {
>> + /* Basic wildmatch features */
>> + { 1, "foo", "foo" },
>> + { 0, "foo", "bar" },
>> + { 1, "", "" },
>
> These patterns share the "magic-ness" of Ævar's test cases... although
> your version is certainly more concise.

Another thing will make me move away from this style is, you can't
mark one test broken. In the end, we may have some macro that issue
one match() call per line, very similar to how t3070 does now. Then we
have more freedom in marking tests.

> BTW IIRC Ævar explicitly said that he needs to use `ls-files` in order to
> test the interaction with the index, so that would probably take a little
> bit more work.

Yeah, run_command() and stuff, not super hard (but then it opens up
another aspect I didn't address in this quick hack: collecting output
log of a test and only showing it when the test fails, could be
tricker to do in C than shell.

>> diff --git a/t/t3071-wildmatch.sh b/t/t3071-wildmatch.sh
>> new file mode 100755
>> index 00..6e83b4d684
>> --- /dev/null
>> +++ b/t/t3071-wildmatch.sh
>> @@ -0,0 +1,3 @@
>> +#!/bin/sh
>> +
>> +exec helper/test-3071-wildmatch t3071-wildmatch "$@"
>
> Should it not be `exec test-3071-wildmatch "${0%.sh}" "$@"`?

No, test-lib.sh is required to set up $PATH properly so you can run
test programs without path. This is another sticky point. Some
integration with test-lib.sh is needed. I would like to have something
like this

-- 8< --
cat >t3071-wildmatch-c.sh 

Re: git svn clone of messy repository

[Eric Wong]

Jason Greenbaum  wrote:
> --trunk=trunk/project_of_interest \
> --branches=branches/FF-1.0/project_of_interest \
> --branches=branches/FF-1.1/project_of_interest \

> The trunk seems to become the 'master' branch just fine, but my svn
> branches are not pulled down.  I'm not sure I have the syntax right or
> if this is even possible without first reorganizing the svn repo in
> place, updating the .git/config file, or by some other means.

By default, the basename ("project_of_interest") is used and you
get collisions.

I think this section of the git-svn manpage should help:

| When using multiple --branches or --tags, 'git svn' does not automatically
| handle name collisions (for example, if two branches from different paths have
| the same name, or if a branch and a tag have the same name).  In these cases,
| use 'init' to set up your Git repository then, before your first 'fetch', edit
| the $GIT_DIR/config file so that the branches and tags are associated
| with different name spaces.  For example:
| 
|   branches = stable/*:refs/remotes/svn/stable/*
|   branches = debug/*:refs/remotes/svn/debug/*