Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

[Kaartic Sivaraam]

On Friday 01 December 2017 11:59 PM, Jeff King wrote:

On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:


Thanks for the review :-)


Actually, I meant to bikeshed one part but forgot. ;)


+   fprintf(stderr, _("hint: Waiting for your editor 
input..."));


I found "waiting for editor input" to be a funny way of saying this. I
input to the editor, the editor does not input to Git. :)

Maybe "waiting for your editor finish" or something would make more
sense?


May be the good "Launched editor. Waiting ..." message, that was used in 
a previous version, itself makes sense?





Or given that the goal is really just making it clear that we've spawned
an editor, something like "starting editor %s..." would work.


There was already discussion related to the "continuous tense" used in 
the phrase.


Extract from [1]:

-- 8< --
> fprintf(stderr, "Launching your editor...");

"It takes quite some time to launch this special Git Editor"

As Lars pointed out, the editor may be launched in the background,
that the user would not know, but they might expect a thing to
pop up as a modal dialog as is always with UIs.

So despite it being technically wrong at this point in time,
I would phrase it in past tense or in a way that indicates that the
user needs to take action already.

The "Launching..." sounds as if I need to wait for an event to occur.
-- >8 --

[1]: 
https://public-inbox.org/git/CAGZ79kZbm8SGY4rXKZHV82E-HX9qbQ4iyCbMgJEBFQf4fj3u=q...@mail.gmail.com/




I think
the "waiting for..." pattern is perfectly fine, though.

Re: UNITED NATION COMPENSATIONS,

[UNITED NATION COMPENSATIONS]

Re:Hello Dear,

What has actually kept you waiting to claim your fund $870.000.00 since then?

Your fund has been approved since and nobody has heard from you.

hurry and get back to me with your valid receiving data immediately you receive 
this mail to avoid error procedures because the United Nation Newly Elected 
president has approved the release of your awaited funds.


Regards,

Mr. Jake Brandon,

CUSTOMER CARE ON FOREIGN PAYMENT.

Re: [PATCH] git-gui: allow Ctrl+T to toggle multiple paths (Re: [BUG] git gui can't commit multiple files)

[Jonathan Nieder]

Jonathan Nieder wrote:

> From: Johannes Schindelin 
> Subject: git-gui: allow Ctrl+T to toggle multiple paths
>
> In the Unstaged Changes panel, selecting multiple lines (using
> shift+click) and pressing ctrl+t to stage them causes one file to be
> staged instead of all of the selected files.  The same also happens
> when unstaging files.
>
> This regression appears to have been introduced by gitgui-0.21.0~7^2~1
> (Allow keyboard control to work in the staging widgets, 2016-10-01).
>
> Also reported by zosrothko as a Git-for-Windows issue:
> https://github.com/git-for-windows/git/issues/1012
>
> [jn: fleshed out commit message]
>
> Reported-by: Timon 
> Signed-off-by: Johannes Schindelin 
> Signed-off-by: Jonathan Nieder 

Gah, this should say:

Signed-off-by; Jonathan Nieder 

[...]
>> I applied it locally to git-2.15.0 and can confirm it fixed the problem for 
>> me.
>> If you need any more info/help from me, or would like me to test
>> anything, feel free to ask.
>
> Thanks for this pointer.  I'm including the patch here so the project
> can consider applying it (it doesn't appear to have been sent upstream
> before).  I have not tested it or verified the claim it makes about
> what change introduced this regression --- if you're able to help with
> that, that would be welcome.

Can you bisect?  That is:

 git clone git://repo.or.cz/git-gui
 cd git-gui
 git bisect start
 git bisect good gitgui-0.20.0
 git bisect bad gitgui-0.21.0

Then cut to the chase:

 git checkout gitgui-0.21.0~7^2~1
 ... test test test ...
 git bisect (good|bad)

 git checkout gitgui-0.21.0~7^2~1^
 ... test test test ...
 git bisect (good|bad)

and follow the instructions if it suggests testing additional versions.

Then I'll be happy to re-send the patch with the results from your
testing.

Thanks again,
Jonathan

[PATCH] git-gui: allow Ctrl+T to toggle multiple paths (Re: [BUG] git gui can't commit multiple files)

[Jonathan Nieder]

From: Johannes Schindelin 
Subject: git-gui: allow Ctrl+T to toggle multiple paths

In the Unstaged Changes panel, selecting multiple lines (using
shift+click) and pressing ctrl+t to stage them causes one file to be
staged instead of all of the selected files.  The same also happens
when unstaging files.

This regression appears to have been introduced by gitgui-0.21.0~7^2~1
(Allow keyboard control to work in the staging widgets, 2016-10-01).

Also reported by zosrothko as a Git-for-Windows issue:
https://github.com/git-for-windows/git/issues/1012

[jn: fleshed out commit message]

Reported-by: Timon 
Signed-off-by: Johannes Schindelin 
Signed-off-by: Jonathan Nieder 
---
Hi Timon,

Timon wrote:
>>> On Sun, Dec 04, 2016 at 05:36:46PM +0100, Timon wrote:

 This is a regression in git 2.11.0 (version 2.10.2 is fine).

 In git-gui I select multiple files in the Unstaged Changes (using
 shift+click) and press ctrl+t to stage them. Then only one files gets
 staged instead of all of the selected files.
 The same happens when unstaging files.
[...]
> Originally I had this problem in gentoo and assumed in the end it's
> likely due to my specific configuration.
> However recently I switched to nixos and am still experiencing it.
>
> I've search again if I can find anything and lo and behold, it's
> already fixed in the *windows* version of git-gui...
>
> issue thread: https://github.com/git-for-windows/git/issues/1012
> commit: 
> https://github.com/git-for-windows/git/commit/1c4d4e7cbcf404c168df5537d55e9afd57f2b01b
>
> I applied it locally to git-2.15.0 and can confirm it fixed the problem for 
> me.
> If you need any more info/help from me, or would like me to test
> anything, feel free to ask.

Thanks for this pointer.  I'm including the patch here so the project
can consider applying it (it doesn't appear to have been sent upstream
before).  I have not tested it or verified the claim it makes about
what change introduced this regression --- if you're able to help with
that, that would be welcome.

Thanks,
Jonathan

 git-gui.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/git-gui.sh b/git-gui.sh
index ed24aa9..ef98fc2 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2501,6 +2501,19 @@ proc toggle_or_diff {mode w args} {
set pos [split [$w index @$x,$y] .]
foreach {lno col} $pos break
} else {
+   if {$mode eq "toggle"} {
+   if {$w eq $ui_workdir} {
+   do_add_selection
+   set last_clicked {}
+   return
+   }
+   if {$w eq $ui_index} {
+   do_unstage_selection
+   set last_clicked {}
+   return
+   }
+   }
+
if {$last_clicked ne {}} {
set lno [lindex $last_clicked 1]
} else {
-- 
2.15.0.531.g2ccb3012c9

[PATCH v2 1/2] git-prompt: make __git_eread intended use explicit

[Robert Abel]

__git_eread is used to read a single line of a given file (if it exists)
into a variable without the EOL. All six current users of __git_eread
use it that way and don't expect multi-line content.

Thus, add a comment and explicitly use $2 instead of shifting the args
down and using $@.

Signed-off-by: Robert Abel 
---
 contrib/completion/git-prompt.sh | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c6cbef38c..41a471957 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -278,11 +278,12 @@ __git_ps1_colorize_gitstring ()
r="$c_clear$r"
 }
 
+# Helper function to read the first line of a file into a variable.
+# __git_eread requires 2 arguments, the file path and the name of the
+# variable, in that order.
 __git_eread ()
 {
-   local f="$1"
-   shift
-   test -r "$f" && read "$@" <"$f"
+   test -r "$1" && read "$2" <"$1"
 }
 
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-- 
2.15.1

[PATCH v2 2/2] git-prompt: fix reading files with windows line endings

[Robert Abel]

If any of the files read by __git_eread have \r\n line endings, read
will only strip \n, leaving \r. This results in an ugly prompt, where
instead of

user@pc MINGW64 /path/to/repo (BARE:master)

the last parenthesis is printed over the beginning of the prompt like

)ser@pc MINGW64 /path/to/repo (BARE:master

Signed-off-by: Robert Abel 
---
 contrib/completion/git-prompt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 41a471957..983e419d2 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -283,7 +283,7 @@ __git_ps1_colorize_gitstring ()
 # variable, in that order.
 __git_eread ()
 {
-   test -r "$1" && read "$2" <"$1"
+   test -r "$1" && IFS=$'\r\n' read "$2" <"$1"
 }
 
 # __git_ps1 accepts 0 or 1 arguments (i.e., format string)
-- 
2.15.1

[WIP 2/2] submodule: read-only super-backed ref backend

[Jonathan Tan]

Note that a few major parts are still missing:
 - special handling of the current branch of the superproject
 - writing (whether "refs/..." to the superproject as an index change or
   a commit, or non-"refs/..." directly to the subproject like usual)

Signed-off-by: Jonathan Tan 
---
 Makefile   |   1 +
 refs.c |  11 +-
 refs/refs-internal.h   |   1 +
 refs/sp-backend.c  | 261 +
 submodule.c|  43 +--
 submodule.h|   2 +
 t/t1406-submodule-ref-store.sh |  26 
 7 files changed, 331 insertions(+), 14 deletions(-)
 create mode 100644 refs/sp-backend.c

diff --git a/Makefile b/Makefile
index e53750ca0..74120b5d7 100644
--- a/Makefile
+++ b/Makefile
@@ -858,6 +858,7 @@ LIB_OBJS += refs/files-backend.o
 LIB_OBJS += refs/iterator.o
 LIB_OBJS += refs/packed-backend.o
 LIB_OBJS += refs/ref-cache.o
+LIB_OBJS += refs/sp-backend.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
diff --git a/refs.c b/refs.c
index 339d4318e..1f7922733 100644
--- a/refs.c
+++ b/refs.c
@@ -1575,12 +1575,17 @@ static struct ref_store *lookup_ref_store_map(struct 
hashmap *map,
 static struct ref_store *ref_store_init(const char *gitdir,
unsigned int flags)
 {
-   const char *be_name = "files";
-   struct ref_storage_be *be = find_ref_storage_backend(be_name);
+   struct ref_storage_be *be;
struct ref_store *refs;
 
+   if (getenv("USE_SP")) {
+   be = _be_sp;
+   } else {
+   be = _be_files;
+   }
+
if (!be)
-   die("BUG: reference backend %s is unknown", be_name);
+   die("BUG: reference backend %s is unknown", "files");
 
refs = be->init(gitdir, flags);
return refs;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index dd834314b..a8ec03d90 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -651,6 +651,7 @@ struct ref_storage_be {
 
 extern struct ref_storage_be refs_be_files;
 extern struct ref_storage_be refs_be_packed;
+extern struct ref_storage_be refs_be_sp;
 
 /*
  * A representation of the reference store for the main repository or
diff --git a/refs/sp-backend.c b/refs/sp-backend.c
new file mode 100644
index 0..31e8cec4b
--- /dev/null
+++ b/refs/sp-backend.c
@@ -0,0 +1,261 @@
+#include "../cache.h"
+#include "../config.h"
+#include "../refs.h"
+#include "refs-internal.h"
+#include "ref-cache.h"
+#include "packed-backend.h"
+#include "../iterator.h"
+#include "../dir-iterator.h"
+#include "../lockfile.h"
+#include "../object.h"
+#include "../dir.h"
+#include "../submodule.h"
+
+/*
+ * Future: need to be in "struct repository"
+ * when doing a full libification.
+ */
+struct sp_ref_store {
+   struct ref_store base;
+   unsigned int store_flags;
+
+   /*
+* Ref store of this repository (the submodule), used only for the
+* reflog.
+*/
+   struct ref_store *files;
+
+   /*
+* Ref store of the superproject, for refs.
+*/
+   struct ref_store *files_superproject;
+};
+
+/*
+ * Create a new submodule ref cache and add it to the internal
+ * set of caches.
+ */
+static struct ref_store *sp_init(const char *gitdir, unsigned int flags)
+{
+   struct sp_ref_store *refs = xcalloc(1, sizeof(*refs));
+   struct ref_store *ref_store = (struct ref_store *)refs;
+
+   base_ref_store_init(ref_store, _be_sp);
+   refs->store_flags = flags;
+   refs->files = refs_be_files.init(gitdir, flags);
+
+   return ref_store;
+}
+
+/*
+ * Downcast ref_store to sp_ref_store. Die if ref_store is not a
+ * sp_ref_store. required_flags is compared with ref_store's
+ * store_flags to ensure the ref_store has all required capabilities.
+ * "caller" is used in any necessary error messages.
+ */
+static struct sp_ref_store *sp_downcast(struct ref_store *ref_store,
+ unsigned int required_flags,
+ const char *caller)
+{
+   struct sp_ref_store *refs;
+
+   if (ref_store->be != _be_sp)
+   die("BUG: ref_store is type \"%s\" not \"sp\" in %s",
+   ref_store->be->name, caller);
+
+   refs = (struct sp_ref_store *)ref_store;
+
+   if ((refs->store_flags & required_flags) != required_flags)
+   die("BUG: operation %s requires abilities 0x%x, but only have 
0x%x",
+   caller, required_flags, refs->store_flags);
+
+   return refs;
+}
+
+static int sp_read_raw_ref(struct ref_store *ref_store,
+ const char *refname, struct object_id *oid,
+ struct strbuf *referent, unsigned int *type)
+{
+   struct sp_ref_store *refs;
+
+   refs = sp_downcast(ref_store, REF_STORE_READ, "read_raw_ref");
+
+

[WIP 0/2] Submodule ref backend that mirrors superproject

[Jonathan Tan]

I sent out an earlier email [1] that discusses the idea of a submodule
ref backend that mirrors the superproject. Basically, if this backend is
used (for example, through a configuration option), the submodule itself
will not store any "refs/..." refs, but will check the gitlink of the
commit of the ref of the same name in the superproject. For example, if
the superproject has at refs/heads/foo:

superproject/
  sub [gitlink -> 1234...]

and at refs/heads/bar:

superproject/
  sub [gitlink -> 5678...]

Inside sub, "git rev-parse foo" will output "1234...", and "git rev-parse
bar" will output "5678...", even though "foo" and "bar" are not defined
anywhere inside sub.

(The submodule handles refs that do not start with "refs/" - for
example, HEAD and FETCH_HEAD - like usual.)

[1] also describes what happens when the submodule attempts to write to
any "refs/..." ref.

For those interested, here's what such an implementation might look
like, and a test to demonstrate such functionality. I have partial
read-only functionality - a lot of it still remains to be done.

[1] 
https://public-inbox.org/git/20171108172945.33c42a0e91b4ac494217b...@google.com/

Jonathan Tan (2):
  submodule: refactor acquisition of superproject info
  submodule: read-only super-backed ref backend

 Makefile   |   1 +
 refs.c |  11 +-
 refs/refs-internal.h   |   1 +
 refs/sp-backend.c  | 261 +
 submodule.c| 107 +++--
 submodule.h|   5 +
 t/t1406-submodule-ref-store.sh |  26 
 7 files changed, 374 insertions(+), 38 deletions(-)
 create mode 100644 refs/sp-backend.c

-- 
2.15.0.531.g2ccb3012c9-goog

[WIP 1/2] submodule: refactor acquisition of superproject info

[Jonathan Tan]

Signed-off-by: Jonathan Tan 
---
 submodule.c | 76 +
 submodule.h |  3 +++
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/submodule.c b/submodule.c
index bb531e0e5..ce511180e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1977,16 +1977,24 @@ void absorb_git_dir_into_superproject(const char 
*prefix,
}
 }
 
-const char *get_superproject_working_tree(void)
+/*
+ * Get the full filename of the gitlink entry corresponding to the
+ * superproject having this repo as a submodule.
+ *
+ * full_name will point to an internal buffer upon success.
+ *
+ * Return 0 if successful, 1 if not (for example, if the parent
+ * directory is not a repo or an unrelated one).
+ */
+int get_superproject_entry(const char **full_name)
 {
+   static struct strbuf sb = STRBUF_INIT;
+
struct child_process cp = CHILD_PROCESS_INIT;
-   struct strbuf sb = STRBUF_INIT;
const char *one_up = real_path_if_valid("../");
const char *cwd = xgetcwd();
-   const char *ret = NULL;
const char *subpath;
int code;
-   ssize_t len;
 
if (!is_inside_work_tree())
/*
@@ -1994,11 +2002,15 @@ const char *get_superproject_working_tree(void)
 * We might have a superproject, but it is harder
 * to determine.
 */
-   return NULL;
+   return 1;
 
if (!one_up)
-   return NULL;
+   return 1;
 
+   /*
+* No need to reset sb, since relative_path() will do it if
+* necessary.
+*/
subpath = relative_path(cwd, one_up, );
 
prepare_submodule_repo_env(_array);
@@ -2017,45 +2029,49 @@ const char *get_superproject_working_tree(void)
if (start_command())
die(_("could not start ls-files in .."));
 
-   len = strbuf_read(, cp.out, PATH_MAX);
+   strbuf_read(, cp.out, PATH_MAX);
close(cp.out);
 
-   if (starts_with(sb.buf, "16")) {
-   int super_sub_len;
-   int cwd_len = strlen(cwd);
-   char *super_sub, *super_wt;
+   code = finish_command();
 
+   if (starts_with(sb.buf, "16")) {
/*
 * There is a superproject having this repo as a submodule.
 * The format is  SP  SP  TAB  \0,
 * We're only interested in the name after the tab.
 */
-   super_sub = strchr(sb.buf, '\t') + 1;
-   super_sub_len = sb.buf + sb.len - super_sub - 1;
-
-   if (super_sub_len > cwd_len ||
-   strcmp([cwd_len - super_sub_len], super_sub))
-   die (_("BUG: returned path string doesn't match cwd?"));
-
-   super_wt = xstrdup(cwd);
-   super_wt[cwd_len - super_sub_len] = '\0';
-
-   ret = real_path(super_wt);
-   free(super_wt);
+   char *tab = strchr(sb.buf, '\t');
+   if (!tab)
+   die("BUG: ls-files returned line with no tab");
+   *full_name = tab + 1;
+   return 0;
}
-   strbuf_release();
-
-   code = finish_command();
 
if (code == 128)
/* '../' is not a git repository */
-   return NULL;
-   if (code == 0 && len == 0)
+   return 1;
+   if (code == 0)
/* There is an unrelated git repository at '../' */
+   return 1;
+   die(_("ls-tree returned unexpected return code %d"), code);
+}
+
+const char *get_superproject_working_tree(void)
+{
+   const char *full_name;
+   char *super_wt;
+   size_t len;
+   const char *ret;
+
+   if (get_superproject_entry(_name))
return NULL;
-   if (code)
-   die(_("ls-tree returned unexpected return code %d"), code);
 
+   super_wt = xstrdup(xgetcwd());
+   if (!strip_suffix(super_wt, full_name, ))
+   die("BUG: returned path string doesn't match cwd?");
+   super_wt[len] = '\0';
+   ret = real_path(super_wt);
+   free(super_wt);
return ret;
 }
 
diff --git a/submodule.h b/submodule.h
index f0da0277a..29ab302cc 100644
--- a/submodule.h
+++ b/submodule.h
@@ -134,6 +134,9 @@ extern void absorb_git_dir_into_superproject(const char 
*prefix,
  * Return the absolute path of the working tree of the superproject, which this
  * project is a submodule of. If this repository is not a submodule of
  * another repository, return NULL.
+ *
+ * The return value, if not NULL, points to the same shared buffer as the one
+ * returned by real_path().
  */
 extern const char *get_superproject_working_tree(void);
 
-- 
2.15.0.531.g2ccb3012c9-goog

Re: [PATCH] sha1_file: use strbuf_add() instead of strbuf_addf()

[Jeff King]

On Fri, Dec 01, 2017 at 02:50:05PM -0500, Derrick Stolee wrote:

> > > + baselen = path->len;
> > We set this here so that the '/' is included as part of the base. Makes
> > sense, but can we now drop the earlier setting of baselen before the
> > opendir() call?
> 
> Yeah, probably. I had briefly considered just adding the '/' before the
> first assignment of "baselen", but didn't want to change the error output. I
> also don't know if there are side effects for other platforms by calling
> opendir() with a '/'-terminated path.

I noticed that, too. Since it's so easy to keep doing the opendir
without the slash, I'd prefer to avoid finding out if there are such
platforms. :)

> Good catch! A big reason to pull it inside and use strbuf_add over
> strbuf_addstr is to avoid a duplicate strlen() calculation. However, I can
> store the length before the conditional.

I'd give 50/50 odds no whether a compiler could optimize out that
strlen. We inline addstr exactly so that callsites can see that strlen
(it's primarily for string literals, where it can become a compile-time
constant, but I think it could apply here). But sometimes C's pointer
aliasing rules can be surprising in blocking "obviously correct"
optimizations like that.

The generated asm is a little dense, but I _think_ "gcc -O2" does in
fact do this with a single strlen based on the following tweak on top of
your patch:

diff --git a/sha1_file.c b/sha1_file.c
index 2160323c4a..f234519744 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1921,11 +1921,12 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
if (is_dot_or_dotdot(de->d_name))
continue;
 
+   strbuf_setlen(path, baselen);
+   strbuf_addstr(path, de->d_name);
+
if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
!hex_to_bytes(oid.hash + 1, de->d_name,
  GIT_SHA1_RAWSZ - 1)) {
-   strbuf_setlen(path, baselen);
-   strbuf_add(path, de->d_name, GIT_SHA1_HEXSZ - 2);
if (obj_cb) {
r = obj_cb(, path->buf, data);
if (r)

Not that I overly mind the manual assignment of the strlen result in
this particular case. But I'm a curious fellow by nature, and knowing
these kinds of answers helps us build up an accurate gut instinct for
future cases.

> Small change by storing the length in advance of the conditional:
> 
> while (de = readdir(...)) {
>     int namelen = strlen(de->d_name);
>     strbuf_setlen(path, baselen);
>     strbuf_add(path, de->d_name, namelen);
> 
>     if (namelen == HEXSZ - 2)
>         obj_cb(path->buf)
>     else
>         cruft_cb(path->buf);
> }

Yup, I don't mind that approach either, but do please use size_t to
store the result of strlen (I know it's nearly impossible to overflow in
this case, but I've been trying to push the codebase in that direction
slowly over time).

> >- there's an extra micro-optimization there, which is that if there's
> >  no obj_cb, we have no need to assemble the full path at all. I doubt
> >  it makes much of a difference, as most callers would pass an object
> >  callback (I'd be surprised if we even have one that doesn't).
> 
> After doing a few 'git grep' commands, I found several that include a NULL
> cruft_cb but none that have a NULL obj_cb.

Yeah, that agrees with my cursory look.

Thanks!

-Peff

[PATCH] l10n: update de_DE translation

[Robert Abel]

Der-, die- and dasselbe and their declensions are spelt as one word in German.

Signed-off-by: Robert Abel 
---
 po/de.po | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/po/de.po b/po/de.po
index a05aca5f3..400262625 100644
--- a/po/de.po
+++ b/po/de.po
@@ -2925,7 +2925,7 @@ msgstr "  (benutzen Sie \"git branch --unset-upstream\" 
zum Beheben)\n"
 #: remote.c:2083
 #, c-format
 msgid "Your branch is up to date with '%s'.\n"
-msgstr "Ihr Branch ist auf dem selben Stand wie '%s'.\n"
+msgstr "Ihr Branch ist auf demselben Stand wie '%s'.\n"
 
 #: remote.c:2087
 #, c-format
@@ -10874,7 +10874,7 @@ msgstr "Kann nicht überschreiben"
 
 #: builtin/mv.c:239
 msgid "multiple sources for the same target"
-msgstr "mehrere Quellen für das selbe Ziel"
+msgstr "mehrere Quellen für dasselbe Ziel"
 
 #: builtin/mv.c:241
 msgid "destination directory does not exist"
@@ -11827,7 +11827,7 @@ msgstr ""
 "\n"
 "git push %s HEAD:%s\n"
 "\n"
-"Um auf den Branch mit dem selben Namen im Remote-Repository zu versenden,\n"
+"Um auf den Branch mit demselben Namen im Remote-Repository zu versenden,\n"
 "benutzen Sie:\n"
 "\n"
 "git push %s %s\n"
-- 
2.15.1

Re: git-clone ignores submodule.recurse configuration

[Brandon Williams]

On 12/01, Ralf Thielow wrote:
> Today I played around a bit with git submodules and noticed
> that the very handy configuration "submodule.recurse" is not
> working for the git-clone command.
> 
> "git help config" tells me that submodule.recurse affects
> all commands that have a --recurse-submodules option.
> git-clone seems to be an exception which is also mentioned in
> a workflow example in Documentation/gitsubmodules.txt that
> says:
> 
>   # Unlike the other commands below clone still needs
>   # its own recurse flag:
>   git clone --recurse  
>   cd 
> 
> So this seems to be a known issue. Is someone already
> working on this or has a plan to do it? Or is there a reason
> not doing this for git-clone but for git-pull?

When we introduced the "submodule.recurse" config we explicitly had it
not work with clone because a recursive clone ends up pulling data from
other sources aside from the URL the user explicitly provides.
Historically there have been a number of security related bugs with
respect to cloning submodules so we felt it was best to require a
recursive clone to be requested explicitly, at least for the time being.

-- 
Brandon Williams

Re: [BUG] git gui can't commit multiple files

[Timon]

Originally I had this problem in gentoo and assumed in the end it's
likely due to my specific configuration.
However recently I switched to nixos and am still experiencing it.

I've search again if I can find anything and lo and behold, it's
already fixed in the *windows* version of git-gui...

issue thread: https://github.com/git-for-windows/git/issues/1012
commit: 
https://github.com/git-for-windows/git/commit/1c4d4e7cbcf404c168df5537d55e9afd57f2b01b

I applied it locally to git-2.15.0 and can confirm it fixed the problem for me.
If you need any more info/help from me, or would like me to test
anything, feel free to ask.

On 12/5/16, Timon  wrote:
> On 12/4/16, David Aguilar  wrote:
>> On Sun, Dec 04, 2016 at 05:36:46PM +0100, Timon wrote:
>>> This is a regression in git 2.11.0 (version 2.10.2 is fine).
>>>
>>> In git-gui I select multiple files in the Unstaged Changes (using
>>> shift+click) and press ctrl+t to stage them. Then only one files gets
>>> staged instead of all of the selected files.
>>> The same happens when unstaging files.
>>>
>>> Git-cola also exhibits the same behavior. Although there I could stage
>>> multiple files if I used a popup menu instead of the keyboard shortcut
>>> (I'm guessing it goes through a different code path?).
>>
>> Can you elaborate a bit?
>>
>> I just tested git-cola with Git 2.11 and it worked fine for me.
>> I selected several files and used the Ctrl+s hotkey to stage the
>> selected files.  They all got staged.
>>
>> If you have a test repo, or reproduction recipe, I'd be curious
>> to try it out.
>> --
>> David
>>
>
> Can you try with git gui?
> Though I guess it's probably specific to my distro or configuration.
> I'm running 64bit gentoo with:
> linux 4.8.12
> gcc 5.4.0
> glibc 2.23-r3
> tk 8.6.6
> gettext 0.19.8.1
> openssl 1.0.2j
> Not sure if that's helpful though.
>

Re: [PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test

[Jonathan Nieder]

Todd Zullinger wrote:

> Reviewed-by: Jonathan Nieder 
> Signed-off-by: Todd Zullinger 
> ---
>  t/lib-git-svn.sh | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

This and the previous one are indeed still
Reviewed-by: Jonathan Nieder 

Thanks.

Re: [PATCH] sha1_file: use strbuf_add() instead of strbuf_addf()

[Derrick Stolee]

On 12/1/2017 1:22 PM, Jeff King wrote:

On Fri, Dec 01, 2017 at 12:49:56PM -0500, Derrick Stolee wrote:

[snip]

diff --git a/sha1_file.c b/sha1_file.c
index 8ae6cb6285..2160323c4a 100644

This overall looks good, but I noticed one bug and a few cosmetic
improvements.


Thanks for finding quality improvements to this patch. I'll let it sit 
over the weekend for more feedback before sending a v2.





--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1914,17 +1914,18 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
}
  
  	oid.hash[0] = subdir_nr;

+   strbuf_add(path, "/", 1);

Maybe:

   strbuf_addch(path, '/');

would be a bit more readable (it's also a tiny bit faster, but this
isn't part of the tight loop).


+   baselen = path->len;

We set this here so that the '/' is included as part of the base. Makes
sense, but can we now drop the earlier setting of baselen before the
opendir() call?


Yeah, probably. I had briefly considered just adding the '/' before the 
first assignment of "baselen", but didn't want to change the error 
output. I also don't know if there are side effects for other platforms 
by calling opendir() with a '/'-terminated path.



while ((de = readdir(dir))) {
if (is_dot_or_dotdot(de->d_name))
continue;
  
-		strbuf_setlen(path, baselen);

-   strbuf_addf(path, "/%s", de->d_name);
-
if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
!hex_to_bytes(oid.hash + 1, de->d_name,
  GIT_SHA1_RAWSZ - 1)) {
+   strbuf_setlen(path, baselen);
+   strbuf_add(path, de->d_name, GIT_SHA1_HEXSZ - 2);

Moving this code into the conditional makes sense here, since that's
where we know the actual size. But what about the case when we _don't_
hit this conditional. We still do:

   if (cruft_cb)
 cruft_cb(path->buf);

which is now broken (it will just get the directory name without the
entry appended).


Good catch! A big reason to pull it inside and use strbuf_add over 
strbuf_addstr is to avoid a duplicate strlen() calculation. However, I 
can store the length before the conditional.



I think the optimized versions probably needs to be something more like:

   while (de = readdir(...)) {
   strbuf_setlen(path, baselen);
   if (size is HEXSZ - 2) {
   strbuf_add(path, de->d_name, HEXSZ - 2);
  obj_cb(path->buf);
   } else if (cruft_cb) {
   strbuf_addstr(path, de->d_name);
  cruft_cb(path->buf);
   }
   }


Small change by storing the length in advance of the conditional:

while (de = readdir(...)) {
    int namelen = strlen(de->d_name);
    strbuf_setlen(path, baselen);
    strbuf_add(path, de->d_name, namelen);

    if (namelen == HEXSZ - 2)
        obj_cb(path->buf)
    else
        cruft_cb(path->buf);
}


Two other thoughts:

   - from past discussions, I suspect most of your performance
 improvement actually comes from avoiding addf(), and the difference
 between addstr() and add() may be negligible here. It might be worth
 timing strbuf_addstr(). If that's similarly fast, that means we
 could keep the logic out of the conditional entirely.


addstr() duplicates the strlen(), which isn't much but we might as well 
save cycles where we can if it isn't too hard.



   - there's an extra micro-optimization there, which is that if there's
 no obj_cb, we have no need to assemble the full path at all. I doubt
 it makes much of a difference, as most callers would pass an object
 callback (I'd be surprised if we even have one that doesn't).


After doing a few 'git grep' commands, I found several that include a 
NULL cruft_cb but none that have a NULL obj_cb.


Thanks,
-Stolee

Re: [PATCH v4 1/4] Makefile: generate Perl header from template file

[Andreas Schwab]

On Dez 01 2017, Dan Jacques  wrote:

> I am not a `sed` wizard, but perhaps the tool is ignoring the semicolon 
> because
> it's in the middle of the "s" expression? The shell may similarly be ignoring 
> it
> because it's nested in between single-quotes?

As far as POSIX is concerned, semicolons are not special, but can be
used to separate commands instead of newlines.

Andreas.

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

git-clone ignores submodule.recurse configuration

[Ralf Thielow]

Today I played around a bit with git submodules and noticed
that the very handy configuration "submodule.recurse" is not
working for the git-clone command.

"git help config" tells me that submodule.recurse affects
all commands that have a --recurse-submodules option.
git-clone seems to be an exception which is also mentioned in
a workflow example in Documentation/gitsubmodules.txt that
says:

  # Unlike the other commands below clone still needs
  # its own recurse flag:
  git clone --recurse  
  cd 

So this seems to be a known issue. Is someone already
working on this or has a plan to do it? Or is there a reason
not doing this for git-clone but for git-pull?

[no subject]

[@United Nation]

The United Nations, World Bank-Group and International Monetary Fund have 
agreed to compensate you with the sum of One Million Five Hundred Thousand US 
Dollars contact us for cliams

RE: [RFE] Inverted sparseness

[Randall S. Becker]

On December 1, 2017 1:19 PM, Jeff Hostetler wrote:
>On 12/1/2017 12:21 PM, Randall S. Becker wrote:
>> I recently encountered a really strange use-case relating to sparse 
>> clone/fetch that is really backwards from the discussion that has been going 
>> on, and well, I'm a bit embarrassed to bring it up, but I have no good 
>> solution including building a separate data store that will end up 
>> inconsistent with repositories (a bad solution).  The use-case is as follows:
>> 
>> Given a backbone of multiple git repositories spread across an organization 
>> with a server farm and upstream vendors.
>> The vendor delivers code by having the client perform git pull into a 
>> specific branch.
>> The customer may take the code as is or merge in customizations.
>> The vendor wants to know exactly what commit of theirs is installed on each 
>> server, in near real time.
>> The customer is willing to push the commit-ish to the vendor's upstream repo 
>> but does not want, by default, to share the actual commit contents for 
>> security reasons.
>>  Realistically, the vendor needs to know that their own commit id was 
>> put somewhere (process exists to track this, so not part of the use-case) 
>> and whether there is a subsequent commit contributed >by the customer, but 
>> the content is not relevant initially.
>> 
>> After some time, the vendor may request the commit contents from the 
>> customer in order to satisfy support requirements - a.k.a. a defect was 
>> found but has to be resolved.
>> The customer would then perform a deeper push that looks a lot like a 
>> "slightly" symmetrical operation of a deep fetch following a prior sparse 
>> fetch to supply the vendor with the specific commit(s).

>Perhaps I'm not understanding the subtleties of what you're describing, but 
>could you do this with stock git functionality.

>Let the vendor publish a "well known branch" for the client.
>Let the client pull that and build.
>Let the client create a branch set to the same commit that they fetched.
>Let the client push that branch as a client-specific branch to the vendor to 
>indicate that that is the official release they are based on.

>Then the vendor would know the official commit that the client was using.
This is the easy part, and it doesn't require anything sparse to exist.

>If the client makes local changes, does the vendor really need the SHA of 
>those -- without the actual content?
>I mean any SHA would do right?  Perhaps let the client create a second 
>client-specific branch (set to
> the same commit as the first) to indicate they had mods.
>Later, when the vendor needs the actual client changes, the client does a 
>normal push to this 2nd client-specific branch at the vendor.
>This would send everything that the client has done to the code since the 
>official release.

What I should have added to the use-case was that there is a strong audit 
requirement (regulatory, actually) involved that the SHA is exact, immutable, 
and cannot be substitute or forged (one of the reasons git is in such high 
regard). So, no I can't arrange a fake SHA to represent a SHA to be named 
later. It SHA of the installed commit is part of the official record of what 
happened on the specific server, so I'm stuck with it.

>I'm not sure what you mean about "it is inside a tree".

m---a---b---c---H1
  `---d---H2

d would be at a head. b would be inside. Determining content of c is 
problematic if b is sparse, so I'm really unsure that any of this is possible.

Cheers,
Randall

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

Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

[Jeff King]

On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:

> > These are obviously the result of devils-advocate poking at the feature.
> > I doubt any editor would end its output with a CR. But the first case is
> > probably going to be common, especially for actual graphical editors. We
> > know that emacsclient prints its own line, and I wouldn't be surprised
> > if other graphical editors spew some telemetry to stderr (certainly
> > anything built against GTK tends to do so).
> 
> True! However, if a Git user is not bothered by a graphical editor that
> spews telemetry to stderr, then I think the same user wouldn't be
> bothered by one additional line printed by Git either, right?

Yeah, if there's a lot of spew, I agree it probably doesn't matter.

The "emacsclient" one is probably the worst case, because it would print
a single line of its own, which would get tacked onto the "Waiting..."
message printed by Git, since it doesn't end with a newline.

> > The patch itself looks fine, as far as correctly implementing the
> > design.
> 
> Thanks for the review :-)

Actually, I meant to bikeshed one part but forgot. ;)

> +   fprintf(stderr, _("hint: Waiting for your editor 
> input..."));

I found "waiting for editor input" to be a funny way of saying this. I
input to the editor, the editor does not input to Git. :)

Maybe "waiting for your editor finish" or something would make more
sense?

Or given that the goal is really just making it clear that we've spawned
an editor, something like "starting editor %s..." would work. I think
the "waiting for..." pattern is perfectly fine, though.

-Peff

Re: How hard would it be to implement sparse fetching/pulling?

[Jonathan Nieder]

Jeff Hostetler wrote:
> On 11/30/2017 6:43 PM, Philip Oakley wrote:

>> The 'companies' problem is that it tends to force a client-server, always-on
>> on-line mentality. I'm also wanting the original DVCS off-line capability to
>> still be available, with _user_ control, in a generic sense, of what they
>> have locally available (including files/directories they have not yet looked
>> at, but expect to have. IIUC Jeff's work is that on-line view, without the
>> off-line capability.
>>
>> I'd commented early in the series at [1,2,3].
>
> Yes, this does tend to lead towards an always-online mentality.
> However, there are 2 parts:
> [a] dynamic object fetching for missing objects, such as during a
> random command like diff or blame or merge.  We need this
> regardless of usage -- because we can't always predict (or
> dry-run) every command the user might run in advance.
> [b] batch fetch mode, such as using partial-fetch to match your
> sparse-checkout so that you always have the blobs of interest
> to you.  And assuming you don't wander outside of this subset
> of the tree, you should be able to work offline as usual.
> If you can work within the confines of [b], you wouldn't need to
> always be online.

Just to amplify this: for our internal use we care a lot about
disconnected usage working.  So it is not like we have forgotten about
this use case.

> We might also add a part [c] with explicit commands to back-fill or
> alter your incomplete view of the ODB

Agreed, this will be a nice thing to add.

[...]
>> At its core, my idea was to use the object store to hold markers for the
>> 'not yet fetched' objects (mainly trees and blobs). These would be in a
>> known fixed format, and have the same effect (conceptually) as the
>> sub-module markers - they _confirm_ the oid, yet say 'not here, try
>> elsewhere'.
>
> We do have something like this.  Jonathan can explain better than I, but
> basically, we denote possibly incomplete packfiles from partial clones
> and fetches as "promisor" and have special rules in the code to assert
> that a missing blob referenced from a "promisor" packfile is OK and can
> be fetched later if necessary from the "promising" remote.
>
> The main problem with markers or other lists of missing objects is
> that it has scale problems for large repos.

Any chance that we can get a design doc in Documentation/technical/
giving an overview of the design, with a brief "alternatives
considered" section describing this kind of thing?

E.g. some of the earlier descriptions like
 
https://public-inbox.org/git/20170915134343.3814d...@twelve2.svl.corp.google.com/
 https://public-inbox.org/git/cover.1506714999.git.jonathanta...@google.com/
 https://public-inbox.org/git/20170113155253.1644-1-benpe...@microsoft.com/
may help as a starting point.

Thanks,
Jonathan

Re: [PATCH] sha1_file: use strbuf_add() instead of strbuf_addf()

[Jeff King]

On Fri, Dec 01, 2017 at 12:49:56PM -0500, Derrick Stolee wrote:

> Replace use of strbuf_addf() with strbuf_add() when enumerating
> loose objects in for_each_file_in_obj_subdir(). Since we already
> check the length and hex-values of the string before consuming
> the path, we can prevent extra computation by using the lower-
> level method.

Makes sense. It's a shame that "addf" is turning out to be so slow (I'm
still mildly curious if that differs between Windows and glibc, but
there are so many other variables between the two platforms it's hard to
test).

> One consumer of for_each_file_in_obj_subdir() is the abbreviation
> code. OID abbreviations use a cached list of loose objects (per
> object subdirectory) to make repeated queries fast, but there is
> significant cache load time when there are many loose objects.
> 
> Most repositories do not have many loose objects before repacking,
> but in the GVFS case the repos can grow to have millions of loose
> objects. Profiling 'git log' performance in GitForWindows on a
> GVFS-enabled repo with ~2.5 million loose objects revealed 12% of
> the CPU time was spent in strbuf_addf().

Yeah, we haven't heavily micro-optimized the case for having lots of
loose objects. The general philosophy about having lots of loose objects
is: don't. You're generally going to pay a system call for each access,
which is much heavier-weight than packfiles.

I think abbreviation-finding is the exception there, though, because we
literally just readdir() the entries and never do anything else with
them.

> Add a new performance test to p4211-line-log.sh that is more
> sensitive to this cache-loading. By limiting to 1000 commits, we
> more closely resemble user wait time when reading history into a
> pager.
> 
> For a copy of the Linux repo with two ~512 MB packfiles and ~572K
> loose objects, running 'git log --oneline --raw --parents -1000'
> had the following performance:
> 
>  HEAD~1HEAD
> 
>  7.70(7.15+0.54)   7.44(7.09+0.29) -3.4%

Thanks for including numbers. I think the setup here highlights a
weakness of the perf suite that we've discussed: if there's a
performance regression for your case, nobody is likely to notice because
they won't test on a repo with 500k loose objects.

Probably "repo with a bunch of loose objects" ought to be a stock
repository profile for doing regression runs.

> diff --git a/sha1_file.c b/sha1_file.c
> index 8ae6cb6285..2160323c4a 100644

This overall looks good, but I noticed one bug and a few cosmetic
improvements.

> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1914,17 +1914,18 @@ int for_each_file_in_obj_subdir(unsigned int 
> subdir_nr,
>   }
>  
>   oid.hash[0] = subdir_nr;
> + strbuf_add(path, "/", 1);

Maybe:

  strbuf_addch(path, '/');

would be a bit more readable (it's also a tiny bit faster, but this
isn't part of the tight loop).

> + baselen = path->len;

We set this here so that the '/' is included as part of the base. Makes
sense, but can we now drop the earlier setting of baselen before the
opendir() call?

>   while ((de = readdir(dir))) {
>   if (is_dot_or_dotdot(de->d_name))
>   continue;
>  
> - strbuf_setlen(path, baselen);
> - strbuf_addf(path, "/%s", de->d_name);
> -
>   if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
>   !hex_to_bytes(oid.hash + 1, de->d_name,
> GIT_SHA1_RAWSZ - 1)) {
> + strbuf_setlen(path, baselen);
> + strbuf_add(path, de->d_name, GIT_SHA1_HEXSZ - 2);

Moving this code into the conditional makes sense here, since that's
where we know the actual size. But what about the case when we _don't_
hit this conditional. We still do:

  if (cruft_cb)
cruft_cb(path->buf);

which is now broken (it will just get the directory name without the
entry appended).

I think the optimized versions probably needs to be something more like:

  while (de = readdir(...)) {
  strbuf_setlen(path, baselen);
  if (size is HEXSZ - 2) {
  strbuf_add(path, de->d_name, HEXSZ - 2);
  obj_cb(path->buf);
  } else if (cruft_cb) {
  strbuf_addstr(path, de->d_name);
  cruft_cb(path->buf);
  }
  }

Two other thoughts:

  - from past discussions, I suspect most of your performance
improvement actually comes from avoiding addf(), and the difference
between addstr() and add() may be negligible here. It might be worth
timing strbuf_addstr(). If that's similarly fast, that means we
could keep the logic out of the conditional entirely.

  - there's an extra micro-optimization there, which is that if there's
no obj_cb, we have no need to assemble the full path at all. I doubt
it makes much of a difference, as most callers would pass an object
callback (I'd be surprised if we even have one that doesn't).

-Peff

Re: [PATCH v4 1/4] Makefile: generate Perl header from template file

[Dan Jacques]

On Fri, 1 Dec 2017, Johannes Sixt wrote:

>>> This doesn't work, unfortunately. When $(pathsep) is ';', we get an
>>> incomplete sed expression because ';' is also a command separator in the 
>>> sed language.
>>
>> Funny, I tried this also with ';' as pathsep, and it worked in the Git for
>> Windows SDK...
>
> My ancient sed vs. your modern sed, perhaps? I can check this on Monday.

If you wouldn't mind, that would be much appreciated. Did you actually observe
this issue, or is it just coming up in review?

I am not a `sed` wizard, but perhaps the tool is ignoring the semicolon because
it's in the middle of the "s" expression? The shell may similarly be ignoring it
because it's nested in between single-quotes?

-Dan

Re: [RFE] Inverted sparseness

[Jeff Hostetler]

On 12/1/2017 12:21 PM, Randall S. Becker wrote:

I recently encountered a really strange use-case relating to sparse clone/fetch 
that is really backwards from the discussion that has been going on, and well, 
I'm a bit embarrassed to bring it up, but I have no good solution including 
building a separate data store that will end up inconsistent with repositories 
(a bad solution).  The use-case is as follows:

Given a backbone of multiple git repositories spread across an organization 
with a server farm and upstream vendors.
The vendor delivers code by having the client perform git pull into a specific 
branch.
The customer may take the code as is or merge in customizations.
The vendor wants to know exactly what commit of theirs is installed on each 
server, in near real time.
The customer is willing to push the commit-ish to the vendor's upstream repo 
but does not want, by default, to share the actual commit contents for security 
reasons.
Realistically, the vendor needs to know that their own commit id was 
put somewhere (process exists to track this, so not part of the use-case) and 
whether there is a subsequent commit contributed by the customer, but the 
content is not relevant initially.

After some time, the vendor may request the commit contents from the customer 
in order to satisfy support requirements - a.k.a. a defect was found but has to 
be resolved.
The customer would then perform a deeper push that looks a lot like a 
"slightly" symmetrical operation of a deep fetch following a prior sparse fetch 
to supply the vendor with the specific commit(s).

This is not hard to realize if the sparse commit is HEAD on a branch, but if 
its inside a tree, well, I don't even know where to start. To self-deprecate, 
this is likely a bad idea, but it has come up a few times.

Thoughts? Nasty Remarks?

Randall


Perhaps I'm not understanding the subtleties of what you're describing,
but could you do this with stock git functionality.

Let the vendor publish a "well known branch" for the client.
Let the client pull that and build.
Let the client create a branch set to the same commit that they fetched.
Let the client push that branch as a client-specific branch to
the vendor to indicate that that is the official release they
are based on.

Then the vendor would know the official commit that the client was
using.

If the client makes local changes, does the vendor really need the
SHA of those -- without the actual content?  I mean any SHA would
do right?  Perhaps let the client create a second client-specific
branch (set to the same commit as the first) to indicate they had
mods.

Later, when the vendor needs the actual client changes, the client
does a normal push to this 2nd client-specific branch at the vendor.
This would send everything that the client has done to the code
since the official release.

I'm not sure what you mean about "it is inside a tree".

Hope this helps,
Jeff

Re: How hard would it be to implement sparse fetching/pulling?

[Jonathan Nieder]

Hi,

Jeff Hostetler wrote:
> On 11/30/2017 3:03 PM, Jonathan Nieder wrote:

>> One piece of missing functionality that looks intereseting to me: that
>> series batches fetches of the missing blobs involved in a "git
>> checkout" command:
>>
>>   https://public-inbox.org/git/20171121211528.21891-14-...@jeffhostetler.com/
>>
>> But if doesn't batch fetches of the missing blobs involved in a "git
>> diff  " command.  That might be a good place to get
>> your hands dirty. :)
>
> Jonathan Tan added code in unpack-trees to bulk fetch missing blobs
> before a checkout.  This is limited to the missing blobs needed for
> the target commit.  We need this to make checkout seamless, but it
> does mean that checkout may need online access.

Just to clarify: other parts of the series already fetch all missing
blobs that are required for a command.  What that bulk-fetch patch
does is to make that more efficient, by using a single fetch request
to grab all the blobs that are needed for checkout, instead of one
fetch per blob.

This doesn't change the online access requirement: online access is
needed if and only if you don't have the required objects already
available locally.  For example, if at clone time you specified a
sparse checkout pattern and you haven't changed that sparse checkout
pattern, then online access is not needed for checkout.

> I've also talked about a pre-fetch capability to bulk fetch missing
> blobs in advance of some operation.  You could speed up the above
> diff command or back-fill all the blobs I might need before going
> offline for a while.

In particular, something like this seems like a very valuable thing to
have when changing the sparse checkout pattern.

Thanks,
Jonathan

[PATCH] sha1_file: use strbuf_add() instead of strbuf_addf()

[Derrick Stolee]

Replace use of strbuf_addf() with strbuf_add() when enumerating
loose objects in for_each_file_in_obj_subdir(). Since we already
check the length and hex-values of the string before consuming
the path, we can prevent extra computation by using the lower-
level method.

One consumer of for_each_file_in_obj_subdir() is the abbreviation
code. OID abbreviations use a cached list of loose objects (per
object subdirectory) to make repeated queries fast, but there is
significant cache load time when there are many loose objects.

Most repositories do not have many loose objects before repacking,
but in the GVFS case the repos can grow to have millions of loose
objects. Profiling 'git log' performance in GitForWindows on a
GVFS-enabled repo with ~2.5 million loose objects revealed 12% of
the CPU time was spent in strbuf_addf().

Add a new performance test to p4211-line-log.sh that is more
sensitive to this cache-loading. By limiting to 1000 commits, we
more closely resemble user wait time when reading history into a
pager.

For a copy of the Linux repo with two ~512 MB packfiles and ~572K
loose objects, running 'git log --oneline --raw --parents -1000'
had the following performance:

 HEAD~1HEAD

 7.70(7.15+0.54)   7.44(7.09+0.29) -3.4%

Signed-off-by: Derrick Stolee 
---
 sha1_file.c  | 7 ---
 t/perf/p4211-line-log.sh | 4 
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 8ae6cb6285..2160323c4a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1914,17 +1914,18 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
}
 
oid.hash[0] = subdir_nr;
+   strbuf_add(path, "/", 1);
+   baselen = path->len;
 
while ((de = readdir(dir))) {
if (is_dot_or_dotdot(de->d_name))
continue;
 
-   strbuf_setlen(path, baselen);
-   strbuf_addf(path, "/%s", de->d_name);
-
if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&
!hex_to_bytes(oid.hash + 1, de->d_name,
  GIT_SHA1_RAWSZ - 1)) {
+   strbuf_setlen(path, baselen);
+   strbuf_add(path, de->d_name, GIT_SHA1_HEXSZ - 2);
if (obj_cb) {
r = obj_cb(, path->buf, data);
if (r)
diff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh
index e0ed05907c..392bcc0e51 100755
--- a/t/perf/p4211-line-log.sh
+++ b/t/perf/p4211-line-log.sh
@@ -35,4 +35,8 @@ test_perf 'git log --oneline --raw --parents' '
git log --oneline --raw --parents >/dev/null
 '
 
+test_perf 'git log --oneline --raw --parents -1000' '
+   git log --oneline --raw --parents -1000 >/dev/null
+'
+
 test_done
-- 
2.15.0

Re: [PATCH v4 1/4] Makefile: generate Perl header from template file

[Johannes Sixt]

Am 01.12.2017 um 18:13 schrieb Johannes Schindelin:

Hi Hannes,

On Fri, 1 Dec 2017, Johannes Sixt wrote:


Am 29.11.2017 um 16:56 schrieb Dan Jacques:

@@ -1989,6 +1986,15 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
   +GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak
Makefile
+   $(QUIET_GEN)$(RM) $@ && \
+   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory
instlibdir` && \
+   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" &&
\
+   sed -e 's=@@PATHSEP@@=$(pathsep)=g' \


This doesn't work, unfortunately. When $(pathsep) is ';', we get an incomplete
sed expression because ';' is also a command separator in the sed language.


Funny, I tried this also with ';' as pathsep, and it worked in the Git for
Windows SDK...


My ancient sed vs. your modern sed, perhaps? I can check this on Monday.

-- Hannes

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

[Johannes Sixt]

Am 30.11.2017 um 00:10 schrieb Igor Djordjevic:

On 29/11/2017 20:11, Johannes Sixt wrote:

With git-post, I make a fixup commit commit on the integration
branch, then `git post B && git merge B`:

  ...A...C  <- topics A, C
  \   \
---o---o---o---o---f---F<- integration
  /   /   /
  ...B...D   /  <- topic D
  \ /
   f'--'<- topic B

The merge F does not introduce any changes on the integration branch,
so I do not need it, but it helps keep topic B off radar when I ask
`git branch --no-merged` later.


But you`re not committing (posting) on your HEAD`s (direct) parent in
the first place (topic B), so `commit --onto-parent` isn`t right tool
for the job... yet :)


Ohh..kay.


To work with `--onto-parent` and be able to commit on top of any of
the topic branches, you would need a situation like this instead:

  (1)  ...C  <- topic C
  |
 ...A |  <- topic A
 \|
   ...o I<- integration
 /|
 ...B |  <- topic B
  |
   ...D  <- topic D


This is a very, VERY exotic workflow, I would say. How would you 
construct commit I when three or more topics have conflicts? 
merge-octopus does not support this use-case.



With `commit --onto-parent` you would skip `git post B && git merge
B` steps, where "fixup commit" would be done with `--onto-parent B`,
So you end up in situation like this:

  (2)  ...C  <- topic C
  |
 ...A |  <- topic A
 \|
   ...o I'   <- integration
 /|
 ...B---f |  <- topic B
  |
   ...D  <- topic D

State of index and working tree files in your F and my I' commit is
exactly the same (I' = I + f), where in my case (2) history looks
like "f" was part of topic B from the start, before integration
merge happened.

BUT, all this said, I understand that your starting position, where
not all topic branches are merged at the same time (possibly to keep
conflict resolution sane), is probably more often to be encountered
in real-life work... and as existing `--onto-parent` machinery should
be able to support it already, I`m looking forward to make it happen :)

Once there, starting from your initial position:


...A...C<- topics A, C
\   \ E
  ---o---o---o---o I<- integration <- HEAD
/   /
...B...D<- topics B, D


... and doing something like `git commit --onto B --merge` would yield:
  
  (3) ...A...C<- topics A, C

  \   \ E
---o---o---o---o I'   <- integration
  /   /|
  ...B...D |  <- topic D
  \|
   f---'  <- topic B

... where (I' = I + f) is still true.


I am not used to this picture. I would not think that it is totally 
unacceptable, but it still has a hmm-factor.



If that`s preferred in some
cases, it could even look like this instead:

  (4) ...A...C <- topics A, C
  \   \ E  I
---o---o---o---o---F   <- integration
  /   /   /
  ...B...D   / <- topic D
  \ /
   f---'   <- topic B

... where F(4) = I'(3), so similar situation, just that we don`t
discard I but post F on top of it.


This is very acceptable.

Nevertheless, IMO, it is not the task of git-commit to re-compute a 
merge commit. It would be OK that it commits changes on top of a branch 
that is not checked out. Perhaps it would even be OK to remove the 
change from the current workspace (index and worktree), because it will 
return in the form of a merge later, but producing that merge is 
certainly not the task of git-commit.


-- Hannes

Re: How hard would it be to implement sparse fetching/pulling?

[Jeff Hostetler]

On 11/30/2017 6:43 PM, Philip Oakley wrote:

From: "Vitaly Arbuzov" 

[...]

comments below..


On Thu, Nov 30, 2017 at 9:01 AM, Vitaly Arbuzov  wrote:

Hey Jeff,

It's great, I didn't expect that anyone is actively working on this.
I'll check out your branch, meanwhile do you have any design docs that
describe these changes or can you define high level goals that you
want to achieve?

On Thu, Nov 30, 2017 at 6:24 AM, Jeff Hostetler 
wrote:



On 11/29/2017 10:16 PM, Vitaly Arbuzov wrote:

[...]




I have, for separate reasons been _thinking_ about the issue ($dayjob is in
defence, so a similar partition would be useful).

The changes would almost certainly need to be server side (as well as 
client
side), as it is the server that decides what is sent over the wire in 
the pack files, which would need to be a 'narrow' pack file.


Yes, there will need to be both client and server changes.
In the current 3 part patch series, the client sends a "filter_spec"
to the server as part of the fetch-pack/upload-pack protocol.
If the server chooses to honor it, upload-pack passes the filter_spec
to pack-objects to build an "incomplete" packfile omitting various
objects (currently blobs).  Proprietary servers will need similar
changes to support this feature.

Discussing this feature in the context of the defense industry
makes me a little nervous.  (I used to be in that area.)
What we have in the code so far may be a nice start, but
probably doesn't have the assurances that you would need
for actual deployment.  But it's a start




If we had such a feature then all we would need on top is a separate
tool that builds the right "sparse" scope for the workspace based on
paths that developer wants to work on.

In the world where more and more companies are moving towards large
monorepos this improvement would provide a good way of scaling git to
meet this demand.


The 'companies' problem is that it tends to force a client-server, 
always-on
on-line mentality. I'm also wanting the original DVCS off-line 
capability to

still be available, with _user_ control, in a generic sense, of what they
have locally available (including files/directories they have not yet 
looked

at, but expect to have. IIUC Jeff's work is that on-line view, without the
off-line capability.

I'd commented early in the series at [1,2,3].


Yes, this does tend to lead towards an always-online mentality.
However, there are 2 parts:
[a] dynamic object fetching for missing objects, such as during a
random command like diff or blame or merge.  We need this
regardless of usage -- because we can't always predict (or
dry-run) every command the user might run in advance.
[b] batch fetch mode, such as using partial-fetch to match your
sparse-checkout so that you always have the blobs of interest
to you.  And assuming you don't wander outside of this subset
of the tree, you should be able to work offline as usual.
If you can work within the confines of [b], you wouldn't need to
always be online.

We might also add a part [c] with explicit commands to back-fill or
alter your incomplete view of the ODB (as I explained in response
to the "git diff  " comment later in this thread.



At its core, my idea was to use the object store to hold markers for the
'not yet fetched' objects (mainly trees and blobs). These would be in a 
known fixed format, and have the same effect (conceptually) as the 
sub-module markers - they _confirm_ the oid, yet say 'not here, try 
elsewhere'.


We do have something like this.  Jonathan can explain better than I, but
basically, we denote possibly incomplete packfiles from partial clones
and fetches as "promisor" and have special rules in the code to assert
that a missing blob referenced from a "promisor" packfile is OK and can
be fetched later if necessary from the "promising" remote.

The main problem with markers or other lists of missing objects is
that it has scale problems for large repos.  Suppose I have 100M
blobs in my repo.  If I do a blob:none clone, I'd have 100M missing
blobs that would need tracking.  If I then do a batch fetch of the
blobs needed to do a sparse checkout of HEAD, I'd have to remove
those entries from the tracking data.  Not impossible, but not
speedy either.



The comaprison with submodules mean there is the same chance of
de-synchronisation with triangular and upstream servers, unless managed.

The server side, as noted, will need to be included as it is the one that
decides the pack file.

Options for a server management are:

- "I accept narrow packs?" No; yes

- "I serve narrow packs?" No; yes.

- "Repo completeness checks on reciept": (must be complete) || (allow 
narrow to nothing).


we have new config settings for the server to allow/reject
partial clones.

and we have code in fsck/gc to handle these incomplete packfiles.



For server farms (e.g. Github..) the settings could be global, or by repo.
(note that 

[RFE] Inverted sparseness

[Randall S. Becker]

I recently encountered a really strange use-case relating to sparse clone/fetch 
that is really backwards from the discussion that has been going on, and well, 
I'm a bit embarrassed to bring it up, but I have no good solution including 
building a separate data store that will end up inconsistent with repositories 
(a bad solution).  The use-case is as follows:

Given a backbone of multiple git repositories spread across an organization 
with a server farm and upstream vendors.
The vendor delivers code by having the client perform git pull into a specific 
branch.
The customer may take the code as is or merge in customizations.
The vendor wants to know exactly what commit of theirs is installed on each 
server, in near real time.
The customer is willing to push the commit-ish to the vendor's upstream repo 
but does not want, by default, to share the actual commit contents for security 
reasons.
Realistically, the vendor needs to know that their own commit id was 
put somewhere (process exists to track this, so not part of the use-case) and 
whether there is a subsequent commit contributed by the customer, but the 
content is not relevant initially.

After some time, the vendor may request the commit contents from the customer 
in order to satisfy support requirements - a.k.a. a defect was found but has to 
be resolved.
The customer would then perform a deeper push that looks a lot like a 
"slightly" symmetrical operation of a deep fetch following a prior sparse fetch 
to supply the vendor with the specific commit(s).

This is not hard to realize if the sparse commit is HEAD on a branch, but if 
its inside a tree, well, I don't even know where to start. To self-deprecate, 
this is likely a bad idea, but it has come up a few times.

Thoughts? Nasty Remarks?

Randall

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

Re: [PATCH v4 1/4] Makefile: generate Perl header from template file

[Johannes Schindelin]

Hi Hannes,

On Fri, 1 Dec 2017, Johannes Sixt wrote:

> Am 29.11.2017 um 16:56 schrieb Dan Jacques:
> > @@ -1989,6 +1986,15 @@ GIT-PERL-DEFINES: FORCE
> > echo "$$FLAGS" >$@; \
> >fi
> >   +GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak
> > Makefile
> > +   $(QUIET_GEN)$(RM) $@ && \
> > +   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory
> > instlibdir` && \
> > +   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
> > +   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" &&
> > \
> > +   sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
> 
> This doesn't work, unfortunately. When $(pathsep) is ';', we get an incomplete
> sed expression because ';' is also a command separator in the sed language.

Funny, I tried this also with ';' as pathsep, and it worked in the Git for
Windows SDK...

Ciao,
Dscho

Re: [PATCH v4 1/4] Makefile: generate Perl header from template file

[Johannes Sixt]

Am 29.11.2017 um 16:56 schrieb Dan Jacques:

@@ -1989,6 +1986,15 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
  
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak Makefile

+   $(QUIET_GEN)$(RM) $@ && \
+   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` 
&& \
+   INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+   INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
+   sed -e 's=@@PATHSEP@@=$(pathsep)=g' \


This doesn't work, unfortunately. When $(pathsep) is ';', we get an 
incomplete sed expression because ';' is also a command separator in the 
sed language.



+   -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+   $< >$@+ && \
+   mv $@+ $@
  
  .PHONY: gitweb

  gitweb:


-- Hannes

Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Jeff Hostetler]

On 11/28/2017 8:17 PM, Junio C Hamano wrote:

[Cooking]


* jh/object-filtering (2017-11-22) 6 commits
   (merged to 'next' on 2017-11-27 at e5008c3b28)
  + pack-objects: add list-objects filtering
  + rev-list: add list-objects filtering support
  + list-objects: filter objects in traverse_commit_list
  + oidset: add iterator methods to oidset
  + oidmap: add oidmap iterator methods
  + dir: allow exclusions from blob in addition to file
  (this branch is used by jh/fsck-promisors and jh/partial-clone.)

  In preparation for implementing narrow/partial clone, the object
  walking machinery has been taught a way to tell it to "filter" some
  objects from enumeration.

  Will merge to 'master'.


Could we wait on this.  I'd like to send up a V6 that
addresses the assert() questions raised.  And Jonathan
Nieder had a few suggestions that I want to address.

Thanks.
Jeff

Re: How hard would it be to implement sparse fetching/pulling?

[Jeff Hostetler]

On 11/30/2017 3:03 PM, Jonathan Nieder wrote:

Hi Vitaly,

Vitaly Arbuzov wrote:


Found some details here: https://github.com/jeffhostetler/git/pull/3

Looking at commits I see that you've done a lot of work already,
including packing, filtering, fetching, cloning etc.
What are some areas that aren't complete yet? Do you need any help
with implementation?


That's a great question!  I've filed https://crbug.com/git/2 to track
this project.  Feel free to star it to get updates there, or to add
updates of your own.


Thanks!



As described at https://crbug.com/git/2#c1, currently there are three
patch series for which review would be very welcome.  Building on top
of them is welcome as well.  Please make sure to coordinate with
jeffh...@microsoft.com and jonathanta...@google.com (e.g. through the
bug tracker or email).

One piece of missing functionality that looks intereseting to me: that
series batches fetches of the missing blobs involved in a "git
checkout" command:

  https://public-inbox.org/git/20171121211528.21891-14-...@jeffhostetler.com/

But if doesn't batch fetches of the missing blobs involved in a "git
diff  " command.  That might be a good place to get
your hands dirty. :)


Jonathan Tan added code in unpack-trees to bulk fetch missing blobs
before a checkout.  This is limited to the missing blobs needed for
the target commit.  We need this to make checkout seamless, but it
does mean that checkout may need online access.

I've also talked about a pre-fetch capability to bulk fetch missing
blobs in advance of some operation.  You could speed up the above
diff command or back-fill all the blobs I might need before going
offline for a while.

You can use the options that were added to rev-list to help with this.
For example:
git rev-list --objects [--filter=] --missing=print 
git rev-list --objects [--filter=] --missing=print ..
And then pipe that into a "git fetch-pack --stdin".

You might experiment with this.




Thanks,
Jonathan



Thanks,
Jeff

[PATCH v2 2/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test

[Todd Zullinger]

Setting SVNSERVE_PORT enables several tests which require a local
svnserve daemon to be run (in t9113 & t9126).  The tests share setup of
the local svnserve via `start_svnserve()`.  The function uses svnserve's
`--listen-once` option, which causes svnserve to accept one connection
on the port, serve it, and exit.  When running the tests in parallel
this fails if one test tries to start svnserve while the other is still
running.

Use the test number as the svnserve port (similar to httpd tests) to
avoid port conflicts.  Developers can set GIT_TEST_SVNSERVE to any value
other than 'false' or 'auto' to enable these tests.

Acked-by: Eric Wong 
Reviewed-by: Jonathan Nieder 
Signed-off-by: Todd Zullinger 
---
 t/lib-git-svn.sh | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 84366b2624..4c1f81f167 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -110,14 +110,16 @@ EOF
 }
 
 require_svnserve () {
-   if test -z "$SVNSERVE_PORT"
+   test_tristate GIT_TEST_SVNSERVE
+   if ! test "$GIT_TEST_SVNSERVE" = true
then
-   skip_all='skipping svnserve test. (set $SVNSERVE_PORT to 
enable)'
+   skip_all='skipping svnserve test. (set $GIT_TEST_SVNSERVE to 
enable)'
test_done
fi
 }
 
 start_svnserve () {
+   SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
svnserve --listen-port $SVNSERVE_PORT \
 --root "$rawsvnrepo" \
 --listen-once \
-- 
2.15.1

[PATCH v2 1/2] t/lib-git-svn: cleanup inconsistent tab/space usage

[Todd Zullinger]

Acked-by: Eric Wong 
Reviewed-by: Jonathan Nieder 
Signed-off-by: Todd Zullinger 
---
 t/lib-git-svn.sh | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 688313ed5c..84366b2624 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -17,8 +17,8 @@ SVN_TREE=$GIT_SVN_DIR/svn-tree
 svn >/dev/null 2>&1
 if test $? -ne 1
 then
-skip_all='skipping git svn tests, svn not found'
-test_done
+   skip_all='skipping git svn tests, svn not found'
+   test_done
 fi
 
 svnrepo=$PWD/svnrepo
@@ -110,18 +110,18 @@ EOF
 }
 
 require_svnserve () {
-if test -z "$SVNSERVE_PORT"
-then
-   skip_all='skipping svnserve test. (set $SVNSERVE_PORT to enable)'
-test_done
-fi
+   if test -z "$SVNSERVE_PORT"
+   then
+   skip_all='skipping svnserve test. (set $SVNSERVE_PORT to 
enable)'
+   test_done
+   fi
 }
 
 start_svnserve () {
-svnserve --listen-port $SVNSERVE_PORT \
- --root "$rawsvnrepo" \
- --listen-once \
- --listen-host 127.0.0.1 &
+   svnserve --listen-port $SVNSERVE_PORT \
+--root "$rawsvnrepo" \
+--listen-once \
+--listen-host 127.0.0.1 &
 }
 
 prepare_a_utf8_locale () {
-- 
2.15.1

Re: jn/reproducible-build, was Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Lars Schneider]

> On 01 Dec 2017, at 15:32, Johannes Schindelin  
> wrote:
> 
> Hi Junio & Jonathan (Nieder, there is another active Jonathan again),
> 
> On Wed, 29 Nov 2017, Junio C Hamano wrote:
> 
>> * jn/reproducible-build (2017-11-22) 3 commits
>>  (merged to 'next' on 2017-11-27 at 6ae6946f8c)
>> + Merge branch 'jn/reproducible-build' of ../git-gui into 
>> jn/reproducible-build
>> + git-gui: sort entries in optimized tclIndex
>> + generate-cmdlist: avoid non-deterministic output
>> 
>> The build procedure has been taught to avoid some unnecessary
>> instability in the build products.
> 
> I like this, from a purely security-informed point of view. Maybe there
> would be a way to integrate this with the Continuous Testing we do? Like,
> letting Travis verify that the binaries built from a certain Debian
> package are really identical to the binaries built from the corresponding
> commit? But I guess Travis is the wrong vehicle for this, as Travis needs
> a *commit* to be pushed, not a new package to be made available via apt...

That's a neat idea. We could make TravisCI publish the hashes of our
release builds and then people could check them against the builds that
they have installed. Could that add value as a start?

- Lars

Re: How hard would it be to implement sparse fetching/pulling?

[Jeff Hostetler]

On 11/30/2017 12:44 PM, Vitaly Arbuzov wrote:

Found some details here: https://github.com/jeffhostetler/git/pull/3

Looking at commits I see that you've done a lot of work already,
including packing, filtering, fetching, cloning etc.
What are some areas that aren't complete yet? Do you need any help
with implementation?



Sure.  Extra hands are always welcome.

Jonathan Tan and I have been working on this together.
Our V5 is on the mailing now.  We have privately exchanged
some commits that I hope to push up as a V6 today or Monday.

As for how to help, I'll have to think about that a bit.
Without knowing your experience level in the code or your
availability, it is hard to pick something specific right
now.

As a first step, build my core/pc5_p3 branch and try using
partial clone/fetch between local repos.  You can look at
the tests we added (t0410, t5317, t5616, t6112) to see sample
setups using a local pair of repos.  Then try actually using
the partial clone repo for actual work (dogfooding) and see
how it falls short of your expectations.

You might try duplicating the above tests to use a
local "git daemon" serving the remote and do partial clones
using localhost URLs rather than file:// URLs.  That would
exercise the transport differently.

The t5616 test has the start of some end-to-end tests that
try combine multiple steps -- such as do a partial clone
with no blobs and then run blame on a file.  You could extend
that with more tests that test odd combinations of commands
and confirm that we can handle missing blobs in different
scenarios.

Since you've expressed an interest in sparse-checkout and
having a complete end-to-end experience, you might also
experiment with adapting the above tests to use the sparse
filter (--filter=sparse:oid=) instead of blobs:none
or blobs:limit.  See where that takes you and add tests
as you see fit.  The goal being to get tests in place that
match the usage you want to see (even if they fail) and
see what that looks like.

I know it is not as glamorous as adding new functionality,
but it would help get you up-to-speed on the code and we
do need additional tests.

Thanks
Jeff

Re: How hard would it be to implement sparse fetching/pulling?

[Jeff Hostetler]

On 11/30/2017 12:01 PM, Vitaly Arbuzov wrote:

Hey Jeff,

It's great, I didn't expect that anyone is actively working on this.
I'll check out your branch, meanwhile do you have any design docs that
describe these changes or can you define high level goals that you
want to achieve?



There are no summary docs in a traditional sense.
The patch series does have updated docs which show
the changes to some of the commands and protocols.
I would start there.

Jeff

jn/reproducible-build, was Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Johannes Schindelin]

Hi Junio & Jonathan (Nieder, there is another active Jonathan again),

On Wed, 29 Nov 2017, Junio C Hamano wrote:

> * jn/reproducible-build (2017-11-22) 3 commits
>   (merged to 'next' on 2017-11-27 at 6ae6946f8c)
>  + Merge branch 'jn/reproducible-build' of ../git-gui into 
> jn/reproducible-build
>  + git-gui: sort entries in optimized tclIndex
>  + generate-cmdlist: avoid non-deterministic output
> 
>  The build procedure has been taught to avoid some unnecessary
>  instability in the build products.

I like this, from a purely security-informed point of view. Maybe there
would be a way to integrate this with the Continuous Testing we do? Like,
letting Travis verify that the binaries built from a certain Debian
package are really identical to the binaries built from the corresponding
commit? But I guess Travis is the wrong vehicle for this, as Travis needs
a *commit* to be pushed, not a new package to be made available via apt...

Ciao,
Dscho

Re: How hard would it be to implement sparse fetching/pulling?

[Jeff Hostetler]

On 11/30/2017 8:51 PM, Vitaly Arbuzov wrote:

I think it would be great if we high level agree on desired user
experience, so let me put a few possible use cases here.

1. Init and fetch into a new repo with a sparse list.
Preconditions: origin blah exists and has a lot of folders inside of
src including "bar".
Actions:
git init foo && cd foo
git config core.sparseAll true # New flag to activate all sparse
operations by default so you don't need to pass options to each
command.
echo "src/bar" > .git/info/sparse-checkout
git remote add origin blah
git pull origin master
Expected results: foo contains src/bar folder and nothing else,
objects that are unrelated to this tree are not fetched.
Notes: This should work same when fetch/merge/checkout operations are
used in the right order.


With the current patches (parts 1,2,3) we can pass a blob-ish
to the server during a clone that refers to a sparse-checkout
specification.  There's a bit of a chicken-n-egg problem getting
things set up.  So if we assume your team would create a series
of "known enlistments" under version control, then you could
just reference one by : during your clone.  The
server can lookup that blob and just use it.

git clone --filter=sparse:oid=master:templates/bar URL

And then the server will filter-out the unwanted blobs during
the clone.  (The current version only filters blobs; you still
get full commits and trees.  That will be revisited later.)

On the client side, the partial clone installs local config
settings into the repo so that subsequent fetches default to
the same filter criteria as used in the clone.


I don't currently have provision to send a full sparse-checkout
specification to the server during a clone or fetch.  That
seemed like too much to try to squeeze into the protocols.
We can revisit this later if there is interest, but it wasn't
critical for the initial phase.




2. Add a file and push changes.
Preconditions: all steps above followed.
touch src/bar/baz.txt && git add -A && git commit -m "added a file"
git push origin master
Expected results: changes are pushed to remote.


I don't believe partial clone and/or partial fetch will cause
any changes for push.




3. Clone a repo with a sparse list as a filter.
Preconditions: same as for #1
Actions:
echo "src/bar" > /tmp/blah-sparse-checkout
git clone --sparse /tmp/blah-sparse-checkout blah # Clone should be
the only command that would requires specific option key being passed.
Expected results: same as for #1 plus /tmp/blah-sparse-checkout is
copied into .git/info/sparse-checkout


There are 2 independent concepts here: clone and checkout.
Currently, there isn't any automatic linkage of the partial clone to
the sparse-checkout settings, so you could do something like this:

git clone --no-checkout --filter=sparse:oid=master:templates/bar URL
git cat-file ... templates/bar >.git/info/sparse-checkout
git config core.sparsecheckout true
git checkout ...

I've been focused on the clone/fetch issues and have not looked
into the automation to couple them.




4. Showing log for sparsely cloned repo.
Preconditions: #3 is followed
Actions:
git log
Expected results: recent changes that affect src/bar tree.


If I understand your meaning, log would only show changes
within the sparse subset of the tree.  This is not on my
radar for partial clone/fetch.  It would be a nice feature
to have, but I think it would be better to think about it
from the point of view of sparse-checkout rather than clone.




5. Showing diff.
Preconditions: #3 is followed
Actions:
git diff HEAD^ HEAD
Expected results: changes from the most recent commit affecting
src/bar folder are shown.
Notes: this can be tricky operation as filtering must be done to
remove results from unrelated subtrees.


I don't have any plan for this and I don't think it fits within
the scope of clone/fetch.  I think this too would be a sparse-checkout
feature.




*Note that I intentionally didn't mention use cases that are related
to filtering by blob size as I think we should logically consider them
as a separate, although related, feature.


I've grouped blob-size and sparse filter together for the
purposes of clone/fetch since the basic mechanisms (filtering,
transport, and missing object handling) are the same for both.
They do lead to different end-uses, but that is above my level
here.




What do you think about these examples above? Is that something that
more-or-less fits into current development? Are there other important
flows that I've missed?


These are all good ideas and it is good to have someone else who
wants to use partial+sparse thinking about it and looking for gaps
as we try to make a complete end-to-end feature.


-Vitaly


Thanks
Jeff

Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Lars Schneider]

> On 29 Nov 2017, at 02:17, Junio C Hamano  wrote:
> 
> * jc/editor-waiting-message (2017-11-17) 1 commit
> - launch_editor(): indicate that Git waits for user input
> 
> Git shows a message to tell the user that it is waiting for the
> user to finish editing when spawning an editor, in case the editor
> opens to a hidden window or somewhere obscure and the user gets
> lost.
> 
> Will merge to 'next'.


Please take a look at v4 before you merge to 'next':
https://public-inbox.org/git/20171129143752.60553-1-lars.schnei...@autodesk.com/

Thanks,
Lars

jt/diff-anchored-patience, was Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Johannes Schindelin]

Hi Junio,

On Wed, 29 Nov 2017, Junio C Hamano wrote:

> * jt/diff-anchored-patience (2017-11-28) 1 commit
>  - diff: support anchoring line(s)
> 
>  "git diff" learned a variant of the "--patience" algorithm, to
>  which the user can specify which 'unique' line to be used as
>  anchoring points.

FWIW I am very much in favor of this feature. I often wished for something
easy to make sure that certain lines (which I know are unchanged) are not
marked as "-" and then "+". And Jonathan's design (--anchor=)
seems sound. If need be, I can always extend it to
(--anchor-regex=).

Ciao,
Dscho

cc/require-tcl-tk-for-build, was Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Johannes Schindelin]

Hi Junio,

On Wed, 29 Nov 2017, Junio C Hamano wrote:

> * cc/require-tcl-tk-for-build (2017-11-27) 1 commit
>  - Makefile: check that tcl/tk is installed
> 
>  A first-time builder of Git may have installed neither tclsh nor
>  msgfmt, in which case git-gui and gitk part will fail and break the
>  build.  As a workaround, refuse to run a build when tclsh is not
>  installed and NO_TCLTK is not set.
> 
>  Undecided.
>  I still feel that requring tclsh to be installed, with or without
>  "escape hatch" for experts, may be too heavy-handed.

FWIW I agree, from a maintainer's point of view (even if Git for Windows
always had a working Tcl/Tk, I remember the times when I did not have the
luxury of tclsh being there, and my quota was not enough to allow for
compiling *both* emacs and Tcl/Tk on that box).

Ciao,
Dscho

Re: What's cooking in git.git (Nov 2017, #08; Tue, 28)

[Johannes Schindelin]

Hi Junio,

On Wed, 29 Nov 2017, Junio C Hamano wrote:

> * tz/complete-branch-copy (2017-11-17) 1 commit
>   (merged to 'next' on 2017-11-20 at 6d22384fcd)
>  + completion: add '--copy' option to 'git branch'
> 
>  Command line completion (in contrib/) has been taught about the
>  "--copy" option of "git branch".
> 
> 
> * tz/notes-error-to-stderr (2017-11-15) 1 commit
>   (merged to 'next' on 2017-11-20 at a45d441ee2)
>  + notes: send "Automatic notes merge failed" messages to stderr
> 
>  "git notes" sent its error message to its standard output stream,
>  which was corrected.
> 
> 
> * tz/redirect-fix (2017-11-14) 2 commits
>   (merged to 'next' on 2017-11-20 at 5b13f7026e)
>  + rebase: fix stderr redirect in apply_autostash()
>  + t/lib-gpg: fix gpgconf stderr redirect to /dev/null
> 
>  A few scripts (both in production and tests) incorrectly redirected
>  their error output.  These have been corrected.

I guess I am the only one, but I have to say: I get confused *constantly*
when you mention Todd's patches in the "What's cooking" mails, always
expecting something timezone-related to be going on.

Ciao,
Dscho

P.S.: You mentioned that you'd go offline today, should I hold off with
more of Git for Windows' upstreaming? If so, how long?

Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

[Lars Schneider]

> On 30 Nov 2017, at 21:51, Jeff King  wrote:
> 
> On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schnei...@autodesk.com wrote:
> ...
>> The standard advise() function is not used here as it would always add
>> a newline which would make deleting the message harder.
> 
> I tried to think of ways this "show a message and then delete it" could
> go wrong. It should work OK with editors that just do curses-like
> things, taking over the terminal and then restoring it at the end.
> 
> It does behave in a funny way if the editor produces actual lines of
> output outside of the curses handling. E.g. (I just quit vim
> immediately, hence the aborting message):
> 
>  $ GIT_EDITOR='echo foo; vim' git commit
>  hint: Waiting for your editor input...foo
>  Aborting commit due to empty commit message.
> 
> our "foo" gets tacked onto the hint line, and then our deletion does
> nothing (because the newline after "foo" bumped us to a new line, and
> there was nothing on that line to erase).
> 
> An even worse case (and yes, this is really reaching) is:
> 
>  $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
>  hint: Waiting for your editor input...one
>  Aborting commit due to empty commit message.
> 
> There we ate the "two" line.
> 
> These are obviously the result of devils-advocate poking at the feature.
> I doubt any editor would end its output with a CR. But the first case is
> probably going to be common, especially for actual graphical editors. We
> know that emacsclient prints its own line, and I wouldn't be surprised
> if other graphical editors spew some telemetry to stderr (certainly
> anything built against GTK tends to do so).

True! However, if a Git user is not bothered by a graphical editor that
spews telemetry to stderr, then I think the same user wouldn't be
bothered by one additional line printed by Git either, right?


> The patch itself looks fine, as far as correctly implementing the
> design.

Thanks for the review :-)

- Lars

Re: git-p4: cloning with a change number does not import all files

[Lars Schneider]

> On 29 Nov 2017, at 04:48, Patrick Rouleau  wrote:
> 
> Hi,
> 
> On Mon, Nov 27, 2017 at 7:52 AM, Lars Schneider
>  wrote:
>> 
>> what is your goal here? Do you want to convert the repo to Git or do you
>> want to use Git to interact with a P4 repo?
> 
> I want to use git to interact with a P4 repo. I am used to git tools
> and I prefer them to
> P4V. Visual Studio is also slower with P4 plugin and I hate P4
> approach to check out
> a file before modifying it. The .sln files always get check outed for no 
> reason.
> 
> I am working remotely. The P4 server is behind a VPN. That one reason
> I am trying to
> clone from some point in the past. I also want to have some history to
> see from where
> come the code and from who.

Oh, I am with you. However, I only used git-p4 for a very short time in the 
way you want to use it. Therefore, I don't have much experience in that kind 
of usage pattern. I was able to convice my management to move all source to 
Git and I used git-p4 to migrate the repositories ;-)

Here is a talk on the topic if you want to learn more:
https://www.youtube.com/watch?v=QNixDNtwYJ0

Cheers,
Lars

Aborting a merge will delete new files added

[Pärham FH]

Hi!

While sorting out a merge conflict I accidentally added an untracked
folder to the index. Later I decided to abort the merge with `git
merge --abort` and I expected it to unstage the untracked file but it
instead removed it. This was not what I expected.

Cheers,
Pärham

Re: [PATCH] git-prompt: fix reading files with windows line endings

[Johannes Schindelin]

Hi Robert,

On Thu, 30 Nov 2017, Robert Abel wrote:

> On 30 Nov 2017 16:21, Johannes Schindelin wrote:
> > On Thu, 30 Nov 2017, Robert Abel wrote:
> >> So reading a dummy variable along with the actual content variable
> >> works for git-prompt:
> >>
> >> __git_eread ()
> >> {
> >> local f="$1"
> >> local dummy
> >> shift
> >> test -r "$f" && IFS=$'\r\n' read "$@" dummy < "$f"
> >> }
> >>
> >> I feel like this would be the most readable solution thus far.
> > 
> > Hmm. I am just a little concerned about "dummy" swallowing the rest of the
> > line, e.g. when reading "1 2 3" via `__git_eread line`... the way I read
> > it, dummy would consume "2 3" and line would *not* receive "1 2 3" but
> > only "1"...
> You missed that tab and space aren't field separator anymore,
> because IFS=$'\r\n'. The way I see it, __git_eread was never meant to
> split tokens. Its primary purpose was to test if a file exists and if
> so, read all its contents sans the newline into a variable.

Ah. The "$@* put me on the wrong track. If you hard-code the expectation
that __git_eread is not used to split tokens, maybe there should be a
preparatory patch (after carefully ensuring that all callers pass only one
argument) to change the "$@" to "$1"?

That will prevent future callers from expecting the token-splitting
behavior that is promised by using "$@".

Ciao,
Johannes