Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input
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,
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 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)
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
__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
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
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 = &refs_be_sp; + } else { + be = &refs_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, &refs_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 != &refs_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"); + + if
[WIP 0/2] Submodule ref backend that mirrors superproject
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
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, &sb); prepare_submodule_repo_env(&cp.env_array); @@ -2017,45 +2029,49 @@ const char *get_superproject_working_tree(void) if (start_command(&cp)) die(_("could not start ls-files in ..")); - len = strbuf_read(&sb, cp.out, PATH_MAX); + strbuf_read(&sb, 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(&cp); + 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[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(&sb); - - code = finish_command(&cp); 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(&full_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, &len)) + 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()
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(&oid, 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
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
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
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
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()
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
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
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]
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
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&UNIX 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
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?
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()
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
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
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?
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()
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(&oid, 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
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)
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?
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 the completeness requirement and narrow reciept option
[RFE] Inverted sparseness
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&UNIX 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
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
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)
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?
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
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
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)
> 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?
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?
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)
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?
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)
> 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)
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)
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)
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
> 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
> 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
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
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