Documentation: update "man git-add" to include "[]"
Current "man git-add" emphasizes single letter interactive shortcut commands with "[]". Signed-off-by: Robert P. J. Day --- diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 45652fe4a6..ad9bd7c7a6 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -239,8 +239,8 @@ and type return, like this: *** Commands *** - 1: status 2: update 3: revert 4: add untracked - 5: patch6: diff 7: quit 8: help + 1: [s]tatus 2: [u]pdate 3: [r]evert 4: [a]dd untracked + 5: [p]atch 6: [d]iff 7: [q]uit 8: [h]elp What now> 1 -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca/dokuwiki Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH] mailmap: update brandon williams's email address
On Sat, Dec 08 2018, Junio C Hamano wrote: > Stefan Beller writes: > >> On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder wrote: >>> >>> Brandon Williams wrote: >>> >>> > Signed-off-by: Brandon Williams >>> > --- >>> > .mailmap | 1 + >>> > 1 file changed, 1 insertion(+) >>> >>> I can confirm that this is indeed the same person. >> >> What would be more of interest is why we'd be interested in this patch >> as there is no commit/patch sent by Brandon with this email in gits history. > > Once I "git am" the message that began this thread, there will be a > commit under this new ident, so that would be somewhat a moot point. "Get to the top of 'git shortlog -sn' with this one easy trick" :) (The patch makes sense, good to see you back on-list Brandon)
Re: [PATCH] mailmap: update brandon williams's email address
On Sat, Dec 8, 2018 at 7:09 AM Junio C Hamano wrote: > If this were "Jonathan asked Brandon if we want to record an address > we can reach him in our .mailmap file and sent a patch to add one", Not sure about Jonathan, but I did. > then the story is different, and I tend to agree with you that such > a patch is more or less pointless. That's not the purpose of the > mailmap file. Not directly, but when multiple commands use mailmap to show the canonical mail addresses, then it kinda is. When I look back at an old commit message, I may look up the author name and address, which is mapped. > Not until git-send-email learns to use that file to rewrite > To/cc/etc to the "canonical" addresses, anyway ;-) git-send-email does not have to when I copy/paste the address from git-log anyway. > I am not sure if there are people whose "canonical" address to be > used as the author is not necessarily the best address they want to > get their e-mails at, though. If we can be reasonably sure that the > set of such people is empty, then people can take the above mention > about send-email as a hint about a low-hanging fruit ;-) > > Thanks. > > -- Duy
Re: [PATCH] mailmap: update brandon williams's email address
On Fri, Dec 7, 2018 at 10:08 PM Junio C Hamano wrote: > > Stefan Beller writes: > > > On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder wrote: > >> > >> Brandon Williams wrote: > >> > >> > Signed-off-by: Brandon Williams > >> > --- > >> > .mailmap | 1 + > >> > 1 file changed, 1 insertion(+) > >> > >> I can confirm that this is indeed the same person. > > > > What would be more of interest is why we'd be interested in this patch > > as there is no commit/patch sent by Brandon with this email in gits history. > > Once I "git am" the message that began this thread, there will be a > commit under this new ident, so that would be somewhat a moot point. > > If this were "Jonathan asked Brandon if we want to record an address > we can reach him in our .mailmap file and sent a patch to add one", > then the story is different, and I tend to agree with you that such > a patch is more or less pointless. That's not the purpose of the > mailmap file. > Turns out this is exactly the reason :) I've had a couple of people reach out to me asking me to do this because CCing my old email bounces and they've wanted my input/comments on something related to work I've done. If that's not the intended purpose then please ignore this patch > Not until git-send-email learns to use that file to rewrite > To/cc/etc to the "canonical" addresses, anyway ;-) > > I am not sure if there are people whose "canonical" address to be > used as the author is not necessarily the best address they want to > get their e-mails at, though. If we can be reasonably sure that the > set of such people is empty, then people can take the above mention > about send-email as a hint about a low-hanging fruit ;-) > > Thanks. > >
Re: [PATCH] mailmap: update brandon williams's email address
Stefan Beller writes: > On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder wrote: >> >> Brandon Williams wrote: >> >> > Signed-off-by: Brandon Williams >> > --- >> > .mailmap | 1 + >> > 1 file changed, 1 insertion(+) >> >> I can confirm that this is indeed the same person. > > What would be more of interest is why we'd be interested in this patch > as there is no commit/patch sent by Brandon with this email in gits history. Once I "git am" the message that began this thread, there will be a commit under this new ident, so that would be somewhat a moot point. If this were "Jonathan asked Brandon if we want to record an address we can reach him in our .mailmap file and sent a patch to add one", then the story is different, and I tend to agree with you that such a patch is more or less pointless. That's not the purpose of the mailmap file. Not until git-send-email learns to use that file to rewrite To/cc/etc to the "canonical" addresses, anyway ;-) I am not sure if there are people whose "canonical" address to be used as the author is not necessarily the best address they want to get their e-mails at, though. If we can be reasonably sure that the set of such people is empty, then people can take the above mention about send-email as a hint about a low-hanging fruit ;-) Thanks.
Re: [wishlist] git submodule update --reset-hard
On Fri, 07 Dec 2018, Yaroslav Halchenko wrote: > On Fri, 07 Dec 2018, Stefan Beller wrote: > > > the initial "git submodule update --reset-hard" is pretty much a > > > crude workaround for some of those cases, so I would just go earlier in > > > the history, and redo some things, whenever I could just drop or revert > > > some selected set of commits. > > That makes sense. > > Do you want to give the implementation a try for the --reset-hard switch? > ok, will do, thanks for the blessing ;-) The patch is attached (please advise if should be done differently) and also submitted as PR https://github.com/git/git/pull/563 I guess it would need more tests. Took me some time to figure out why I was getting fatal: bad value for update parameter after all my changes to the git-submodule.sh script after looking at an example commit 42b491786260eb17d97ea9fb1c4b70075bca9523 which introduced --merge to the update ;-) -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik From 170296dc661b4bc3d942917ce27288df52ff650d Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 7 Dec 2018 21:28:29 -0500 Subject: [PATCH] submodule: Add --reset-hard option for git submodule update This patch adds a --reset-hard option for the update command to hard reset submodule(s) to the gitlink for the corresponding submodule in the superproject. This feature is desired e.g. to be able to discard recent changes in the entire hierarchy of the submodules after running git reset --hard PREVIOUS_STATE in the superproject which leaves submodules in their original state, and git reset --hard --recurse-submodules PREVIOUS_STATE would result in the submodules being checked into detached HEADs. As in the original git reset --hard no checks or any kind of safe-guards against jumping into some state which was never on the current branch is done. must_die_on_failure is not set to yes to mimic behavior of a update --checkout strategy, which would leave user with a non-clean state immediately apparent via git status so an informed decision/actions could be done manually. Signed-off-by: Yaroslav Halchenko --- Documentation/git-submodule.txt | 12 +++- Documentation/gitmodules.txt| 4 ++-- builtin/submodule--helper.c | 3 ++- git-submodule.sh| 10 +- submodule.c | 4 submodule.h | 1 + t/t7406-submodule-update.sh | 17 - 7 files changed, 45 insertions(+), 6 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index ba3c4df550..f90a42d265 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -124,7 +124,7 @@ If you really want to remove a submodule from the repository and commit that use linkgit:git-rm[1] instead. See linkgit:gitsubmodules[7] for removal options. -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]:: +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge|--reset-hard] [--reference ] [--depth ] [--recursive] [--jobs ] [--] [...]:: + -- Update the registered submodules to match what the superproject @@ -358,6 +358,16 @@ the submodule itself. If the key `submodule.$name.update` is set to `rebase`, this option is implicit. +--reset-hard:: + This option is only valid for the update command. + Hard reset current state to the commit recorded in the superproject. +If this option is given, the submodule's HEAD will not get detached +if it was not detached before. Note that, like with a regular +git reset --hard no safe-guards are in place to prevent jumping +to a commit which was never part of the current branch. + If the key `submodule.$name.update` is set to `reset-hard`, this + option is implicit. + --init:: This option is only valid for the update command. Initialize all submodules for which "git submodule init" has not been diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 312b6f9259..e085dbc01f 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -43,8 +43,8 @@ submodule..update:: command in the superproject. This is only used by `git submodule init` to initialize the configuration variable of the same name. Allowed values here are 'checkout', 'rebase', - 'merge' or 'none'. See description of 'update' command in - linkgit:git-submodule[1] for their meaning. Note that the + 'merge', 'reset-hard' or 'none'. See description of 'update' command + in l
Re: [wishlist] git submodule update --reset-hard
On Fri, 07 Dec 2018, Stefan Beller wrote: > > the initial "git submodule update --reset-hard" is pretty much a > > crude workaround for some of those cases, so I would just go earlier in > > the history, and redo some things, whenever I could just drop or revert > > some selected set of commits. > That makes sense. > Do you want to give the implementation a try for the --reset-hard switch? ok, will do, thanks for the blessing ;-) -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik
[PATCH 1/4] submodule update: add regression test with old style setups
As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07) was produced shortly before a release, nobody asked for a regression test to be included. Add a regression test that makes sure that the invocation of `git submodule update` on old setups doesn't produce errors as pointed out in f178c13fda. The place to add such a regression test may look odd in t7412, but that is the best place as there we setup old style submodule setups explicitly. Signed-off-by: Stefan Beller --- t/t7412-submodule-absorbgitdirs.sh | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh index ce74c12da2..1cfa150768 100755 --- a/t/t7412-submodule-absorbgitdirs.sh +++ b/t/t7412-submodule-absorbgitdirs.sh @@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' ' GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \ core.worktree "../../../nested" && # make sure this re-setup is correct - git status --ignore-submodules=none + git status --ignore-submodules=none && + + # also make sure this old setup does not regress + git submodule update --init --recursive >out 2>err && + test_must_be_empty out && + test_must_be_empty err ' test_expect_success 'absorb the git dir in a nested submodule' ' -- 2.20.0.rc2.403.gdbc3b29805-goog
Re: [PATCH] mailmap: update brandon williams's email address
Hi, Stefan Beller wrote: > What would be more of interest is why we'd be interested in this patch > as there is no commit/patch sent by Brandon with this email in gits history. I think there's an implicit assumption in this question that isn't spelled out. Do I understand correctly that you're saying the main purpose of .mailmap is to figure out whether two commits are by the same author? My own uses of .mailmap primarily have a different purpose: to find out the preferred contact address for the author of a given commit. Thanks, Jonathan
Re: [PATCH] mailmap: update brandon williams's email address
On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder wrote: > > Brandon Williams wrote: > > > Signed-off-by: Brandon Williams > > --- > > .mailmap | 1 + > > 1 file changed, 1 insertion(+) > > I can confirm that this is indeed the same person. What would be more of interest is why we'd be interested in this patch as there is no commit/patch sent by Brandon with this email in gits history. Is that so you get cc'd on your private address and can follow things you worked on without being subscribed to the mailing list? (I'd be interested to see the use case in the commit message;) Thanks, Stefan
Re: [wishlist] git submodule update --reset-hard
On Thu, Dec 6, 2018 at 5:23 PM Yaroslav Halchenko wrote: > > There was a proposal to "re-attach HEAD" in the submodule, i.e. > > if the branch branch points at the same commit, we don't need > > a detached HEAD, but could go with the branch instead. > > if I got the idea right, if we are talking about any branch, it > would also non-deterministic since who knows what left over branch(es) > point to that commit. Not sure if I would have used that ;) I would think we'd rather want to have it deterministic, i.e. something like 1) record branch name of the submodule 2) update submodules HEAD to to superprojects gitlink 3) if recorded branch (1) matches the sha1 of detached HEAD, have HEAD point to the branch instead. You notice a small inefficiency here as we write HEAD twice, so it could be reworded as: 1) compare superprojects gitlink with the submodules branch 2a) if equal, set submodules HEAD to branch 2b) if unequal set HEAD to gitlink value, resulting in detached HEAD Note that this idea of reattaching reflects the idea (a) below. > > a) "stay on submodule branch (i.e. HEAD still points at $branch), and > > reset --hard" such that the submodule has a clean index and at that $branch > > or > > b) "stay on submodule branch (i.e. HEAD still points at $branch), but > > $branch is > >set to the gitlink from the superproject, and then a reset --hard > >will have the worktree set to it as well. > NB "gitlink" -- just now discovered the thing for me. Thought it would be > called a subproject echoing what git diff/log -p shows for submodule > commits. The terminology is messy: The internal representation in Gits object model is a "gitlink" entry in a tree object. Once we have a .gitmodules entry, we call it submodule. The term 'subproject' is a historic artifact and will likely not be changed in the diff output (or format-patch), because these diffs can be applied using git-am for example. That makes the diff output effectively a transport protocol, and changing protocols is hard if you have no versioning in them. More in https://git-scm.com/docs/gitsubmodules (a rather recent new write of a man page, going into concepts). > > > right -- I meant the local changes and indeed reset --recurse-submodules > > > indeed seems to recurse nicely. Then the undesired effect remaining only > > > the detached HEAD > > > For that we may want to revive discussions in > > https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/ > > well, isn't that one requires a branch to be specified in .gitmodules? Ah good point. > > git reset --hard --recursive=hard,keep-branch PREVIOUSPOINT > > 'keep-branch' (given aforementioned keeping the specified in .gitmodules > branch) might be confusing. Also what if a submodule already in a > detached HEAD? IMHO --recursive=hard, and just saying that it would do > "reset --hard", is imho sufficient. (that is why I like pure > --reset hard since it doesn't care and neither does anything to the > branch) For that we might want to first do the git submodule update --reset-hard which runs reset --hard inside the submodule, no matter which branch the submodule is on (if any) and resets to the given superproject sha1. See git-submodule.sh in git.git[1] in cmd_update. We'd need to add a command line flag (`--reset-hard` would be the obvious choice?) which would set the `update` variable, which then is evaluated to what needs to be done in the submodule, which in that case would be the hard reset. https://github.com/git/git/blob/master/git-submodule.sh#L606 Once that is done we'd want to add a test case, presumably in t/t7406-submodule-update.sh > > > I would have asked for > > > >git revert --recursive ... > > >git rebase --recursive [-i] ... > > > > which I also frequently desire (could elaborate on the use cases etc). > > > These would be nice to have. It would be nice if you'd elaborate on the > > use cases for future reference in the mailing list archive. :-) > > ok, will try to do so ;-) In summary: they are just a logical extension > of git support for submodules for anyone actively working with > submodules to keep entire tree in sync. Then quite often the need for > reverting a specific commit (which also has changes reflected in > submodules) arises. The same with rebase, especially to trim away some > no longer desired changes reflected in submodules. > > the initial "git submodule update --reset-hard" is pretty much a > crude workaround for some of those cases, so I would just go earlier in > the history, and redo some things, whenever I could just drop or revert > some selected set of commits. That makes sens
Re: [PATCH] mailmap: update brandon williams's email address
Brandon Williams wrote: > Signed-off-by: Brandon Williams > --- > .mailmap | 1 + > 1 file changed, 1 insertion(+) I can confirm that this is indeed the same person. Reviewed-by: Jonathan Nieder Welcome back!
[PATCH] mailmap: update brandon williams's email address
Signed-off-by: Brandon Williams --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index eb7b5fc7b..247a3deb7 100644 --- a/.mailmap +++ b/.mailmap @@ -27,6 +27,7 @@ Ben Walton Benoit Sigoure Bernt Hansen Brandon Casey +Brandon Williams brian m. carlson brian m. carlson Bryan Larsen -- 2.19.1
Re: [PATCH] git-rebase.txt: update note about directory rename detection and am
On Fri, Dec 7, 2018 at 9:51 AM Johannes Sixt wrote: > > From: Elijah Newren > > In commit 6aba117d5cf7 ("am: avoid directory rename detection when > calling recursive merge machinery", 2018-08-29), the git-rebase manpage > probably should have also been updated to note the stronger > incompatibility between git-am and directory rename detection. Update > it now. > > Signed-off-by: Elijah Newren > Signed-off-by: Johannes Sixt > --- > Documentation/git-rebase.txt | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 41631df6e4..7bea21e8e3 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -569,8 +569,9 @@ it to keep commits that started empty. > Directory rename detection > ~~ > > -The merge and interactive backends work fine with > -directory rename detection. The am backend sometimes does not. > +Directory rename heuristics are enabled in the merge and interactive > +backends. Due to the lack of accurate tree information, directory > +rename detection is disabled in the am backend. > > include::merge-strategies.txt[] I was intending to send this out the past couple days, was just kinda busy. Thanks for handling it for me.
[PATCH] git-rebase.txt: update note about directory rename detection and am
From: Elijah Newren In commit 6aba117d5cf7 ("am: avoid directory rename detection when calling recursive merge machinery", 2018-08-29), the git-rebase manpage probably should have also been updated to note the stronger incompatibility between git-am and directory rename detection. Update it now. Signed-off-by: Elijah Newren Signed-off-by: Johannes Sixt --- Documentation/git-rebase.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 41631df6e4..7bea21e8e3 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -569,8 +569,9 @@ it to keep commits that started empty. Directory rename detection ~~ -The merge and interactive backends work fine with -directory rename detection. The am backend sometimes does not. +Directory rename heuristics are enabled in the merge and interactive +backends. Due to the lack of accurate tree information, directory +rename detection is disabled in the am backend. include::merge-strategies.txt[] -- 2.19.1.1133.g2dd3d172d2
Re: [wishlist] git submodule update --reset-hard
Hi Stefan, Thanks for the dialogue! Replies are embedded below. On Thu, 06 Dec 2018, Stefan Beller wrote: > ... > > > What if the branch differs from the sha1 recorded in the superproject? > > git reset --hard itself is an operation which should be done with some > > level of competence in doing "the right thing" by calling it. You > > can hop branches even in current (without any submodules in question) > > repository with it and cause as much chaos as you desire. > Right. > git reset --hard would the branch (as well as the working tree) to the > given sha1, which is confusing as submodules get involved. > The Right Thing as of now is the sha1 as found in the > superprojects gitlink. But as that can be different from any branch > in the submodule, we'd rather detach the HEAD to make it > deterministic. yeap, makes total sense to be the thing do that by default ;-) > There was a proposal to "re-attach HEAD" in the submodule, i.e. > if the branch branch points at the same commit, we don't need > a detached HEAD, but could go with the branch instead. if I got the idea right, if we are talking about any branch, it would also non-deterministic since who knows what left over branch(es) point to that commit. Not sure if I would have used that ;) > > If desired though, a number of prevention mechanisms could be in place (but > > would require option(s) to overcome) to allow submodule to be reset > > --hard'ed > > only when some conditions met (e.g. only to the commit which is among parent > > commits path of the current branch). This way wild hops would be prevented, > > although you might still end up in some feature branch. But since "reset > > --hard" itself doesn't have any safe-guards, I do not really think they > > should > > be implemented here either. > So are you looking for > a) "stay on submodule branch (i.e. HEAD still points at $branch), and > reset --hard" such that the submodule has a clean index and at that $branch > or > b) "stay on submodule branch (i.e. HEAD still points at $branch), but $branch > is >set to the gitlink from the superproject, and then a reset --hard >will have the worktree set to it as well. yes! NB "gitlink" -- just now discovered the thing for me. Thought it would be called a subproject echoing what git diff/log -p shows for submodule commits. > (a) is what the referenced submodule.repoLike option implements. sounds like it indeed, thanks for spelling out > I'd understand the desire for (b) as well, as it is a "real" hard reset on > the superproject level, without detaching branches. yeap > > > git reset --hard --recurse-submodules HEAD > > it does indeed some trick(s) but not all seems to be the ones I desire: > > 1. Seems to migrate submodule's .git directories into the top level > > .git/modules > Ah yes, that happens too. This will help once you want to git-rm > a submodule and checkout states before and after. yeap ;-) > > > undesirable in the sense of still having local changes (that is what > > > the above reset with `--recurse` would fix) or changed the branch > > > state? (i.e. is detached but was on a branch before?) > > right -- I meant the local changes and indeed reset --recurse-submodules > > indeed seems to recurse nicely. Then the undesired effect remaining only > > the detached HEAD > For that we may want to revive discussions in > https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/ well, isn't that one requires a branch to be specified in .gitmodules? > > > > git submodule update --recursive > > > > I would end up in the detached HEADs within submodules. > > > > What I want is to retain current branch they are at (or may be possible > > > > "were in"? reflog records might have that information) > > > So something like > > > git submodule foreach --recursive git reset --hard > > > ? > > not quite -- this would just kill all local changes within each submodule, > > not > > to reset it to the desired state, which wouldn't be specified in such > > invocation, and is only known to the repo containing it > With this answer it sounds like you'd want (b) from above. yeap > > > You may be interested in > > > https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/ > > > which introduces a switch `submodule.repoLike [ = true]`, which > > > when set would not detach HEAD in submodules. > > Thanks! looks interesting -- was there more discussion/activity beyond > > those 5 > > posts in the thread? >
Re: [wishlist] git submodule update --reset-hard
On Thu, Dec 6, 2018 at 1:25 PM Yaroslav Halchenko wrote: > > > On Thu, 06 Dec 2018, Stefan Beller wrote: > > > On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko > > wrote: > > > > Dear Git Gurus, > > > > I wondered what would be your take on my wishlist request to add > > > --reset-hard option, which would be very similar to regular "update" which > > > checks out necessary commit, but I want it to remain in the branch. > > > What if the branch differs from the sha1 recorded in the superproject? > > git reset --hard itself is an operation which should be done with some > level of competence in doing "the right thing" by calling it. You > can hop branches even in current (without any submodules in question) > repository with it and cause as much chaos as you desire. Right. git reset --hard would the branch (as well as the working tree) to the given sha1, which is confusing as submodules get involved. The Right Thing as of now is the sha1 as found in the superprojects gitlink. But as that can be different from any branch in the submodule, we'd rather detach the HEAD to make it deterministic. There was a proposal to "re-attach HEAD" in the submodule, i.e. if the branch branch points at the same commit, we don't need a detached HEAD, but could go with the branch instead. > If desired though, a number of prevention mechanisms could be in place (but > would require option(s) to overcome) to allow submodule to be reset --hard'ed > only when some conditions met (e.g. only to the commit which is among parent > commits path of the current branch). This way wild hops would be prevented, > although you might still end up in some feature branch. But since "reset > --hard" itself doesn't have any safe-guards, I do not really think they should > be implemented here either. So are you looking for a) "stay on submodule branch (i.e. HEAD still points at $branch), and reset --hard" such that the submodule has a clean index and at that $branch or b) "stay on submodule branch (i.e. HEAD still points at $branch), but $branch is set to the gitlink from the superproject, and then a reset --hard will have the worktree set to it as well. (a) is what the referenced submodule.repoLike option implements. I'd understand the desire for (b) as well, as it is a "real" hard reset on the superproject level, without detaching branches. > > git reset --hard --recurse-submodules HEAD > it does indeed some trick(s) but not all seems to be the ones I desire: > > 1. Seems to migrate submodule's .git directories into the top level > .git/modules Ah yes, that happens too. This will help once you want to git-rm a submodule and checkout states before and after. > > undesirable in the sense of still having local changes (that is what > > the above reset with `--recurse` would fix) or changed the branch > > state? (i.e. is detached but was on a branch before?) > > right -- I meant the local changes and indeed reset --recurse-submodules > indeed seems to recurse nicely. Then the undesired effect remaining only > the detached HEAD For that we may want to revive discussions in https://public-inbox.org/git/20170501180058.8063-5-sbel...@google.com/ > > > git submodule update --recursive > > > > I would end up in the detached HEADs within submodules. > > > > What I want is to retain current branch they are at (or may be possible > > > "were in"? reflog records might have that information) > > > So something like > > > git submodule foreach --recursive git reset --hard > > > ? > > not quite -- this would just kill all local changes within each submodule, > not > to reset it to the desired state, which wouldn't be specified in such > invocation, and is only known to the repo containing it With this answer it sounds like you'd want (b) from above. > > You may be interested in > > https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/ > > which introduces a switch `submodule.repoLike [ = true]`, which > > when set would not detach HEAD in submodules. > > Thanks! looks interesting -- was there more discussion/activity beyond those 5 > posts in the thread? Unfortunately there was not. > This feature might indeed come handy but if I got it right, it is somewhat > complimentary to just having submodule update --reset-hard . E.g. submodules > might be in different branches (if I am not tracking based on branch names), > so > I would not want a recursive checkout with -b|-B. But we would indeed benefit > from such functionality, since this difficulty of managing branches of > submodules I think would be elevated with it! (e.g. in one
Re: [wishlist] git submodule update --reset-hard
On Thu, 06 Dec 2018, Stefan Beller wrote: > On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko > wrote: > > Dear Git Gurus, > > I wondered what would be your take on my wishlist request to add > > --reset-hard option, which would be very similar to regular "update" which > > checks out necessary commit, but I want it to remain in the branch. > What if the branch differs from the sha1 recorded in the superproject? git reset --hard itself is an operation which should be done with some level of competence in doing "the right thing" by calling it. You can hop branches even in current (without any submodules in question) repository with it and cause as much chaos as you desire. If desired though, a number of prevention mechanisms could be in place (but would require option(s) to overcome) to allow submodule to be reset --hard'ed only when some conditions met (e.g. only to the commit which is among parent commits path of the current branch). This way wild hops would be prevented, although you might still end up in some feature branch. But since "reset --hard" itself doesn't have any safe-guards, I do not really think they should be implemented here either. > > Rationale: In DataLad we heavily rely on submodules, and we have established > > easy ways to do some manipulations across full hierarchies of them. E.g. a > > single command could introduce a good number of commits across deep > > hierarchy > > of submodules, e.g. while committing changes within deep submodule, while > > also > > doing all necessary commits in the repositories leading to that submodule so > > the entire tree of them stays in a "clean" state. The difficulty comes when > > there is a need to just "forget" some changes. The obvious way is to e.g. > >git reset --hard PREVIOUS_STATE > git reset --hard --recurse-submodules HEAD > would do the trick it does indeed some trick(s) but not all seems to be the ones I desire: 1. Seems to migrate submodule's .git directories into the top level .git/modules $> git reset --hard --recurse-submodules HEAD^^^ Migrating git directory of 'famface' from '/tmp/gobbini/famface/.git' to '/tmp/gobbini/.git/modules/famface' Migrating git directory of 'famface/data' from '/tmp/gobbini/famface/data/.git' to '/tmp/gobbini/.git/modules/famface/modules/data' Migrating git directory of 'famface/data/scripts/mridefacer' from '/tmp/gobbini/famface/data/scripts/mridefacer/.git' to '/tmp/gobbini/.git/modules/famface/modules/data/modules/scripts/mridefacer' HEAD is now at 9b4296d [DATALAD] aggregated meta data we might eventually adopt this default already for years model (git annex seems to be ok, in that it then replaces .git symlink file with the actual symlink .git -> ../../.git/modules/... So things seems to keep working for annex) 2. It still does the detached HEAD for me $> git submodule status --recursive 2569ab436501a832d35afbbe9cc20ffeb6077eb1 famface (2569ab4) f1e8c9b8b025c311424283b9711efc6bc906ba2b famface/data (BIDS-v1.0.1) 49b0fe42696724c2a8492f999736056e51b77358 famface/data/scripts/mridefacer (49b0fe4) > > in the top level repository. But that leaves all the submodules now in > > the undesired state. If I do > undesirable in the sense of still having local changes (that is what > the above reset with `--recurse` would fix) or changed the branch > state? (i.e. is detached but was on a branch before?) right -- I meant the local changes and indeed reset --recurse-submodules indeed seems to recurse nicely. Then the undesired effect remaining only the detached HEAD > > git submodule update --recursive > > I would end up in the detached HEADs within submodules. > > What I want is to retain current branch they are at (or may be possible > > "were in"? reflog records might have that information) > So something like > git submodule foreach --recursive git reset --hard > ? not quite -- this would just kill all local changes within each submodule, not to reset it to the desired state, which wouldn't be specified in such invocation, and is only known to the repo containing it > You may be interested in > https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/ > which introduces a switch `submodule.repoLike [ = true]`, which > when set would not detach HEAD in submodules. Thanks! looks interesting -- was there more discussion/activity beyond those 5 posts in the thread? https://public-inbox.org/git/87h8i9ift4@evledraar.gmail.com/#r This feature might indeed come handy but if I got it right, it is somewhat complimentary to just having submodule update --reset-hard .
Re: [wishlist] git submodule update --reset-hard
On Thu, Dec 6, 2018 at 10:02 AM Yaroslav Halchenko wrote: > > Dear Git Gurus, > > I wondered what would be your take on my wishlist request to add > --reset-hard option, which would be very similar to regular "update" which > checks out necessary commit, but I want it to remain in the branch. What if the branch differs from the sha1 recorded in the superproject? > Rationale: In DataLad we heavily rely on submodules, and we have established > easy ways to do some manipulations across full hierarchies of them. E.g. a > single command could introduce a good number of commits across deep hierarchy > of submodules, e.g. while committing changes within deep submodule, while also > doing all necessary commits in the repositories leading to that submodule so > the entire tree of them stays in a "clean" state. The difficulty comes when > there is a need to just "forget" some changes. The obvious way is to e.g. > >git reset --hard PREVIOUS_STATE git reset --hard --recurse-submodules HEAD would do the trick > in the top level repository. But that leaves all the submodules now in > the undesired state. If I do undesirable in the sense of still having local changes (that is what the above reset with `--recurse` would fix) or changed the branch state? (i.e. is detached but was on a branch before?) > git submodule update --recursive > > I would end up in the detached HEADs within submodules. > > What I want is to retain current branch they are at (or may be possible > "were in"? reflog records might have that information) So something like git submodule foreach --recursive git reset --hard ? You may be interested in https://public-inbox.org/git/20180927221603.148025-1-sbel...@google.com/ which introduces a switch `submodule.repoLike [ = true]`, which when set would not detach HEAD in submodules. Can you say more about the first question above: Would you typically have situations where the submodule branch is out of sync with the superproject and how do you deal with that? Adding another mode to `git submodule update` sounds reasonable to me, too. Stefan
[wishlist] git submodule update --reset-hard
Dear Git Gurus, I wondered what would be your take on my wishlist request to add --reset-hard option, which would be very similar to regular "update" which checks out necessary commit, but I want it to remain in the branch. Rationale: In DataLad we heavily rely on submodules, and we have established easy ways to do some manipulations across full hierarchies of them. E.g. a single command could introduce a good number of commits across deep hierarchy of submodules, e.g. while committing changes within deep submodule, while also doing all necessary commits in the repositories leading to that submodule so the entire tree of them stays in a "clean" state. The difficulty comes when there is a need to just "forget" some changes. The obvious way is to e.g. git reset --hard PREVIOUS_STATE in the top level repository. But that leaves all the submodules now in the undesired state. If I do git submodule update --recursive I would end up in the detached HEADs within submodules. What I want is to retain current branch they are at (or may be possible "were in"? reflog records might have that information) Example: # Have to use datalad install since git clone --recurse-submodules # seems to not consider alternative locations for submodules' .git/ # with url being just a relative path, and where submodules aren't # all residing up under toplevel URL .git/ $> datalad install -r http://datasets.datalad.org/labs/gobbini/.git [INFO ] Cloning http://datasets.datalad.org/labs/gobbini/.git into '/tmp/gobbini' install(ok): /tmp/gobbini (dataset) [INFO ] Installing recursively [INFO ] Cloning http://datasets.datalad.org/labs/gobbini/famface/.git into '/tmp/gobbini/famface' [INFO ] Cloning http://datasets.datalad.org/labs/gobbini/famface/data/.git into '/tmp/gobbini/famface/data' [INFO ] access to dataset sibling "datasets.datalad.org" not auto-enabled, enable with: | datalad siblings -d "/tmp/gobbini/famface/data" enable -s datasets.datalad.org [INFO ] Cloning http://datasets.datalad.org/labs/gobbini/famface/data/scripts/mridefacer/.git [2 other candidates] into '/tmp/gobbini/famface/data/scripts/mridefacer' action summary: install (ok: 4) so I have a hierarchy in a good state and all checked out in master branch $> cd gobbini $> git submodule status --recursive b9071a6bc9f7665f7c75549c63d29f16d40e8af7 famface (heads/master) e59ba76b42f219bdf14b6b547dd6d9cc0ed5227f famface/data (BIDS-v1.0.1-3-ge59ba76b) 5d8036c0aaeebb448a00df6296ddc9f799efdd1f famface/data/scripts/mridefacer (heads/master) $> git submodule foreach --recursive cat .git/HEAD Entering 'famface' ref: refs/heads/master Entering 'famface/data' ref: refs/heads/master Entering 'famface/data/scripts/mridefacer' ref: refs/heads/master and if I do roll back $> git reset --hard HEAD^^^ HEAD is now at 9b4296d [DATALAD] aggregated meta data changes on filesystem: famface | 2 +- and default update --recursive $> git submodule update --recursive Submodule path 'famface': checked out '2569ab436501a832d35afbbe9cc20ffeb6077eb1' Submodule path 'famface/data': checked out 'f1e8c9b8b025c311424283b9711efc6bc906ba2b' Submodule path 'famface/data/scripts/mridefacer': checked out '49b0fe42696724c2a8492f999736056e51b77358' I end up in detached HEADs $> git submodule status --recursive 2569ab436501a832d35afbbe9cc20ffeb6077eb1 famface (2569ab4) f1e8c9b8b025c311424283b9711efc6bc906ba2b famface/data (BIDS-v1.0.1) 49b0fe42696724c2a8492f999736056e51b77358 famface/data/scripts/mridefacer (49b0fe4) I do see that there is a "custom command" way to do it via "submodule..update" config setting, but that is not easy to use for my case since all the `` would be different to specify !git reset --hard for all of them via config option and I could not find any way to "glob" config (like submodule.*.update). But in effect that is probably what I need: # restarting from a clean state here $> git -c submodule.famface.update='!git reset --hard' submodule update --recursive HEAD is now at 2569ab4 [DATALAD] aggregated meta data Submodule path 'famface': 'git reset --hard 2569ab436501a832d35afbbe9cc20ffeb6077eb1' Submodule path 'famface/data': checked out
Re: [PATCH v2] l10n: update German translation
Hi, thanks for your great work! Just two remarks: #: midx.c:407 -#, fuzzy, c-format +#, c-format msgid "failed to add packfile '%s'" -msgstr "Fehler beim Lesen der Reihenfolgedatei '%s'." +msgstr "Fehler beim Hinzufügen von Packdatei'%s'." A Space is missing: "Fehler beim Hinzufügen von Packdatei '%s'." #: run-command.c:1229 -#, fuzzy, c-format +#, c-format msgid "cannot create async thread: %s" -msgstr "kann Thread nicht erzeugen: %s" +msgstr "Kann Thread für async nicht erzeugen: %s" I think we should use "Konnte" here. Best regards, Phillip
[PATCH v2] l10n: update German translation
Signed-off-by: Ralf Thielow --- v2 updates the translation up to the latest update of git.pot. range-diff: 1: f0a6c76bf ! 1: f8313495e l10n: update German translation @@ -205,13 +205,13 @@ -msgstr "" +msgstr "Falsche Reihenfolge bei multi-pack-index Pack-Namen: '%s' vor '%s'" #: midx.c:205 #, c-format - msgid "bad pack-int-id: %u (%u total packs" + msgid "bad pack-int-id: %u (%u total packs)" -msgstr "" -+msgstr "Fehlerhafte pack-int-id: %u (%u Pakete insgesamt)" ++msgstr "Ungültige pack-int-id: %u (%u Pakete insgesamt)" #: midx.c:246 msgid "multi-pack-index stores a 64-bit offset, but off_t is too small" -msgstr "" +msgstr "multi-pack-index speichert einen 64-Bit Offset, aber off_t ist zu klein." @@ -364,31 +364,31 @@ +#, c-format msgid "unable to join load_cache_entries thread: %s" -msgstr "kann Thread nicht erzeugen: %s" +msgstr "Kann Thread für load_cache_entries nicht erzeugen: %s" - #: read-cache.c:2200 + #: read-cache.c:2201 -#, fuzzy, c-format +#, c-format msgid "unable to create load_index_extensions thread: %s" -msgstr "kann Thread nicht erzeugen: %s" +msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s" - #: read-cache.c:2227 + #: read-cache.c:2228 -#, fuzzy, c-format +#, c-format msgid "unable to join load_index_extensions thread: %s" -msgstr "kann Thread nicht erzeugen: %s" -+msgstr "Kann Thread für load_index_extensions nicht erzeugen: %s" ++msgstr "Kann Thread für load_index_extensions nicht beitreten: %s" - #: read-cache.c:2953 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086 + #: read-cache.c:2982 sequencer.c:4727 wrapper.c:658 builtin/merge.c:1086 #, c-format msgid "could not close '%s'" -msgstr "Konnte '%s' nicht schließen" +msgstr "Konnte '%s' nicht schließen." - #: read-cache.c:3026 sequencer.c:2203 sequencer.c:3592 + #: read-cache.c:3055 sequencer.c:2203 sequencer.c:3592 #, c-format @@ msgstr "Konnte '%s' nicht entfernen." #: rebase-interactive.c:10 @@ -802,14 +802,19 @@ #: builtin/grep.c:1051 -#, fuzzy msgid "invalid option combination, ignoring --threads" -msgstr "keine Unterstützung von Threads, --threads wird ignoriert" -+msgstr "ungültige Kombination von Optionen, --threads wird ignoriert" ++msgstr "Ungültige Kombination von Optionen, --threads wird ignoriert." - #: builtin/grep.c:1054 builtin/pack-objects.c:3395 + #: builtin/grep.c:1054 builtin/pack-objects.c:3397 msgid "no threads support, ignoring --threads" +-msgstr "keine Unterstützung von Threads, --threads wird ignoriert" ++msgstr "Keine Unterstützung für Threads, --threads wird ignoriert." + + #: builtin/grep.c:1057 builtin/index-pack.c:1503 builtin/pack-objects.c:2716 + #, c-format @@ msgstr "Für '%s' wurde der Alias '%s' angelegt." #: builtin/help.c:444 -#, fuzzy, c-format @@ -944,17 +949,17 @@ #: builtin/pack-objects.c:2123 msgid "suboptimal pack - out of memory" @@ "packen" - #: builtin/pack-objects.c:3316 + #: builtin/pack-objects.c:3318 -#, fuzzy msgid "respect islands during delta compression" -msgstr "Größe des Fensters für die Delta-Kompression" +msgstr "Delta-Islands bei Delta-Kompression beachten" - #: builtin/pack-objects.c:3340 + #: builtin/pack-objects.c:3342 #, c-format @@ "wurde nicht angefordert." #: builtin/pull.c:565 @@ -964,10 +969,28 @@ -msgstr "Konnte Commit '%s' nicht parsen." +msgstr "Konnte nicht auf Commit '%s' zugreifen." #: builtin/pull.c:843 msgid "ignoring --verify-signatures for rebase" +@@ + "config'." + + #: builtin/push.c:168 +-#, fuzzy, c-format ++#, c-format + msgid "" + "The upstream branch of your current branch does not match\n" + "the name of your current branch. To push to the upstream branch\n" +@@ + "Um auf den Branch mit demselben Namen im Remote-Repository zu versenden,\n" + "benutzen Sie:\n" + "\n" +-"git push %s %s\n" ++"git push %s HEAD\n" + "%s" + + #: builtin/push.c:183 @@ msgstr "unpack-trees protokollieren" #:
[PATCH] l10n: update German translation
erden; Sie werden von oben nach unten\n" "ausgeführt.\n" #: rebase-interactive.c:31 git-rebase--preserve-merges.sh:173 msgid "" @@ -4182,17 +4174,17 @@ msgstr "Fehlerhafter Feldname: %.*s" #, c-format msgid "unknown field name: %.*s" msgstr "Unbekannter Feldname: %.*s" #: ref-filter.c:539 #, c-format msgid "" "not a git repository, but the field '%.*s' requires access to object data" -msgstr "" +msgstr "Kein Git-Repository, aber das Feld '%.*s' erfordert Zugriff auf Objektdaten." #: ref-filter.c:663 #, c-format msgid "format: %%(if) atom used without a %%(then) atom" msgstr "format: %%(if) Atom ohne ein %%(then) Atom verwendet" #: ref-filter.c:726 #, c-format @@ -4446,113 +4438,111 @@ msgstr "doppelte ersetzende Referenz: %s" #: replace-object.c:73 #, c-format msgid "replace depth too high for object %s" msgstr "Ersetzungstiefe zu hoch für Objekt %s" #: rerere.c:217 rerere.c:226 rerere.c:229 msgid "corrupt MERGE_RR" -msgstr "" +msgstr "Fehlerhaftes MERGE_RR" #: rerere.c:264 rerere.c:269 -#, fuzzy msgid "unable to write rerere record" -msgstr "Konnte Notiz-Objekt nicht schreiben" +msgstr "Konnte Rerere-Eintrag nicht schreiben." #: rerere.c:485 rerere.c:692 sequencer.c:3136 sequencer.c:3162 #, c-format msgid "could not write '%s'" msgstr "Konnte '%s' nicht schreiben." #: rerere.c:495 -#, fuzzy, c-format +#, c-format msgid "there were errors while writing '%s' (%s)" -msgstr "Lesefehler beim Indizieren von '%s'." +msgstr "Fehler beim Schreiben von '%s' (%s)." #: rerere.c:498 -#, fuzzy, c-format +#, c-format msgid "failed to flush '%s'" -msgstr "Konnte '%s' nicht lesen" +msgstr "Flush bei '%s' fehlgeschlagen." #: rerere.c:503 rerere.c:1039 -#, fuzzy, c-format +#, c-format msgid "could not parse conflict hunks in '%s'" -msgstr "Konnte Commit '%s' nicht parsen." +msgstr "Konnte Konflikt-Blöcke in '%s' nicht parsen." #: rerere.c:684 -#, fuzzy, c-format +#, c-format msgid "failed utime() on '%s'" msgstr "Fehler beim Aufruf von utime() auf '%s'." #: rerere.c:694 -#, fuzzy, c-format +#, c-format msgid "writing '%s' failed" -msgstr "Konnte '%s' nicht erstellen" +msgstr "Schreiben von '%s' fehlgeschlagen." #: rerere.c:714 #, c-format msgid "Staged '%s' using previous resolution." -msgstr "" +msgstr "'%s' mit vorheriger Konfliktauflösung zum Commit vorgemerkt." #: rerere.c:753 -#, fuzzy, c-format +#, c-format msgid "Recorded resolution for '%s'." -msgstr "aufgezeichnete Auflösung von Merge-Konflikten wiederverwenden" +msgstr "Konfliktauflösung für '%s' aufgezeichnet." #: rerere.c:788 #, c-format msgid "Resolved '%s' using previous resolution." -msgstr "" +msgstr "Konflikte in '%s' mit vorheriger Konfliktauflösung beseitigt." #: rerere.c:803 -#, fuzzy, c-format +#, c-format msgid "cannot unlink stray '%s'" -msgstr "kann symbolische Verknüpfung '%s' auf '%s' nicht erstellen" +msgstr "Kann '%s' nicht löschen." #: rerere.c:807 -#, fuzzy, c-format +#, c-format msgid "Recorded preimage for '%s'" -msgstr "Konnte Log für '%s' nicht parsen." +msgstr "Preimage für '%s' aufgezeichnet." #: rerere.c:881 submodule.c:1763 builtin/submodule--helper.c:1413 #: builtin/submodule--helper.c:1423 #, c-format msgid "could not create directory '%s'" msgstr "Konnte Verzeichnis '%s' nicht erstellen." #: rerere.c:1057 -#, fuzzy, c-format +#, c-format msgid "failed to update conflicted state in '%s'" -msgstr "Fehler beim Ausführen von 'git status' auf '%s'" +msgstr "Fehler beim Aktualisieren des Konflikt-Status in '%s'." #: rerere.c:1068 rerere.c:1075 -#, fuzzy, c-format +#, c-format msgid "no remembered resolution for '%s'" -msgstr "Konnte Merge-Ergebnis von '%s' nicht hinzufügen." +msgstr "Keine aufgezeichnete Konfliktauflösung für '%s'." #: rerere.c:1077 -#, fuzzy, c-format +#, c-format msgid "cannot unlink '%s'" -msgstr "kann Verweis '%s' nicht lesen" +msgstr "Kann '%s' nicht löschen." #: rerere.c:1087 -#, fuzzy, c-format +#, c-format msgid "Updated preimage for '%s'" -msgstr "Ersetzende Referenz '%s' gelöscht." +msgstr "Preimage für '%s' aktualisiert." #: rerere.c:1096 -#, fuzzy, c-format +#, c-format msgid "Forgot resolution for '%s'\n" -msgstr "Konnte Log für '%s' nicht parsen." +msgstr "Aufgezeichnete Konfliktauflösung für '%s' gelöscht.\n" #: rerere.c:119
Re: [PATCH] doc: update diff-format.txt for removed ellipses in --raw
Greg Hurrell writes: > Since 7cb6ac1e4b ("diff: diff_aligned_abbrev: remove ellipsis after > abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff > does not add ellipses in an attempt to align the output, but the > documentation was not updated to reflect this. > > Signed-off-by: Greg Hurrell > --- > > The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now, > to bring back the old output format, but that is already documented in > git.txt, so I am not mentioning it here. Thanks. Will queue.
Re: [RFC PATCH 1/7] Documentation: update INSTALL for NetBSD
Carlo Marcelo Arenas Belón writes: > NetBSD added a BSD licensed reimplementation of GNU libintl to > its base at least since release 4.0 (mid 2012) and git can be > configured to build with it. > > Signed-off-by: Carlo Marcelo Arenas Belón > --- > INSTALL | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Looks good. > diff --git a/INSTALL b/INSTALL > index c39006e8e7..100718989f 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -154,11 +154,11 @@ Issues of note: > git-gui, you can use NO_TCLTK. > > - A gettext library is used by default for localizing Git. The > - primary target is GNU libintl, but the Solaris gettext > - implementation also works. > + primary target is GNU libintl, but the Solaris and NetBSD gettext > + implementations also work. > > We need a gettext.h on the system for C code, gettext.sh (or > - Solaris gettext(1)) for shell scripts, and libintl-perl for Perl > + gettext(1) utility) for shell scripts, and libintl-perl for Perl > programs. > > Set NO_GETTEXT to disable localization support and make Git only
[RFC PATCH 1/7] Documentation: update INSTALL for NetBSD
NetBSD added a BSD licensed reimplementation of GNU libintl to its base at least since release 4.0 (mid 2012) and git can be configured to build with it. Signed-off-by: Carlo Marcelo Arenas Belón --- INSTALL | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/INSTALL b/INSTALL index c39006e8e7..100718989f 100644 --- a/INSTALL +++ b/INSTALL @@ -154,11 +154,11 @@ Issues of note: git-gui, you can use NO_TCLTK. - A gettext library is used by default for localizing Git. The - primary target is GNU libintl, but the Solaris gettext - implementation also works. + primary target is GNU libintl, but the Solaris and NetBSD gettext + implementations also work. We need a gettext.h on the system for C code, gettext.sh (or - Solaris gettext(1)) for shell scripts, and libintl-perl for Perl + gettext(1) utility) for shell scripts, and libintl-perl for Perl programs. Set NO_GETTEXT to disable localization support and make Git only -- 2.20.0.rc1
[PATCH] doc: update diff-format.txt for removed ellipses in --raw
Since 7cb6ac1e4b ("diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff does not add ellipses in an attempt to align the output, but the documentation was not updated to reflect this. Signed-off-by: Greg Hurrell --- The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now, to bring back the old output format, but that is already documented in git.txt, so I am not mentioning it here. Documentation/diff-format.txt | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt index 706916c94c..cdcc17f0ad 100644 --- a/Documentation/diff-format.txt +++ b/Documentation/diff-format.txt @@ -26,12 +26,12 @@ line per changed file. An output line is formatted this way: -in-place edit :100644 100644 bcd1234... 0123456... M file0 -copy-edit :100644 100644 abcd123... 1234567... C68 file1 file2 -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3 -create :00 100644 000... 1234567... A file4 -delete :100644 00 1234567... 000... D file5 -unmerged :00 00 000... 000... U file6 +in-place edit :100644 100644 bcd1234 0123456 M file0 +copy-edit :100644 100644 abcd123 1234567 C68 file1 file2 +rename-edit:100644 100644 abcd123 1234567 R86 file1 file3 +create :00 100644 000 1234567 A file4 +delete :100644 00 1234567 000 D file5 +unmerged :00 00 000 000 U file6 That is, from the left to the right: @@ -75,7 +75,7 @@ and it is out of sync with the index. Example: -:100644 100644 5be4a4.. 00.. M file.c +:100644 100644 5be4a4a 000 M file.c Without the `-z` option, pathnames with "unusual" characters are @@ -100,7 +100,7 @@ from the format described above in the following way: Example: -::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c +::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c Note that 'combined diff' lists only files which were modified from -- 2.19.0
Re: [PATCH] doc: update diff-format.txt for removed ellipses
Thanks for a patch. Greg Hurrell writes: > Commit 7cb6ac1e4b made the diff format omit ellipses by default, but > there is still this place in the documentation where we show examples of > output with ellipses. We prefer to cite an existing commit with its title and date these days, not just with its object name. Since 7cb6ac1e ("diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff does not add ellipsis in an attempt to align the output, but... or something like that. Note that saying this is about the raw format is quite essential thing to tell the readers to explain this hange. > The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now, > to bring back the old output format, but that is already documented in > git.txt, so I am not mentioning it here. Yeah, I do not think it makes sense to use the workaround that is planned for removal, which will later make us revise the example in the documentation again, to end up with the text that you have right now. I do not think this three-line paragraph needs to be in the log message, either, though. Perhaps below the three-dash line. Also please sign-off your patch here (see Documentation/SubmittingPatches). > --- Thanks. > Documentation/diff-format.txt | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt > index 706916c94c..cdcc17f0ad 100644 > --- a/Documentation/diff-format.txt > +++ b/Documentation/diff-format.txt > @@ -26,12 +26,12 @@ line per changed file. > An output line is formatted this way: > > > -in-place edit :100644 100644 bcd1234... 0123456... M file0 > -copy-edit :100644 100644 abcd123... 1234567... C68 file1 file2 > -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3 > -create :00 100644 000... 1234567... A file4 > -delete :100644 00 1234567... 000... D file5 > -unmerged :00 00 000... 000... U file6 > +in-place edit :100644 100644 bcd1234 0123456 M file0 > +copy-edit :100644 100644 abcd123 1234567 C68 file1 file2 > +rename-edit:100644 100644 abcd123 1234567 R86 file1 file3 > +create :00 100644 000 1234567 A file4 > +delete :100644 00 1234567 000 D file5 > +unmerged :00 00 000 000 U file6 > > > That is, from the left to the right: > @@ -75,7 +75,7 @@ and it is out of sync with the index. > Example: > > > -:100644 100644 5be4a4.. 00.. M file.c > +:100644 100644 5be4a4a 000 M file.c > > > Without the `-z` option, pathnames with "unusual" characters are > @@ -100,7 +100,7 @@ from the format described above in the following way: > Example: > > > -::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c > +::100644 100644 100644 fabadb8 cc95eb0 4866510 MMdescribe.c > > > Note that 'combined diff' lists only files which were modified from
[PATCH] doc: update diff-format.txt for removed ellipses
Commit 7cb6ac1e4b made the diff format omit ellipses by default, but there is still this place in the documentation where we show examples of output with ellipses. The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now, to bring back the old output format, but that is already documented in git.txt, so I am not mentioning it here. --- Documentation/diff-format.txt | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt index 706916c94c..cdcc17f0ad 100644 --- a/Documentation/diff-format.txt +++ b/Documentation/diff-format.txt @@ -26,12 +26,12 @@ line per changed file. An output line is formatted this way: -in-place edit :100644 100644 bcd1234... 0123456... M file0 -copy-edit :100644 100644 abcd123... 1234567... C68 file1 file2 -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3 -create :00 100644 000... 1234567... A file4 -delete :100644 00 1234567... 000... D file5 -unmerged :00 00 000... 000... U file6 +in-place edit :100644 100644 bcd1234 0123456 M file0 +copy-edit :100644 100644 abcd123 1234567 C68 file1 file2 +rename-edit:100644 100644 abcd123 1234567 R86 file1 file3 +create :00 100644 000 1234567 A file4 +delete :100644 00 1234567 000 D file5 +unmerged :00 00 000 000 U file6 That is, from the left to the right: @@ -75,7 +75,7 @@ and it is out of sync with the index. Example: -:100644 100644 5be4a4.. 00.. M file.c +:100644 100644 5be4a4a 000 M file.c Without the `-z` option, pathnames with "unusual" characters are @@ -100,7 +100,7 @@ from the format described above in the following way: Example: -::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c +::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c Note that 'combined diff' lists only files which were modified from -- 2.19.0
[PATCH 0/1] mingw: update a link to the official documentation
It is a pretty neat thing that the bulky MSDN documentation has been replaced by something a lot more open, something that can be updated via Pull Requests on GitHub. Let's link to this new documentation instead of the obsolete one. I know, it is really close to the code freeze leading up to Git v2.20.0. But this is just an update to a code comment... :-) Johannes Schindelin (1): mingw: replace an obsolete link with the superseding one compat/mingw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) base-commit: d166e6afe5f257217836ef24a73764eba390c58d Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-81%2Fdscho%2Fmingw-update-msdn-link-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-81/dscho/mingw-update-msdn-link-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/81 -- gitgitgadget
Re: [PATCH v2 1/1] Update .mailmap
"Johannes Schindelin via GitGitGadget" writes: > From: Johannes Schindelin > > This patch makes the output of `git shortlog -nse v2.10.0..master` > duplicate-free. > > Signed-off-by: Johannes Schindelin > --- Thanks, will queue.
[PATCH v2 0/1] Update .mailmap
I noticed that there were a couple of duplicate entries in git shortlog -nse v2.19.1.., and then continued a bit to remove the duplicate entries even for v2.10.0.. Changes relative to v1: * Fixed the commit message (it talked about the opposite commit range than intended). * Added a formerly missing space between the email addresses of Masaya. Johannes Schindelin (1): Update .mailmap .mailmap | 13 + 1 file changed, 13 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-71/dscho/mailmap-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/71 Range-diff vs v1: 1: b590a9bebf ! 1: c121ebc474 Update .mailmap @@ -2,7 +2,7 @@ Update .mailmap -This patch makes the output of `git shortlog -nse v2.10.0` +This patch makes the output of `git shortlog -nse v2.10.0..master` duplicate-free. Signed-off-by: Johannes Schindelin @@ -47,7 +47,7 @@ Mark Rada Martin Langhoff Martin von Zweigbergk -+Masaya Suzuki ++Masaya Suzuki Matt Draisey Matt Kraai Matt McCutchen -- gitgitgadget
[PATCH v2 1/1] Update .mailmap
From: Johannes Schindelin This patch makes the output of `git shortlog -nse v2.10.0..master` duplicate-free. Signed-off-by: Johannes Schindelin --- .mailmap | 13 + 1 file changed, 13 insertions(+) diff --git a/.mailmap b/.mailmap index bef3352b0d..eb7b5fc7b9 100644 --- a/.mailmap +++ b/.mailmap @@ -21,6 +21,8 @@ Anders Kaseorg Aneesh Kumar K.V Amos Waterland Amos Waterland +Ben Peart +Ben Peart Ben Walton Benoit Sigoure Bernt Hansen @@ -32,6 +34,7 @@ Bryan Larsen Cheng Renquan Chris Shoemaker Chris Wright +Christian Ludwig Cord Seele Christian Couder Christian Stimming @@ -51,6 +54,7 @@ David Reiss David S. Miller David Turner David Turner +Derrick Stolee Deskin Miller Dirk Süsserott Eric Blake @@ -98,6 +102,7 @@ Jens Axboe Jens Lindström Jens Lindstrom Jim Meyering Joachim Berdal Haga +Joachim Jablon Johannes Schindelin Johannes Sixt Johannes Sixt @@ -150,6 +155,7 @@ Mark Levedahl Mark Rada Martin Langhoff Martin von Zweigbergk +Masaya Suzuki Matt Draisey Matt Kraai Matt McCutchen @@ -157,6 +163,7 @@ Matthias Kestenholz Matthias Rüster Matthias Ruester Matthias Urlichs Matthias Urlichs +Matthieu Moy Michael Coleman Michael J Gruber Michael J Gruber @@ -180,7 +187,11 @@ Nick Stokoe Nick Woolley Nick Stokoe Nick Woolley Nicolas Morey-Chaisemartin Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin Nicolas Sebrecht +Orgad Shaneh Paolo Bonzini Pascal Obry Pascal Obry @@ -200,6 +211,7 @@ Philipp A. Hartmann Philippe Bruhat Ralf Thielow Ramsay Jones +Randall S. Becker René Scharfe René Scharfe Rene Scharfe Richard Hansen @@ -238,6 +250,7 @@ Steven Walter Sven Verdoolaege Sven Verdoolaege SZEDER Gábor +Tao Qingyun <845767...@qq.com> Tay Ray Chuan Ted Percival Theodore Ts'o -- gitgitgadget
Re: [PATCH 1/1] Update .mailmap
Hi Junio, On Fri, 9 Nov 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > From: Johannes Schindelin > > > > This patch makes the output of `git shortlog -nse v2.10.0` > > duplicate-free. > > Did you mean "v2.10.0..master" or did you really mean this covers > authors recorded up to v2.10.0? Judging from the cover letter, I > think you meant the former. D'oh. Yes, I meant v2.10.0..master. > Can you say a bit more about how one among multiple addresses for > each person was chosen in the log message? E.g. "After asking each > author which one is the preferred one", "Picked the one with the > most recent committer timestamps", "There were two for each but one > of them were bouncing", etc. I looked at each of the authors' mails and tried to determine which one was the preferred one. For example, Masaya sent a patch from their gmail account but signed off using their google account, so I figured the latter was preferable. Nicolas, on the other hand, already had a couple of entries in .mailmap, so I picked the one that seemed to be preferred judging by the .mailmap even if it was not in use lately. For Christian, it was gmail vs googlemail, and I picked the former, as it is more common these days. For Ben and Stolee, I used their Microsoft accounts (preferring the one Ben used originally). For Joachim, Matthieu, Randall and Tao, I used the more personalized email address. For Orgad, I used his personal one rather than his work email. > > @@ -150,6 +155,7 @@ Mark Levedahl > > Mark Rada > > Martin Langhoff > > Martin von Zweigbergk > > > > +Masaya Suzuki > > It is a bit surprising that we can take an entry without SP in > between two addresses and still behave sensibley, but it probably > makes more sense to add one just to look similar to other entries. Whoops. Thanks, fixed! > Thanks for working on this. You're welcome. v2 about to be sent, Dscho
Re: [PATCH 1/1] Update .mailmap
"Johannes Schindelin via GitGitGadget" writes: > From: Johannes Schindelin > > This patch makes the output of `git shortlog -nse v2.10.0` > duplicate-free. Did you mean "v2.10.0..master" or did you really mean this covers authors recorded up to v2.10.0? Judging from the cover letter, I think you meant the former. Can you say a bit more about how one among multiple addresses for each person was chosen in the log message? E.g. "After asking each author which one is the preferred one", "Picked the one with the most recent committer timestamps", "There were two for each but one of them were bouncing", etc. > @@ -150,6 +155,7 @@ Mark Levedahl > Mark Rada > Martin Langhoff > Martin von Zweigbergk > > +Masaya Suzuki It is a bit surprising that we can take an entry without SP in between two addresses and still behave sensibley, but it probably makes more sense to add one just to look similar to other entries. Thanks for working on this.
[PATCH 1/1] Update .mailmap
From: Johannes Schindelin This patch makes the output of `git shortlog -nse v2.10.0` duplicate-free. Signed-off-by: Johannes Schindelin --- .mailmap | 13 + 1 file changed, 13 insertions(+) diff --git a/.mailmap b/.mailmap index bef3352b0d..1d8310073a 100644 --- a/.mailmap +++ b/.mailmap @@ -21,6 +21,8 @@ Anders Kaseorg Aneesh Kumar K.V Amos Waterland Amos Waterland +Ben Peart +Ben Peart Ben Walton Benoit Sigoure Bernt Hansen @@ -32,6 +34,7 @@ Bryan Larsen Cheng Renquan Chris Shoemaker Chris Wright +Christian Ludwig Cord Seele Christian Couder Christian Stimming @@ -51,6 +54,7 @@ David Reiss David S. Miller David Turner David Turner +Derrick Stolee Deskin Miller Dirk Süsserott Eric Blake @@ -98,6 +102,7 @@ Jens Axboe Jens Lindström Jens Lindstrom Jim Meyering Joachim Berdal Haga +Joachim Jablon Johannes Schindelin Johannes Sixt Johannes Sixt @@ -150,6 +155,7 @@ Mark Levedahl Mark Rada Martin Langhoff Martin von Zweigbergk +Masaya Suzuki Matt Draisey Matt Kraai Matt McCutchen @@ -157,6 +163,7 @@ Matthias Kestenholz Matthias Rüster Matthias Ruester Matthias Urlichs Matthias Urlichs +Matthieu Moy Michael Coleman Michael J Gruber Michael J Gruber @@ -180,7 +187,11 @@ Nick Stokoe Nick Woolley Nick Stokoe Nick Woolley Nicolas Morey-Chaisemartin Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin Nicolas Sebrecht +Orgad Shaneh Paolo Bonzini Pascal Obry Pascal Obry @@ -200,6 +211,7 @@ Philipp A. Hartmann Philippe Bruhat Ralf Thielow Ramsay Jones +Randall S. Becker René Scharfe René Scharfe Rene Scharfe Richard Hansen @@ -238,6 +250,7 @@ Steven Walter Sven Verdoolaege Sven Verdoolaege SZEDER Gábor +Tao Qingyun <845767...@qq.com> Tay Ray Chuan Ted Percival Theodore Ts'o -- gitgitgadget
[PATCH 0/1] Update .mailmap
I noticed that there were a couple of duplicate entries in git shortlog -nse v2.19.1.., and then continued a bit to remove the duplicate entries even for v2.10.0.. Johannes Schindelin (1): Update .mailmap .mailmap | 13 + 1 file changed, 13 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-71/dscho/mailmap-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/71 -- gitgitgadget
Update
Hello Dear how are you doing today? my name is Bruce Blake, the manager foreign affairs in City Finance Bank, we have a customer here in bank that has not accessed his account for the past 18 years, after some research made about him we found out he was a victim of the crashed mining company in mexico. he died a single man no wife One son from his first marriage that he divorced, his next of kin is that his only son which we can arrange and make your name his next of kin as his brother, and the mandatory rule of the bank is that after 25 years of a dormant account, the fund should be moved to Government Treasury account. This Customer was my friend because i have known him since he opened an account with us, i can't claim his fund because i am a staff in this bank, that is why i want you to come in and claim this fund as my late clients Relative. if you are interested get back to me so we could discuss and make arrangements for the paper work because everything has to be legit and Legal. Regards Bruce Blake.
Re: [PATCH v3 1/2] worktree: update documentation for lock_reason and lock_reason_valid
nbelakov...@gmail.com writes: > From: Nickolai Belakovski > > Clarify that these fields are to be considered implementation details > and direct the reader to use the is_worktree_locked function to retrieve > said information. > > Signed-off-by: Nickolai Belakovski > --- > worktree.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/worktree.h b/worktree.h > index df3fc30f7..6b12a3cf6 100644 > --- a/worktree.h > +++ b/worktree.h > @@ -10,12 +10,12 @@ struct worktree { > char *path; > char *id; > char *head_ref; /* NULL if HEAD is broken or detached */ > - char *lock_reason; /* internal use */ > + char *lock_reason; /* private - use is_worktree_locked */ s/use /used by/, probably. > struct object_id head_oid; > int is_detached; > int is_bare; > int is_current; > - int lock_reason_valid; > + int lock_reason_valid; /* private */ > }; These annotations to the two fields are not wrong per-se, but I have a feeling that it would equally be important to document what the other "non-private" fields mean, if peeking them *is* the API this subsystem offers. Thanks.
[PATCH v3 1/2] worktree: update documentation for lock_reason and lock_reason_valid
From: Nickolai Belakovski Clarify that these fields are to be considered implementation details and direct the reader to use the is_worktree_locked function to retrieve said information. Signed-off-by: Nickolai Belakovski --- worktree.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/worktree.h b/worktree.h index df3fc30f7..6b12a3cf6 100644 --- a/worktree.h +++ b/worktree.h @@ -10,12 +10,12 @@ struct worktree { char *path; char *id; char *head_ref; /* NULL if HEAD is broken or detached */ - char *lock_reason; /* internal use */ + char *lock_reason; /* private - use is_worktree_locked */ struct object_id head_oid; int is_detached; int is_bare; int is_current; - int lock_reason_valid; + int lock_reason_valid; /* private */ }; /* Functions for acting on the information about worktrees. */ -- 2.14.2
Re: [PATCH] update git-http-backend doc for lighttpd
Glenn Strauss writes: > use "GIT_HTTP_EXPORT_ALL" => "1" with a value for best compatiblity. > lighttpd 1.4.51 setenv.add-environment does add vars with empty value. > lighttpd setenv.set-environment does, but was only introduced in 1.4.46 > > git-http-backend may be found at /usr/libexec/git-core/git-http-backend > > scope lighttpd config directives for git-http-backend under "^/git" > > Signed-off-by: Glenn Strauss > --- > Documentation/git-http-backend.txt | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Documentation/git-http-backend.txt > b/Documentation/git-http-backend.txt > index bb0db195cebd6..905aa1056d26f 100644 > --- a/Documentation/git-http-backend.txt > +++ b/Documentation/git-http-backend.txt > @@ -192,16 +192,16 @@ ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/ > > Lighttpd:: > Ensure that `mod_cgi`, `mod_alias`, `mod_auth`, `mod_setenv` are > - loaded, then set `GIT_PROJECT_ROOT` appropriately and redirect > - all requests to the CGI: > + loaded, then set path to git-http-backend, set `GIT_PROJECT_ROOT` > + appropriately, and redirect all requests to the CGI: The addition here is set path to git-http-backend That reads as if you are telling the reader to do this GIT_PROJECT_ROOT => "/var/www/git", path => "/usr/libexec/git-core/git-http-backend" because the descriptions for these two are next to each other and so similar, but I somehow do not think you meant there is a variable whose name is `path` (note that I do not use lighttpd and am not an expert on its configuration---which makes me the ideal guinea pig to judge if your update makes sense to the target audience). Do you mean something like use `alias.url` to mark that `/git` hierarchy is handled by the `git-http-backend` binary (use the full path to the program). I do not see any quoting in your updated text, but many of what the end-user needs to type literally must be `quoted for monospace`, I would think. > + > > -alias.url += ( "/git" => "/usr/lib/git-core/git-http-backend" ) > $HTTP["url"] =~ "^/git" { > + alias.url += ("/git" => "/usr/libexec/git-core/git-http-backend") > cgi.assign = ("" => "") > setenv.add-environment = ( > "GIT_PROJECT_ROOT" => "/var/www/git", > - "GIT_HTTP_EXPORT_ALL" => "" > + "GIT_HTTP_EXPORT_ALL" => "1" > ) > } > > > -- > https://github.com/git/git/pull/546 Thanks.
[PATCH] update git-http-backend doc for lighttpd
use "GIT_HTTP_EXPORT_ALL" => "1" with a value for best compatiblity. lighttpd 1.4.51 setenv.add-environment does add vars with empty value. lighttpd setenv.set-environment does, but was only introduced in 1.4.46 git-http-backend may be found at /usr/libexec/git-core/git-http-backend scope lighttpd config directives for git-http-backend under "^/git" Signed-off-by: Glenn Strauss --- Documentation/git-http-backend.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt index bb0db195cebd6..905aa1056d26f 100644 --- a/Documentation/git-http-backend.txt +++ b/Documentation/git-http-backend.txt @@ -192,16 +192,16 @@ ScriptAlias /git/ /var/www/cgi-bin/gitweb.cgi/ Lighttpd:: Ensure that `mod_cgi`, `mod_alias`, `mod_auth`, `mod_setenv` are - loaded, then set `GIT_PROJECT_ROOT` appropriately and redirect - all requests to the CGI: + loaded, then set path to git-http-backend, set `GIT_PROJECT_ROOT` + appropriately, and redirect all requests to the CGI: + -alias.url += ( "/git" => "/usr/lib/git-core/git-http-backend" ) $HTTP["url"] =~ "^/git" { + alias.url += ("/git" => "/usr/libexec/git-core/git-http-backend") cgi.assign = ("" => "") setenv.add-environment = ( "GIT_PROJECT_ROOT" => "/var/www/git", - "GIT_HTTP_EXPORT_ALL" => "" + "GIT_HTTP_EXPORT_ALL" => "1" ) } -- https://github.com/git/git/pull/546
[PATCH 01/78] Update makefile in preparation for Documentation/config/*.txt
config.txt is going to be broken down in smaller pieces and put under Documentation/config directory. Update build rules to take these files into account. A dummy file is added to make sure wildcard expansion is predictable (depending on shell setting it could expand to nothing or becomes a path if config directory is empty). The file will be deleted once the move is over. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/Makefile | 2 +- Documentation/config/dummy.txt | 0 Makefile | 2 +- generate-cmdlist.sh| 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 Documentation/config/dummy.txt diff --git a/Documentation/Makefile b/Documentation/Makefile index 95f6a321f2..48d261dc2c 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -285,7 +285,7 @@ docdep_prereqs = \ mergetools-list.made $(mergetools_txt) \ cmd-list.made $(cmds_txt) -doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl +doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) build-docdep.perl $(QUIET_GEN)$(RM) $@+ $@ && \ $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \ mv $@+ $@ diff --git a/Documentation/config/dummy.txt b/Documentation/config/dummy.txt new file mode 100644 index 00..e69de29bb2 diff --git a/Makefile b/Makefile index b08d5ea258..e2ca83203f 100644 --- a/Makefile +++ b/Makefile @@ -2068,7 +2068,7 @@ $(BUILT_INS): git$X command-list.h: generate-cmdlist.sh command-list.txt -command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt +command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt $(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh index fa1e5475e8..709d67405b 100755 --- a/generate-cmdlist.sh +++ b/generate-cmdlist.sh @@ -80,7 +80,7 @@ print_config_list () { cat <
Re: [PATCH 10/10] builtin/fetch: check for submodule updates in any ref update
> Gerrit, the code review tool, has a different workflow than our mailing > list based approach. Usually users upload changes to a Gerrit server and > continuous integration and testing happens by bots. Sometimes however a > user wants to checkout a change locally and look at it locally. For this > use case, Gerrit offers a command line snippet to copy and paste to your > terminal, which looks like As I said in my review of the previous patch, I think this should be squashed into the previous patch. Also, remember to add the version when formatting the e-mail ("[PATCH v? ??/??]").
[PATCH 10/10] builtin/fetch: check for submodule updates in any ref update
Gerrit, the code review tool, has a different workflow than our mailing list based approach. Usually users upload changes to a Gerrit server and continuous integration and testing happens by bots. Sometimes however a user wants to checkout a change locally and look at it locally. For this use case, Gerrit offers a command line snippet to copy and paste to your terminal, which looks like git fetch https:///gerrit refs/changes/ && git checkout FETCH_HEAD For Gerrit changes that contain changing submodule gitlinks, it would be easy to extend both the fetch and checkout with the '--recurse-submodules' flag, such that this command line snippet would produce the state of a change locally. However the functionality added in the previous patch, which would ensure that we fetch the objects in the submodule that the gitlink pointed at, only works for remote tracking branches so far, not for FETCH_HEAD. Make sure that fetching a superproject to its FETCH_HEAD, also respects the existence checks for objects in the submodule recursion. Signed-off-by: Stefan Beller --- builtin/fetch.c | 8 ++-- t/t5526-fetch-submodules.sh | 24 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 95c44bf6ff..f39012d7c2 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -700,8 +700,6 @@ static int update_local_ref(struct ref *ref, what = _("[new ref]"); } - if (recurse_submodules != RECURSE_SUBMODULES_OFF) - check_for_new_submodule_commits(>new_oid); r = s_update_ref(msg, ref, 0); format_display(display, r ? '!' : '*', what, r ? _("unable to update local ref") : NULL, @@ -715,8 +713,6 @@ static int update_local_ref(struct ref *ref, strbuf_add_unique_abbrev(, >object.oid, DEFAULT_ABBREV); strbuf_addstr(, ".."); strbuf_add_unique_abbrev(, >new_oid, DEFAULT_ABBREV); - if (recurse_submodules != RECURSE_SUBMODULES_OFF) - check_for_new_submodule_commits(>new_oid); r = s_update_ref("fast-forward", ref, 1); format_display(display, r ? '!' : ' ', quickref.buf, r ? _("unable to update local ref") : NULL, @@ -729,8 +725,6 @@ static int update_local_ref(struct ref *ref, strbuf_add_unique_abbrev(, >object.oid, DEFAULT_ABBREV); strbuf_addstr(, "..."); strbuf_add_unique_abbrev(, >new_oid, DEFAULT_ABBREV); - if (recurse_submodules != RECURSE_SUBMODULES_OFF) - check_for_new_submodule_commits(>new_oid); r = s_update_ref("forced-update", ref, 1); format_display(display, r ? '!' : '+', quickref.buf, r ? _("unable to update local ref") : _("forced update"), @@ -826,6 +820,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, ref->force = rm->peer_ref->force; } + if (recurse_submodules != RECURSE_SUBMODULES_OFF) + check_for_new_submodule_commits(>old_oid); if (!strcmp(rm->name, "HEAD")) { kind = ""; diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 5a75b57852..799785783f 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -631,4 +631,28 @@ test_expect_success "fetch new submodule commits on-demand outside standard refs ) ' +test_expect_success 'fetch new submodule commits on-demand in FETCH_HEAD' ' + # depends on the previous test for setup + + C=$(git -C submodule commit-tree -m "another change outside refs/heads" HEAD^{tree}) && + git -C submodule update-ref refs/changes/1 $C && + git update-index --cacheinfo 16 $C submodule && + test_tick && + + D=$(git -C sub1 commit-tree -m "another change outside refs/heads" HEAD^{tree}) && + git -C sub1 update-ref refs/changes/2 $D && + git update-index --cacheinfo 16 $D sub1 && + + git commit -m "updated submodules outside of refs/heads" && + E=$(git rev-parse HEAD) && + git update-ref refs/changes/2 $E && + ( + cd downstream && + git fetch --recurse-submodules origin refs/changes/2 && + git -C submodule cat-file -t $C && + git -C sub1 cat-file -t $D && + git checkout --recurse-submodules FETCH_HEAD + ) +' + test_done -- 2.19.0
[PATCH 6/6] doc: fix formatting in git-update-ref
Remove the parapgraph numbers from lines explaining the reflog format and typeset these lines in monospace. Signed-off-by: Andreas Heiduk --- Documentation/git-update-ref.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index fda8516677..9671423117 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -129,8 +129,8 @@ a line to the log file "$GIT_DIR/logs/" (dereferencing all symbolic refs before creating the log name) describing the change in ref value. Log lines are formatted as: -. oldsha1 SP newsha1 SP committer LF -+ +oldsha1 SP newsha1 SP committer LF + Where "oldsha1" is the 40 character hexadecimal value previously stored in , "newsha1" is the 40 character hexadecimal value of and "committer" is the committer's name, email address @@ -138,8 +138,8 @@ and date in the standard Git committer ident format. Optionally with -m: -. oldsha1 SP newsha1 SP committer TAB message LF -+ +oldsha1 SP newsha1 SP committer TAB message LF + Where all fields are as described above and "message" is the value supplied to the -m option. -- 2.19.1
Re: [PATCH] receive: denyCurrentBranch=updateinstead should not blindly update
Hi Junio, On Fri, 19 Oct 2018, Junio C Hamano wrote: > The handling of receive.denyCurrentBranch=updateInstead was added to > a switch statement that handles other values of the variable, but > all the other case arms only checked a condition to reject the > attempted push, or let later logic in the same function to still > intervene, so that a push that does not fast-forward (which is > checked after the switch statement in question) is still rejected. > > But the handling of updateInstead incorrectly took immediate effect, > without giving other checks a chance to intervene. > > Instead of calling update_worktree() that causes the side effect > immediately, just note the fact that we will need to call the > funciton later, and first give other checks chance to reject the > request. After the update-hook gets a chance to reject the push > (which happens as the last step in a series of checks), call > update_worktree() when we earlier detected the need to. Nicely explained, and the patch looks good to me, too. Thanks, Dscho > > Reported-by: Rajesh Madamanchi > Signed-off-by: Junio C Hamano > --- > builtin/receive-pack.c | 12 +--- > t/t5516-fetch-push.sh | 8 +++- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > index 95740f4f0e..79ee320948 100644 > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c > @@ -1026,6 +1026,7 @@ static const char *update(struct command *cmd, struct > shallow_info *si) > const char *ret; > struct object_id *old_oid = >old_oid; > struct object_id *new_oid = >new_oid; > + int do_update_worktree = 0; > > /* only refs/... are allowed */ > if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) { > @@ -1051,9 +1052,8 @@ static const char *update(struct command *cmd, struct > shallow_info *si) > refuse_unconfigured_deny(); > return "branch is currently checked out"; > case DENY_UPDATE_INSTEAD: > - ret = update_worktree(new_oid->hash); > - if (ret) > - return ret; > + /* pass -- let other checks intervene first */ > + do_update_worktree = 1; > break; > } > } > @@ -1118,6 +1118,12 @@ static const char *update(struct command *cmd, struct > shallow_info *si) > return "hook declined"; > } > > + if (do_update_worktree) { > + ret = update_worktree(new_oid->hash); > + if (ret) > + return ret; > + } > + > if (is_null_oid(new_oid)) { > struct strbuf err = STRBUF_INIT; > if (!parse_object(the_repository, old_oid)) { > diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh > index 7a8f56db53..7316365a24 100755 > --- a/t/t5516-fetch-push.sh > +++ b/t/t5516-fetch-push.sh > @@ -1577,7 +1577,13 @@ test_expect_success 'receive.denyCurrentBranch = > updateInstead' ' > test $(git -C .. rev-parse master) = $(git rev-parse HEAD) && > git diff --quiet && > git diff --cached --quiet > - ) > + ) && > + > + # (6) updateInstead intervened by fast-forward check > + test_must_fail git push void master^:master && > + test $(git -C void rev-parse HEAD) = $(git rev-parse master) && > + git -C void diff --quiet && > + git -C void diff --cached --quiet > ' > > test_expect_success 'updateInstead with push-to-checkout hook' ' > -- > 2.19.1-450-ga4b8ab5363 > >
Re: [PATCH] receive: denyCurrentBranch=updateinstead should not blindly update
On Fri, Oct 19, 2018 at 12:57 AM Junio C Hamano wrote: > The handling of receive.denyCurrentBranch=updateInstead was added to > a switch statement that handles other values of the variable, but > all the other case arms only checked a condition to reject the > attempted push, or let later logic in the same function to still > intervene, so that a push that does not fast-forward (which is > checked after the switch statement in question) is still rejected. > > But the handling of updateInstead incorrectly took immediate effect, > without giving other checks a chance to intervene. > > Instead of calling update_worktree() that causes the side effect > immediately, just note the fact that we will need to call the > funciton later, and first give other checks chance to reject the s/funciton/function s/chance/a &/ > request. After the update-hook gets a chance to reject the push > (which happens as the last step in a series of checks), call > update_worktree() when we earlier detected the need to. > > Reported-by: Rajesh Madamanchi > Signed-off-by: Junio C Hamano
[PATCH] receive: denyCurrentBranch=updateinstead should not blindly update
The handling of receive.denyCurrentBranch=updateInstead was added to a switch statement that handles other values of the variable, but all the other case arms only checked a condition to reject the attempted push, or let later logic in the same function to still intervene, so that a push that does not fast-forward (which is checked after the switch statement in question) is still rejected. But the handling of updateInstead incorrectly took immediate effect, without giving other checks a chance to intervene. Instead of calling update_worktree() that causes the side effect immediately, just note the fact that we will need to call the funciton later, and first give other checks chance to reject the request. After the update-hook gets a chance to reject the push (which happens as the last step in a series of checks), call update_worktree() when we earlier detected the need to. Reported-by: Rajesh Madamanchi Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 12 +--- t/t5516-fetch-push.sh | 8 +++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 95740f4f0e..79ee320948 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1026,6 +1026,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) const char *ret; struct object_id *old_oid = >old_oid; struct object_id *new_oid = >new_oid; + int do_update_worktree = 0; /* only refs/... are allowed */ if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) { @@ -1051,9 +1052,8 @@ static const char *update(struct command *cmd, struct shallow_info *si) refuse_unconfigured_deny(); return "branch is currently checked out"; case DENY_UPDATE_INSTEAD: - ret = update_worktree(new_oid->hash); - if (ret) - return ret; + /* pass -- let other checks intervene first */ + do_update_worktree = 1; break; } } @@ -1118,6 +1118,12 @@ static const char *update(struct command *cmd, struct shallow_info *si) return "hook declined"; } + if (do_update_worktree) { + ret = update_worktree(new_oid->hash); + if (ret) + return ret; + } + if (is_null_oid(new_oid)) { struct strbuf err = STRBUF_INIT; if (!parse_object(the_repository, old_oid)) { diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 7a8f56db53..7316365a24 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1577,7 +1577,13 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' ' test $(git -C .. rev-parse master) = $(git rev-parse HEAD) && git diff --quiet && git diff --cached --quiet - ) + ) && + + # (6) updateInstead intervened by fast-forward check + test_must_fail git push void master^:master && + test $(git -C void rev-parse HEAD) = $(git rev-parse master) && + git -C void diff --quiet && + git -C void diff --cached --quiet ' test_expect_success 'updateInstead with push-to-checkout hook' ' -- 2.19.1-450-ga4b8ab5363
Re: [PATCH 2/2] t3430: update to test with custom commentChar
Hi Daniel, [I forgot to address this mail earlier, my apologies] On Tue, 10 Jul 2018, Daniel Harding wrote: > On Tue, 10 Jul 2018 at 16:08:57 +0300, Johannes Schindelin wrote:> > > On Tue, 10 Jul 2018, Daniel Harding wrote: > > > > > On Mon, 09 Jul 2018 at 22:14:58 +0300, Johannes Schindelin wrote: > > > > > > > > On Mon, 9 Jul 2018, Daniel Harding wrote: > > > > > > > > > > On Mon, 09 Jul 2018 at 00:02:00 +0300, brian m. carlson wrote: > > > > > > > > > > > > Should this affect the "# Merge the topic branch" line (and the "# > > > > > > C", > > > > > > "# E", and "# H" lines in the next test) that appears below this? > > > > > > It > > > > > > would seem those would qualify as comments as well. > > > > > > > > > > I intentionally did not change that behavior for two reasons: > > > > > > > > > > a) from a Git perspective, comment characters are only effectual for > > > > > comments > > > > > if they are the first character in a line > > > > > > > > > > and > > > > > > > > > > b) there are places where a '#' character from the todo list is > > > > > actually > > > > > parsed and used e.g. [0] and [1]. I have not yet gotten to the point > > > > > of > > > > > grokking what is going on there, so I didn't want to risk breaking > > > > > something I > > > > > didn't understand. Perhaps Johannes could shed some light on whether > > > > > the > > > > > cases you mentioned should be changed to use the configured > > > > > commentChar or > > > > > not. > > > > > > > > > > [0] > > > > > https://github.com/git/git/blob/53f9a3e157dbbc901a02ac2c73346d375e24978c/sequencer.c#L2869 > > > > > [1] > > > > > https://github.com/git/git/blob/53f9a3e157dbbc901a02ac2c73346d375e24978c/sequencer.c#L3797 > > > > > > > > These are related. The first one tries to support > > > > > > > > merge -C cafecafe second-branch third-branch # Octopus 2nd/3rd branch > > > > > > > > i.e. use '#' to separate between the commit(s) to merge and the oneline > > > > (the latter for the reader's pleasure, just like the onelines in the > > > > `pick > > > > ` lines. > > > > > > > > The second ensures that there is no valid label `#`. > > > > > > > > I have not really thought about the ramifications of changing this to > > > > comment_line_char, but I guess it *could* work if both locations were > > > > changed. > > > > > > Is there interest in such a change? I'm happy to take a stab at it if > > > there > > > is, otherwise I'll leave things as they are. > > > > I think it would be a fine change, once we convinced ourselves that it > > does not break things (I am a little worried about this because I remember > > just how long I had to reflect about the ramifications with regards to the > > label: `#` is a valid ref name, after all, and that was the reason why I > > had to treat it specially, and I wonder whether allowing arbitrary comment > > chars will require us to add more such special handling that is not > > necessary if we stick to `#`). > > Would it simpler/safer to perhaps put the oneline on its own commented line > above? I know it isn't quite consistent with the way onelines are displayed > for normal commits, but it might be a worthwhile tradeoff for the sake of the > code. As an idea of what I am suggesting, your example above would become > perhaps > > # Merge: Octopus 2nd/3rd branch > merge -C cafecafe second-branch third-branch > > or perhaps just > > # Octopus 2nd/3rd branch > merge -C cafecafe second-branch third-branch > > Thoughts? That is a very good idea, if you ask me! Unfortunately, I forgot about this suggestion, and IIUC v2.19.0 shipped with my previously-suggested version. But maybe you want to spend a little time on the code to change it according to your suggestion? The `merge` commands are generated here: https://github.com/git/git/blob/v2.19.0/sequencer.c#L4096-L4120 > > Not that the comment line char feature seems to be all that safe. I could > > imagine that setting it to ' ' (i.e. a single space) wreaks havoc with > > Git, and we have no safeguard to error out in this obviously broken case. > > Technically, I think a single space might actually work with commit messages > (at least, I can't off the top of my head think of a case where git would > insert a non-comment line starting with a space if it wasn't already present > in a commit message). But if someone were actually crazy enough to do that I > might suggest a diagnosis of "if it hurts, don't do that" rather than trying > to equip git defend against that sort of thing. I did want to have an extra special visual marker for users (such as myself), so a space would not quite be enough. That's why I settled for the comment char. Ciao, Johannes
[PATCH] submodule update --remote: introduce pinning
gitmodules(5) sayeth: submodule..branch A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to master. This doesn't allow having a "pinned" submodule that should not be updated from upstream. We should change this to have no default --- if branch is not specified, don't update that submodule, just like in Gerrit's corresponding feature[1]. [1] https://gerrit-review.googlesource.com/Documentation/user-submodules.html#_defining_the_submodule_branch However changing defaults is difficult as it may surprise the user, Jonathan came up with a 4 step plan: Step 0: introduce # current behavior: git submodule update --remote --remote-default-to-master # new behavior: git submodule update --remote --remote-pinned and treat plain "git submodule update --remote" as --default-to-master. Step 1: when neither --default-to-master nor --no-default-to-master has been passed, warn when encountering a submodule with no branch and treat it as "master". Step 2: when neither --default-to-master nor --no-default-to-master has been passed, error out when encountering a submodule with no branch. Step 3: when neither --default-to-master nor --no-default-to-master has been passed, warn when encountering a submodule with no branch and treat it as pinned. Step 4: eliminate the warning. This implements step 0 and 1. Signed-off-by: Stefan Beller --- Sorry that this took so long, this is a rough patch for steps 0 & 1, I'd still need to update docs. Stefan Documentation/gitmodules.txt | 11 ++- builtin/submodule--helper.c | 36 +--- git-submodule.sh | 34 +- t/t7406-submodule-update.sh | 29 + 4 files changed, 89 insertions(+), 21 deletions(-) diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 4d63def206..fe42dbdb3e 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -50,11 +50,12 @@ submodule..update:: submodule..branch:: A remote branch name for tracking updates in the upstream submodule. - If the option is not specified, it defaults to 'master'. A special - value of `.` is used to indicate that the name of the branch in the - submodule should be the same name as the current branch in the - current repository. See the `--remote` documentation in - linkgit:git-submodule[1] for details. + A special value of `.` is used to indicate that the name of the + branch in the submodule should be the same name as the current + branch in the current repository. See the `--remote` documentation + in linkgit:git-submodule[1] for details. + If not set, the default for `git submodule update --remote` is + to update the submodule to the superproject's recorded object id. submodule..fetchRecurseSubmodules:: This option can be used to control recursive fetching of this diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 40844870cf..6413f2b410 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1889,7 +1889,7 @@ static int resolve_relative_path(int argc, const char **argv, const char *prefix return 0; } -static const char *remote_submodule_branch(const char *path) +static const char *remote_submodule_branch(const char *path, int default_pinned) { const struct submodule *sub; const char *branch = NULL; @@ -1904,8 +1904,12 @@ static const char *remote_submodule_branch(const char *path) branch = sub->branch; free(key); - if (!branch) - return "master"; + if (!branch) { + if (default_pinned) + return ""; + else + return "master"; + } if (!strcmp(branch, ".")) { const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL); @@ -1932,12 +1936,30 @@ static int resolve_remote_submodule_branch(int argc, const char **argv, { const char *ret; struct strbuf sb = STRBUF_INIT; - if (argc != 2) - die("submodule--helper remote-branch takes exactly one arguments, got %d", argc); + int default_pinned = 0; + + struct option remote_options[] = { + OPT_SET_INT(0, "default-master", _pinned, + N_("unconfigured submodules default to master branch"), 0), + OPT_SET_INT(0, "default-pinned", _pinned, + N_("unconfigured submodules default to superproject object id"), 1), + OPT_END() + }; + + const char *const git_submodule_helper_usage[] = { + N_("
Re: [PATCH v9 04/21] stash: update test cases conform to coding guidelines
> Subject: stash: update test cases conform to coding guidelines s/stash/t3903/ s/conform/to &/ Alternatively the subject could also be "t3903: modernize style", which would be a bit shorter, and still convey the same information to a reader of 'git log --oneline'. On 09/26, Paul-Sebastian Ungureanu wrote: > Removed whitespaces after redirection operators. s/Removed/Remove/. Commit messages should always use the imperative mood, as described in Documentation/SubmittingPatches. > > Signed-off-by: Paul-Sebastian Ungureanu > --- > t/t3903-stash.sh | 120 --- > 1 file changed, 61 insertions(+), 59 deletions(-) > > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index af7586d43d..de6cab1fe7 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -8,22 +8,22 @@ test_description='Test git stash' > . ./test-lib.sh > > test_expect_success 'stash some dirty working directory' ' > - echo 1 > file && > + echo 1 >file && > git add file && > echo unrelated >other-file && > git add other-file && > test_tick && > git commit -m initial && > - echo 2 > file && > + echo 2 >file && > git add file && > - echo 3 > file && > + echo 3 >file && > test_tick && > git stash && > git diff-files --quiet && > git diff-index --cached --quiet HEAD > ' > > -cat > expect << EOF > +cat >expect < diff --git a/file b/file > index 0cfbf08..00750ed 100644 > --- a/file > @@ -35,7 +35,7 @@ EOF > > test_expect_success 'parents of stash' ' > test $(git rev-parse stash^) = $(git rev-parse HEAD) && > - git diff stash^2..stash > output && > + git diff stash^2..stash >output && > test_cmp output expect > ' > > @@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' ' > > test_expect_success 'apply stashed changes (including index)' ' > git reset --hard HEAD^ && > - echo 6 > other-file && > + echo 6 >other-file && > git add other-file && > test_tick && > git commit -m other-file && > @@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra > options' ' > > test_expect_success 'drop top stash' ' > git reset --hard && > - git stash list > stashlist1 && > - echo 7 > file && > + git stash list >expected && > + echo 7 >file && > git stash && > git stash drop && > - git stash list > stashlist2 && > - test_cmp stashlist1 stashlist2 && > + git stash list >actual && > + test_cmp expected actual && > git stash apply && > test 3 = $(cat file) && > test 1 = $(git show :file) && > @@ -113,9 +113,9 @@ test_expect_success 'drop top stash' ' > > test_expect_success 'drop middle stash' ' > git reset --hard && > - echo 8 > file && > + echo 8 >file && > git stash && > - echo 9 > file && > + echo 9 >file && > git stash && > git stash drop stash@{1} && > test 2 = $(git stash list | wc -l) && > @@ -160,7 +160,7 @@ test_expect_success 'stash pop' ' > test 0 = $(git stash list | wc -l) > ' > > -cat > expect << EOF > +cat >expect < diff --git a/file2 b/file2 > new file mode 100644 > index 000..1fe912c > @@ -170,7 +170,7 @@ index 000..1fe912c > +bar2 > EOF > > -cat > expect1 << EOF > +cat >expect1 < diff --git a/file b/file > index 257cc56..5716ca5 100644 > --- a/file > @@ -180,7 +180,7 @@ index 257cc56..5716ca5 100644 > +bar > EOF > > -cat > expect2 << EOF > +cat >expect2 < diff --git a/file b/file > index 7601807..5716ca5 100644 > --- a/file > @@ -198,79 +198,79 @@ index 000..1fe912c > EOF > > test_expect_success 'stash branch' ' > - echo foo > file && > + echo foo >file && > git commit file -m first && > - echo bar > file && > - echo bar2 > file2 && > + echo bar >file && > + echo bar2 >file2 && > git add file2 && > git stash && > - echo baz > file && > + echo
Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > Junio, can you squash in the following patch or would you prefer I > reroll the entire series? Squash it to f8cd77d5 ("fsmonitor: update GIT_TEST_FSMONITOR support", 2018-09-18) and use the two new lines in the log message? I can do that. Thanks.
Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/28/2018 10:21 AM, Ben Peart wrote: On 9/28/2018 6:01 AM, SZEDER Gábor wrote: On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote: diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we are good to go. diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. But this old comment is different, suggesting copying that script to our $PATH. I prefer your instructions above, because it's only a single step, and, more importantly, it won't pollute my $PATH. I think this comment should be updated to make the advices in both places consistent. Or perhaps even removed, now that all GIT_TEST variables are documented in the same place? I prefer the suggestion to simply remove this text from the test script now that there is documentation for it in the t/README file. Junio, can you squash in the following patch or would you prefer I reroll the entire series? Thanks, Ben From 393007340dc1baf3539ab727e0a8128e7c408a27 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Fri, 28 Sep 2018 10:23:18 -0400 Subject: fixup! fsmonitor: remove outdated instructions from test Remove the outdated instructions on how to run the test suite utilizing fsmonitor now that it is properly documented in t/README. Signed-off-by: Ben Peart --- Notes: Base Ref: git-test-cleanup-v3 Web-Diff: https://github.com/benpeart/git/commit/393007340d Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v1 && git checkout 393007340d t/t7519-status-fsmonitor.sh | 7 --- 1 file changed, 7 deletions(-) diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 8308d6d5b1..3f0dd98010 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -4,13 +4,6 @@ test_description='git status with file system watcher' . ./test-lib.sh -# -# To run the entire git test suite using fsmonitor: -# -# copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. -# - # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' # "git update-index --fsmonitor" can be used to get the extension written # before testing the results. base-commit: 043246d9369fb851c5c2b922466f77fc7ef0327b -- 2.18.0.windows.1
Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/28/2018 6:01 AM, SZEDER Gábor wrote: On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote: diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we are good to go. diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. But this old comment is different, suggesting copying that script to our $PATH. I prefer your instructions above, because it's only a single step, and, more importantly, it won't pollute my $PATH. I think this comment should be updated to make the advices in both places consistent. Or perhaps even removed, now that all GIT_TEST variables are documented in the same place? I prefer the suggestion to simply remove this text from the test script now that there is documentation for it in the t/README file.
Re: [PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
On Tue, Sep 18, 2018 at 11:29:35PM +, Ben Peart wrote: > diff --git a/t/README b/t/README > index 56a417439c..47165f7eab 100644 > --- a/t/README > +++ b/t/README > @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon > pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor > +code path for utilizing a file system monitor to speed up detecting > +new or changed files. Here you tell us to set GIT_TEST_FSMONITOR to an absolute path, and we are good to go. > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh > index 756beb0d8e..d77012ea6d 100755 > --- a/t/t7519-status-fsmonitor.sh > +++ b/t/t7519-status-fsmonitor.sh > @@ -8,7 +8,7 @@ test_description='git status with file system watcher' > # To run the entire git test suite using fsmonitor: > # > # copy t/t7519/fsmonitor-all to a location in your path and then set > -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. > +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. But this old comment is different, suggesting copying that script to our $PATH. I prefer your instructions above, because it's only a single step, and, more importantly, it won't pollute my $PATH. I think this comment should be updated to make the advices in both places consistent. Or perhaps even removed, now that all GIT_TEST variables are documented in the same place?
Re: [PATCH 1/1] read-cache: update index format default to v4
On Tue, Sep 25, 2018 at 8:01 PM Stefan Beller wrote: > > On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee wrote: > > > > On 9/25/2018 3:06 AM, Patrick Steinhardt wrote: > > > On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote: > > >> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via > > >> GitGitGadget wrote: > > >>> From: Derrick Stolee > > >>> > > >>> The index v4 format has been available since 2012 with 9d22778 > > >>> "reach-cache.c: write prefix-compressed names in the index". Since > > >>> the format has been stable for so long, almost all versions of Git > > >>> in use today understand version 4, removing one barrier to upgrade > > >>> -- that someone may want to downgrade and needs a working repo. > > >> What about alternative implementations, like JGit, libgit2, etc.? > > > Speaking of libgit2, we are able to read and write index v4 since > > > commit c1b370e93 > > > > This is a good point, Szeder. > > > > Patrick: I'm glad LibGit2 is up-to-date with index formats. > > > > Unfortunately, taking a look (for the first time) at the JGit code > > reveals that they don't appear to have v4 support. In > > org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the > > DirCache.readFrom() method: lines 488-494, I see the following snippet: > > > > final int ver = NB.decodeInt32(hdr, 4); > > boolean extended = false; > > if (ver == 3) > > extended = true; > > else if (ver != 2) > > throw new > > CorruptObjectException(MessageFormat.format( > > JGitText.get().unknownDIRCVersion, Integer.valueOf(ver))); > > > > It looks like this will immediately throw with versions other than 2 or 3. > > > > I'm adding Jonathan Nieder to CC so he can check with JGit people about > > the impact of this change. > > JGit is used both on the server (which doesn't use index/staging area) > as well as client side as e.g. an Eclipse integration, which would > very much like to use the index. > > Adding Matthias Sohn as well, who is active in JGit and cares > more about the client side than Googlers who only make use > of the server side part of JGit. thanks for the heads up, in fact JGit does not yet support index version 4. I will look into this. -Matthias
[PATCH v9 04/21] stash: update test cases conform to coding guidelines
Removed whitespaces after redirection operators. Signed-off-by: Paul-Sebastian Ungureanu --- t/t3903-stash.sh | 120 --- 1 file changed, 61 insertions(+), 59 deletions(-) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index af7586d43d..de6cab1fe7 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -8,22 +8,22 @@ test_description='Test git stash' . ./test-lib.sh test_expect_success 'stash some dirty working directory' ' - echo 1 > file && + echo 1 >file && git add file && echo unrelated >other-file && git add other-file && test_tick && git commit -m initial && - echo 2 > file && + echo 2 >file && git add file && - echo 3 > file && + echo 3 >file && test_tick && git stash && git diff-files --quiet && git diff-index --cached --quiet HEAD ' -cat > expect << EOF +cat >expect < output && + git diff stash^2..stash >output && test_cmp output expect ' @@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' ' test_expect_success 'apply stashed changes (including index)' ' git reset --hard HEAD^ && - echo 6 > other-file && + echo 6 >other-file && git add other-file && test_tick && git commit -m other-file && @@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra options' ' test_expect_success 'drop top stash' ' git reset --hard && - git stash list > stashlist1 && - echo 7 > file && + git stash list >expected && + echo 7 >file && git stash && git stash drop && - git stash list > stashlist2 && - test_cmp stashlist1 stashlist2 && + git stash list >actual && + test_cmp expected actual && git stash apply && test 3 = $(cat file) && test 1 = $(git show :file) && @@ -113,9 +113,9 @@ test_expect_success 'drop top stash' ' test_expect_success 'drop middle stash' ' git reset --hard && - echo 8 > file && + echo 8 >file && git stash && - echo 9 > file && + echo 9 >file && git stash && git stash drop stash@{1} && test 2 = $(git stash list | wc -l) && @@ -160,7 +160,7 @@ test_expect_success 'stash pop' ' test 0 = $(git stash list | wc -l) ' -cat > expect << EOF +cat >expect < expect1 << EOF +cat >expect1 < expect2 << EOF +cat >expect2 < file && + echo foo >file && git commit file -m first && - echo bar > file && - echo bar2 > file2 && + echo bar >file && + echo bar2 >file2 && git add file2 && git stash && - echo baz > file && + echo baz >file && git commit file -m second && git stash branch stashbranch && test refs/heads/stashbranch = $(git symbolic-ref HEAD) && test $(git rev-parse HEAD) = $(git rev-parse master^) && - git diff --cached > output && + git diff --cached >output && test_cmp output expect && - git diff > output && + git diff >output && test_cmp output expect1 && git add file && git commit -m alternate\ second && - git diff master..stashbranch > output && + git diff master..stashbranch >output && test_cmp output expect2 && test 0 = $(git stash list | wc -l) ' test_expect_success 'apply -q is quiet' ' - echo foo > file && + echo foo >file && git stash && - git stash apply -q > output.out 2>&1 && + git stash apply -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'save -q is quiet' ' - git stash save --quiet > output.out 2>&1 && + git stash save --quiet >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'pop -q is quiet' ' - git stash pop -q > output.out 2>&1 && + git stash pop -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'pop -q --index works and is quiet' ' - echo foo > file && + echo foo >file && git add file && git stash save --quiet && - git stash pop -q --index > output.out 2>&1 && + git stash pop -q --index >output.out 2>&1 && test foo = "$(git show :file)" && test_must_be_empty output.out ' test_expect_success 'drop -q is quiet' ' git stash && - git stash drop -q > output.out 2>&1 && + git stash drop -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'stash -k' ' - echo bar3 > file && - echo bar4 > file2 && + echo bar3 >file && + echo bar4 >file2 && git add file2 && git stash -k && test bar,bar4 = $(cat file),$(cat file2) ' test_expect_success 'stash --no-keep-index' ' - echo bar33 > file && - echo bar44 > file2 && + echo bar33 >file && + echo
[GSoC][PATCH v9 03/21] stash: update test cases conform to coding guidelines
Removed whitespaces after redirection operators. Signed-off-by: Paul-Sebastian Ungureanu --- t/t3903-stash.sh | 120 --- 1 file changed, 61 insertions(+), 59 deletions(-) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index af7586d43d..de6cab1fe7 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -8,22 +8,22 @@ test_description='Test git stash' . ./test-lib.sh test_expect_success 'stash some dirty working directory' ' - echo 1 > file && + echo 1 >file && git add file && echo unrelated >other-file && git add other-file && test_tick && git commit -m initial && - echo 2 > file && + echo 2 >file && git add file && - echo 3 > file && + echo 3 >file && test_tick && git stash && git diff-files --quiet && git diff-index --cached --quiet HEAD ' -cat > expect << EOF +cat >expect < output && + git diff stash^2..stash >output && test_cmp output expect ' @@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' ' test_expect_success 'apply stashed changes (including index)' ' git reset --hard HEAD^ && - echo 6 > other-file && + echo 6 >other-file && git add other-file && test_tick && git commit -m other-file && @@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra options' ' test_expect_success 'drop top stash' ' git reset --hard && - git stash list > stashlist1 && - echo 7 > file && + git stash list >expected && + echo 7 >file && git stash && git stash drop && - git stash list > stashlist2 && - test_cmp stashlist1 stashlist2 && + git stash list >actual && + test_cmp expected actual && git stash apply && test 3 = $(cat file) && test 1 = $(git show :file) && @@ -113,9 +113,9 @@ test_expect_success 'drop top stash' ' test_expect_success 'drop middle stash' ' git reset --hard && - echo 8 > file && + echo 8 >file && git stash && - echo 9 > file && + echo 9 >file && git stash && git stash drop stash@{1} && test 2 = $(git stash list | wc -l) && @@ -160,7 +160,7 @@ test_expect_success 'stash pop' ' test 0 = $(git stash list | wc -l) ' -cat > expect << EOF +cat >expect < expect1 << EOF +cat >expect1 < expect2 << EOF +cat >expect2 < file && + echo foo >file && git commit file -m first && - echo bar > file && - echo bar2 > file2 && + echo bar >file && + echo bar2 >file2 && git add file2 && git stash && - echo baz > file && + echo baz >file && git commit file -m second && git stash branch stashbranch && test refs/heads/stashbranch = $(git symbolic-ref HEAD) && test $(git rev-parse HEAD) = $(git rev-parse master^) && - git diff --cached > output && + git diff --cached >output && test_cmp output expect && - git diff > output && + git diff >output && test_cmp output expect1 && git add file && git commit -m alternate\ second && - git diff master..stashbranch > output && + git diff master..stashbranch >output && test_cmp output expect2 && test 0 = $(git stash list | wc -l) ' test_expect_success 'apply -q is quiet' ' - echo foo > file && + echo foo >file && git stash && - git stash apply -q > output.out 2>&1 && + git stash apply -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'save -q is quiet' ' - git stash save --quiet > output.out 2>&1 && + git stash save --quiet >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'pop -q is quiet' ' - git stash pop -q > output.out 2>&1 && + git stash pop -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'pop -q --index works and is quiet' ' - echo foo > file && + echo foo >file && git add file && git stash save --quiet && - git stash pop -q --index > output.out 2>&1 && + git stash pop -q --index >output.out 2>&1 && test foo = "$(git show :file)" && test_must_be_empty output.out ' test_expect_success 'drop -q is quiet' ' git stash && - git stash drop -q > output.out 2>&1 && + git stash drop -q >output.out 2>&1 && test_must_be_empty output.out ' test_expect_success 'stash -k' ' - echo bar3 > file && - echo bar4 > file2 && + echo bar3 >file && + echo bar4 >file2 && git add file2 && git stash -k && test bar,bar4 = $(cat file),$(cat file2) ' test_expect_success 'stash --no-keep-index' ' - echo bar33 > file && - echo bar44 > file2 && + echo bar33 >file && + echo
Re: [PATCH 1/1] read-cache: update index format default to v4
On 9/25/2018 2:01 PM, Stefan Beller wrote: On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee wrote: On 9/25/2018 3:06 AM, Patrick Steinhardt wrote: On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote: On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The index v4 format has been available since 2012 with 9d22778 "reach-cache.c: write prefix-compressed names in the index". Since the format has been stable for so long, almost all versions of Git in use today understand version 4, removing one barrier to upgrade -- that someone may want to downgrade and needs a working repo. What about alternative implementations, like JGit, libgit2, etc.? Speaking of libgit2, we are able to read and write index v4 since commit c1b370e93 This is a good point, Szeder. Patrick: I'm glad LibGit2 is up-to-date with index formats. Unfortunately, taking a look (for the first time) at the JGit code reveals that they don't appear to have v4 support. In org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the DirCache.readFrom() method: lines 488-494, I see the following snippet: final int ver = NB.decodeInt32(hdr, 4); boolean extended = false; if (ver == 3) extended = true; else if (ver != 2) throw new CorruptObjectException(MessageFormat.format( JGitText.get().unknownDIRCVersion, Integer.valueOf(ver))); It looks like this will immediately throw with versions other than 2 or 3. I'm adding Jonathan Nieder to CC so he can check with JGit people about the impact of this change. JGit is used both on the server (which doesn't use index/staging area) as well as client side as e.g. an Eclipse integration, which would very much like to use the index. Adding Matthias Sohn as well, who is active in JGit and cares more about the client side than Googlers who only make use of the server side part of JGit. Stefan In addition to the compatibility with other implementations of git question, I think we should include Duy's patch [1] to "optimize reading index format v4" before flipping the default to V4. In my testing, this reduced the index load time of V4 by 10%-15% making the CPU cost only ~2% more expensive than V2 or V3 indexes. This increase in CPU cost is more than offset by the significant reduction in file IO due to the reduced index file size. [1] https://public-inbox.org/git/20180825064458.28484-1-pclo...@gmail.com/
Re: [PATCH 1/1] read-cache: update index format default to v4
On Tue, Sep 25, 2018 at 7:30 AM Derrick Stolee wrote: > > On 9/25/2018 3:06 AM, Patrick Steinhardt wrote: > > On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote: > >> On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget > >> wrote: > >>> From: Derrick Stolee > >>> > >>> The index v4 format has been available since 2012 with 9d22778 > >>> "reach-cache.c: write prefix-compressed names in the index". Since > >>> the format has been stable for so long, almost all versions of Git > >>> in use today understand version 4, removing one barrier to upgrade > >>> -- that someone may want to downgrade and needs a working repo. > >> What about alternative implementations, like JGit, libgit2, etc.? > > Speaking of libgit2, we are able to read and write index v4 since > > commit c1b370e93 > > This is a good point, Szeder. > > Patrick: I'm glad LibGit2 is up-to-date with index formats. > > Unfortunately, taking a look (for the first time) at the JGit code > reveals that they don't appear to have v4 support. In > org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the > DirCache.readFrom() method: lines 488-494, I see the following snippet: > > final int ver = NB.decodeInt32(hdr, 4); > boolean extended = false; > if (ver == 3) > extended = true; > else if (ver != 2) > throw new > CorruptObjectException(MessageFormat.format( > JGitText.get().unknownDIRCVersion, Integer.valueOf(ver))); > > It looks like this will immediately throw with versions other than 2 or 3. > > I'm adding Jonathan Nieder to CC so he can check with JGit people about > the impact of this change. JGit is used both on the server (which doesn't use index/staging area) as well as client side as e.g. an Eclipse integration, which would very much like to use the index. Adding Matthias Sohn as well, who is active in JGit and cares more about the client side than Googlers who only make use of the server side part of JGit. Stefan
Re: [PATCH 1/1] read-cache: update index format default to v4
On 9/25/2018 3:06 AM, Patrick Steinhardt wrote: On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote: On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote: From: Derrick Stolee The index v4 format has been available since 2012 with 9d22778 "reach-cache.c: write prefix-compressed names in the index". Since the format has been stable for so long, almost all versions of Git in use today understand version 4, removing one barrier to upgrade -- that someone may want to downgrade and needs a working repo. What about alternative implementations, like JGit, libgit2, etc.? Speaking of libgit2, we are able to read and write index v4 since commit c1b370e93 This is a good point, Szeder. Patrick: I'm glad LibGit2 is up-to-date with index formats. Unfortunately, taking a look (for the first time) at the JGit code reveals that they don't appear to have v4 support. In org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java, the DirCache.readFrom() method: lines 488-494, I see the following snippet: final int ver = NB.decodeInt32(hdr, 4); boolean extended = false; if (ver == 3) extended = true; else if (ver != 2) throw new CorruptObjectException(MessageFormat.format( JGitText.get().unknownDIRCVersion, Integer.valueOf(ver))); It looks like this will immediately throw with versions other than 2 or 3. I'm adding Jonathan Nieder to CC so he can check with JGit people about the impact of this change. Thanks, -Stolee
Re: [PATCH 1/1] read-cache: update index format default to v4
On Mon, Sep 24, 2018 at 11:32:23PM +0200, SZEDER Gábor wrote: > On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget > wrote: > > From: Derrick Stolee > > > > The index v4 format has been available since 2012 with 9d22778 > > "reach-cache.c: write prefix-compressed names in the index". Since > > the format has been stable for so long, almost all versions of Git > > in use today understand version 4, removing one barrier to upgrade > > -- that someone may want to downgrade and needs a working repo. > > What about alternative implementations, like JGit, libgit2, etc.? Speaking of libgit2, we are able to read and write index v4 since commit c1b370e93 (Merge pull request #3837 from novalis/dturner/indexv4, 2016-08-17), released with v0.25 in December 2016. Due to insufficient tests, our read support was initially broken, which got fixed with commit 3bc95cfe3 (Merge pull request #4236 from pks-t/pks/index-v4-fixes, 2017-06-07) and released with v0.26 in June 2017. Right now I'm not aware of additional bugs in our index v4 support, so we'd be fine with changing the default. Patrick > > Despite being stable for a long time, this index version was never > > adopted as the default. This prefix-compressed version of the format > > can get significant space savings on repos with large working > > directories (which naturally tend to have deep nesting). This version > > is set as the default for some external tools, such as VFS for Git. > > Because of this external use, the format has had a lot of "testing in > > production" and also is subject to continuous integration in these > > environments. > > > > Previously, to test version 4 indexes, we needed to run the test > > suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4). > > > > One potential, but short-term, downside is that we lose coverage of > > the version 3 indexes. The trade-off is that we may want to cover > > that version using GIT_TEST_INDEX_VERSION=3. > > > > Signed-off-by: Derrick Stolee > > --- > > read-cache.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/read-cache.c b/read-cache.c > > index 372588260e..af6c8f2a67 100644 > > --- a/read-cache.c > > +++ b/read-cache.c > > @@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct > > cache_entry *ce, > > * Index File I/O > > */ > > > > -#define INDEX_FORMAT_DEFAULT 3 > > +#define INDEX_FORMAT_DEFAULT 4 > > > > static unsigned int get_index_format_default(void) > > { > > -- > > gitgitgadget signature.asc Description: PGP signature
Re: [PATCH 1/1] read-cache: update index format default to v4
SZEDER Gábor writes: > On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget > wrote: >> From: Derrick Stolee >> >> The index v4 format has been available since 2012 with 9d22778 >> "reach-cache.c: write prefix-compressed names in the index". Since >> the format has been stable for so long, almost all versions of Git >> in use today understand version 4, removing one barrier to upgrade >> -- that someone may want to downgrade and needs a working repo. > > What about alternative implementations, like JGit, libgit2, etc.? Good question. Because the index-version of an index file is designed to be sticky, repos that need to be accessed by other implementations can keep whatever current version. A new repo that need to be accessed by them can be (forcibly) written in v2 and keep its v2ness, I would think. And that would serve as an incentive for the implementations to catch up ;-)
Re: [PATCH 1/1] read-cache: update index format default to v4
On Mon, Sep 24, 2018 at 02:15:30PM -0700, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee > > The index v4 format has been available since 2012 with 9d22778 > "reach-cache.c: write prefix-compressed names in the index". Since > the format has been stable for so long, almost all versions of Git > in use today understand version 4, removing one barrier to upgrade > -- that someone may want to downgrade and needs a working repo. What about alternative implementations, like JGit, libgit2, etc.? > Despite being stable for a long time, this index version was never > adopted as the default. This prefix-compressed version of the format > can get significant space savings on repos with large working > directories (which naturally tend to have deep nesting). This version > is set as the default for some external tools, such as VFS for Git. > Because of this external use, the format has had a lot of "testing in > production" and also is subject to continuous integration in these > environments. > > Previously, to test version 4 indexes, we needed to run the test > suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4). > > One potential, but short-term, downside is that we lose coverage of > the version 3 indexes. The trade-off is that we may want to cover > that version using GIT_TEST_INDEX_VERSION=3. > > Signed-off-by: Derrick Stolee > --- > read-cache.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/read-cache.c b/read-cache.c > index 372588260e..af6c8f2a67 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct > cache_entry *ce, > * Index File I/O > */ > > -#define INDEX_FORMAT_DEFAULT 3 > +#define INDEX_FORMAT_DEFAULT 4 > > static unsigned int get_index_format_default(void) > { > -- > gitgitgadget
[PATCH 1/1] read-cache: update index format default to v4
From: Derrick Stolee The index v4 format has been available since 2012 with 9d22778 "reach-cache.c: write prefix-compressed names in the index". Since the format has been stable for so long, almost all versions of Git in use today understand version 4, removing one barrier to upgrade -- that someone may want to downgrade and needs a working repo. Despite being stable for a long time, this index version was never adopted as the default. This prefix-compressed version of the format can get significant space savings on repos with large working directories (which naturally tend to have deep nesting). This version is set as the default for some external tools, such as VFS for Git. Because of this external use, the format has had a lot of "testing in production" and also is subject to continuous integration in these environments. Previously, to test version 4 indexes, we needed to run the test suite with GIT_TEST_INDEX_VERSION=4 (or TEST_GIT_INDEX_VERSION=4). One potential, but short-term, downside is that we lose coverage of the version 3 indexes. The trade-off is that we may want to cover that version using GIT_TEST_INDEX_VERSION=3. Signed-off-by: Derrick Stolee --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index 372588260e..af6c8f2a67 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1484,7 +1484,7 @@ struct cache_entry *refresh_cache_entry(struct cache_entry *ce, * Index File I/O */ -#define INDEX_FORMAT_DEFAULT 3 +#define INDEX_FORMAT_DEFAULT 4 static unsigned int get_index_format_default(void) { -- gitgitgadget
[PATCH 0/1] read-cache: update index format default to v4
After discussing this with several people off-list, I thought I would open the question up to the list: Should we update the default index version to v4? The .git/index file stores the list of every path tracked by Git in the working directory, including merge information, staged paths, and information about the file system contents (based on modified time). The only major update in v4 is that the paths are prefix-compressed. This compression works best in repos with a lot of paths, especially deep paths. For this reason, we set the index to v4 in VFS for Git. Among VFS for Git contributors, we were talking about how the v4 format is not covered by the test suite by default. We are working to increase the number of CI builds that set extra GIT_TEST_* variables that we need. However, I thought it worth having a discussion of whether this is a good thing to recommend for all users of Git. Personally, I'm not an expert here, but I am happy to start the conversation. Thanks, -Stolee Derrick Stolee (1): read-cache: update index format default to v4 read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 53f9a3e157dbbc901a02ac2c73346d375e24978c Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-41%2Fderrickstolee%2Findex-v4-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-41/derrickstolee/index-v4-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/41 -- gitgitgadget
Re: [PATCH] receive-pack: update comment with check_everything_connected
Jeff King writes: > That function is now called "check_connected()", but we forgot to update > this comment in 7043c7071c (check_everything_connected: use a struct > with named options, 2016-07-15). > > Signed-off-by: Jeff King > --- > Just a minor annoyance I happened to notice while discussing in another > thread. I notice both of us still called it check-everything-connected > in our emails; old habits die hard, I suppose. ;) Yup, and now I think I caught up ;-) Thanks. > > builtin/receive-pack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > index a3bb13af10..3b7432c8e4 100644 > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c > @@ -1834,7 +1834,7 @@ static void prepare_shallow_update(struct command > *commands, > /* >* keep hooks happy by forcing a temporary shallow file via >* env variable because we can't add --shallow-file to every > - * command. check_everything_connected() will be done with > + * command. check_connected() will be done with >* true .git/shallow though. >*/ > setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);
[PATCH] receive-pack: update comment with check_everything_connected
That function is now called "check_connected()", but we forgot to update this comment in 7043c7071c (check_everything_connected: use a struct with named options, 2016-07-15). Signed-off-by: Jeff King --- Just a minor annoyance I happened to notice while discussing in another thread. I notice both of us still called it check-everything-connected in our emails; old habits die hard, I suppose. ;) builtin/receive-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index a3bb13af10..3b7432c8e4 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1834,7 +1834,7 @@ static void prepare_shallow_update(struct command *commands, /* * keep hooks happy by forcing a temporary shallow file via * env variable because we can't add --shallow-file to every -* command. check_everything_connected() will be done with +* command. check_connected() will be done with * true .git/shallow though. */ setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1); -- 2.19.0.762.gbd1f43ccbf
[PATCH v3 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- preload-index.c | 2 +- t/README| 3 +++ t/t7519-status-fsmonitor.sh | 4 ++-- t/test-lib.sh | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/preload-index.c b/preload-index.c index 0a4e2933bb..a850e197c2 100644 --- a/preload-index.c +++ b/preload-index.c @@ -85,7 +85,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_FORCE_PRELOAD_TEST", 0)) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_TEST_PRELOAD_INDEX", 0)) threads = 2; if (threads < 2) return; diff --git a/t/README b/t/README index 9b13f6d12e..5670c7aad0 100644 --- a/t/README +++ b/t/README @@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write code path for the index version specified. Can be set to any valid version (currently 2, 3, or 4). +GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path +by overriding the minimum number of cache entries required per thread. + Naming Tests diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index d77012ea6d..8308d6d5b1 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -245,9 +245,9 @@ do git config core.preloadIndex $preload_val && if test $preload_val = true then - GIT_FORCE_PRELOAD_TEST=$preload_val; export GIT_FORCE_PRELOAD_TEST + GIT_TEST_PRELOAD_INDEX=$preload_val; export GIT_TEST_PRELOAD_INDEX else - unset GIT_FORCE_PRELOAD_TEST + sane_unset GIT_TEST_PRELOAD_INDEX fi ' diff --git a/t/test-lib.sh b/t/test-lib.sh index e80c84d13c..8ef86e05a3 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -154,6 +154,7 @@ check_var_migration () { check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION +check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX # Use specific version of the index file format if test -n "${GIT_TEST_INDEX_VERSION:+isset}" -- 2.18.0.windows.1
[PATCH v3 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- config.c| 2 +- t/README| 4 t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 2 +- t/test-lib.sh | 20 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index b3b4d83eaf..f6a856f24c 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -6,7 +6,7 @@ test_description='split index mode tests' # We need total control of index splitting here sane_unset GIT_TEST_SPLIT_INDEX -sane_unset GIT_FSMONITOR_TEST +sane_unset GIT_TEST_FSMONITOR test_expect_success 'enable split index' ' git config splitIndex.maxPercentChange 100 && diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. # # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..653688c067 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -140,6 +140,26 @@ then export GIT_INDEX_VERSION fi +check_var_migration () { + old_name=$1 new_name=$2 + eval "old_isset=\${${old_name}:+isset}" + eval "new_isset=\${${new_name}:+isset}" + case "$old_isset,$new_isset" in + isset,) + echo >&2 "warning: $old_name is now $new_name" + echo >&2 "hint: set $new_name too during the transition period" + eval "$new_name=\$$old_name" + ;; + isset,isset) + # do this later + # echo >&2 "warning: $old_name is now $new_name" + # echo >&2 "hint: remove $old_name" + ;; + esac +} + +check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null || -- 2.18.0.windows.1
[PATCH v3 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- Makefile | 6 +++--- t/README | 4 t/test-lib.sh | 14 -- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/README b/t/README index 47165f7eab..9b13f6d12e 100644 --- a/t/README +++ b/t/README @@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor code path for utilizing a file system monitor to speed up detecting new or changed files. +GIT_TEST_INDEX_VERSION= exercises the index read/write code path +for the index version specified. Can be set to any valid version +(currently 2, 3, or 4). + Naming Tests diff --git a/t/test-lib.sh b/t/test-lib.sh index 653688c067..e80c84d13c 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,12 +134,6 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" -then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" - export GIT_INDEX_VERSION -fi - check_var_migration () { old_name=$1 new_name=$2 eval "old_isset=\${${old_name}:+isset}" @@ -159,6 +153,14 @@ check_var_migration () { } check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION + +# Use specific version of the index file format +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" +then + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" + export GIT_INDEX_VERSION +fi # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind -- 2.18.0.windows.1
Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Junio C Hamano writes: > Ben Peart writes: > >> diff --git a/t/test-lib.sh b/t/test-lib.sh >> index 653688c067..397eb71578 100644 >> --- a/t/test-lib.sh >> +++ b/t/test-lib.sh >> @@ -134,9 +134,9 @@ export EDITOR >> GIT_TRACE_BARE=1 >> export GIT_TRACE_BARE >> >> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" >> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" >> then >> -GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" >> +GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" >> export GIT_INDEX_VERSION >> fi > > Is this done a bit before ... > >> @@ -159,6 +159,7 @@ check_var_migration () { >> } >> >> check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR >> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION > > ... this has a chance to kick in to say things like "Whoa you have > TEST_GIT_INDEX_VERSION that is an old spelling of > GIT_TEST_INDEX_VERSION", isn't it? So, the obvious fix would look like the patch below. One problem with warning is that $ TEST_GIT_INDEX_VERSION=4 sh ./t-basic.sh (or any other depreated variable set without its modern counterpart set) would fail due to extra output produced to the standard error stream. t/test-lib.sh | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 17a56f44ad..8ef86e05a3 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,12 +134,6 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${GIT_TEST_INDEX_VERSION:+isset}" -then - GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" - export GIT_INDEX_VERSION -fi - check_var_migration () { old_name=$1 new_name=$2 eval "old_isset=\${${old_name}:+isset}" @@ -162,6 +156,13 @@ check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX +# Use specific version of the index file format +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" +then + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" + export GIT_INDEX_VERSION +fi + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Ben Peart writes: > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 653688c067..397eb71578 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -134,9 +134,9 @@ export EDITOR > GIT_TRACE_BARE=1 > export GIT_TRACE_BARE > > -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" > +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" > then > - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" > + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" > export GIT_INDEX_VERSION > fi Is this done a bit before ... > @@ -159,6 +159,7 @@ check_var_migration () { > } > > check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR > +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION ... this has a chance to kick in to say things like "Whoa you have TEST_GIT_INDEX_VERSION that is an old spelling of GIT_TEST_INDEX_VERSION", isn't it? > # Add libc MALLOC and MALLOC_PERTURB test > # only if we are not executing the test with valgrind
[PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- Makefile | 6 +++--- t/README | 4 t/test-lib.sh | 5 +++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/README b/t/README index 47165f7eab..9b13f6d12e 100644 --- a/t/README +++ b/t/README @@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor code path for utilizing a file system monitor to speed up detecting new or changed files. +GIT_TEST_INDEX_VERSION= exercises the index read/write code path +for the index version specified. Can be set to any valid version +(currently 2, 3, or 4). + Naming Tests diff --git a/t/test-lib.sh b/t/test-lib.sh index 653688c067..397eb71578 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,9 +134,9 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi @@ -159,6 +159,7 @@ check_var_migration () { } check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind -- 2.18.0.windows.1
[PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- config.c| 2 +- t/README| 4 t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 2 +- t/test-lib.sh | 20 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index b3b4d83eaf..f6a856f24c 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -6,7 +6,7 @@ test_description='split index mode tests' # We need total control of index splitting here sane_unset GIT_TEST_SPLIT_INDEX -sane_unset GIT_FSMONITOR_TEST +sane_unset GIT_TEST_FSMONITOR test_expect_success 'enable split index' ' git config splitIndex.maxPercentChange 100 && diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. # # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..653688c067 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -140,6 +140,26 @@ then export GIT_INDEX_VERSION fi +check_var_migration () { + old_name=$1 new_name=$2 + eval "old_isset=\${${old_name}:+isset}" + eval "new_isset=\${${new_name}:+isset}" + case "$old_isset,$new_isset" in + isset,) + echo >&2 "warning: $old_name is now $new_name" + echo >&2 "hint: set $new_name too during the transition period" + eval "$new_name=\$$old_name" + ;; + isset,isset) + # do this later + # echo >&2 "warning: $old_name is now $new_name" + # echo >&2 "hint: remove $old_name" + ;; + esac +} + +check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null || -- 2.18.0.windows.1
[PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give a warning when the old variable is set to let people know they need to update their environment to use the new variable. Signed-off-by: Ben Peart --- preload-index.c | 2 +- t/README| 3 +++ t/t7519-status-fsmonitor.sh | 4 ++-- t/test-lib.sh | 1 + 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/preload-index.c b/preload-index.c index 0a4e2933bb..a850e197c2 100644 --- a/preload-index.c +++ b/preload-index.c @@ -85,7 +85,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_FORCE_PRELOAD_TEST", 0)) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_TEST_PRELOAD_INDEX", 0)) threads = 2; if (threads < 2) return; diff --git a/t/README b/t/README index 9b13f6d12e..5670c7aad0 100644 --- a/t/README +++ b/t/README @@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write code path for the index version specified. Can be set to any valid version (currently 2, 3, or 4). +GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path +by overriding the minimum number of cache entries required per thread. + Naming Tests diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index d77012ea6d..8308d6d5b1 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -245,9 +245,9 @@ do git config core.preloadIndex $preload_val && if test $preload_val = true then - GIT_FORCE_PRELOAD_TEST=$preload_val; export GIT_FORCE_PRELOAD_TEST + GIT_TEST_PRELOAD_INDEX=$preload_val; export GIT_TEST_PRELOAD_INDEX else - unset GIT_FORCE_PRELOAD_TEST + sane_unset GIT_TEST_PRELOAD_INDEX fi ' diff --git a/t/test-lib.sh b/t/test-lib.sh index 397eb71578..17a56f44ad 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -160,6 +160,7 @@ check_var_migration () { check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION +check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind -- 2.18.0.windows.1
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > The difference here is that core.fsmonitor isn't a boolean value. It > is a string to a command that is executed so it can't be moved over to > get_env_bool(). Ah, of course ;-) Then please take the following as a review comment for 4/4; checking if each getenv(VAR) should or should not become git_env_bool() and updating them should be done as a separate change for variables whether they are being renamed or not in this series. >> I _think_ the renaming should be done without getting mixed with >> other changes like the git_env_bool() done in 4/4. The idea to use >> git_env_bool() in stead of getenv() may be a good one, but then we >> should consistently do so when appropriate, and that would make a >> fine theme for another topic. >>
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/14/2018 1:15 PM, Junio C Hamano wrote: Ben Peart writes: diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); Sorry for not noticing earlier, but unlike 4/4 that changed getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to getenv(VAR), meaning "if it is set to any non-empty string, it is true". Is there a reason for this discrepancy? The difference here is that core.fsmonitor isn't a boolean value. It is a string to a command that is executed so it can't be moved over to get_env_bool(). I _think_ the renaming should be done without getting mixed with other changes like the git_env_bool() done in 4/4. The idea to use git_env_bool() in stead of getenv() may be a good one, but then we should consistently do so when appropriate, and that would make a fine theme for another topic.
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > diff --git a/config.c b/config.c > index 3461993f0a..3555c63f28 100644 > --- a/config.c > +++ b/config.c > @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) > int git_config_get_fsmonitor(void) > { > if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) > - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); > + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); Sorry for not noticing earlier, but unlike 4/4 that changed getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to getenv(VAR), meaning "if it is set to any non-empty string, it is true". Is there a reason for this discrepancy? I _think_ the renaming should be done without getting mixed with other changes like the git_env_bool() done in 4/4. The idea to use git_env_bool() in stead of getenv() may be a good one, but then we should consistently do so when appropriate, and that would make a fine theme for another topic.
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Junio C Hamano writes: > Ben Peart writes: > >> +if test -n "$GIT_FSMONITOR_TEST" >> +then >> +if test -n "$GIT_TEST_FSMONITOR" >> +then >> +echo "warning: the GIT_FSMONITOR_TEST variable has been renamed >> to GIT_TEST_FSMONITOR" >> +else >> +echo "error: the GIT_FSMONITOR_TEST variable has been renamed >> to GIT_TEST_FSMONITOR" >> +exit 1 >> +fi >> +fi > > I would have expected that, because we are now doing multiple pairs > of variables in a single series, we would add a helper function that > can be called like so: > > check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR > > in the earliest step. Perhaps something like this. > > check_var_migration () { > old_name=$1 new_name=$2 > eval "old_isset=\${${old_name}:+isset}" > eval "new_isset=\${${new_name}:+isset}" > case "$old_isset,$new_isset" in > isset,) > echo >&2 "error: $old_name is now $new_name" > exit 1 ;; > isset,isset) > # enable this, once $old_name no longer is valid anywhere > # echo >&2 "warning: $old_name is now $new_name" > # echo >&2 "hint: remove $old_name" > ;; > esac > } Alternatively, we could do this, to warn and then migrate the value given to the old variable automatically to the new variable and let the test proceed. check_var_migration () { old_name=$1 new_name=$2 eval "old_isset=\${${old_name}:+isset}" eval "new_isset=\${${new_name}:+isset}" case "$old_isset,$new_isset" in isset,) echo >&2 "warning: $old_name is now $new_name" echo >&2 "hint: set $new_name too during the transition period" eval "$new_name=\$$old_name" ;; isset,isset) # do this later # echo >&2 "warning: $old_name is now $new_name" # echo >&2 "hint: remove $old_name" ;; esac }
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > +if test -n "$GIT_FSMONITOR_TEST" > +then > + if test -n "$GIT_TEST_FSMONITOR" > + then > + echo "warning: the GIT_FSMONITOR_TEST variable has been renamed > to GIT_TEST_FSMONITOR" > + else > + echo "error: the GIT_FSMONITOR_TEST variable has been renamed > to GIT_TEST_FSMONITOR" > + exit 1 > + fi > +fi I would have expected that, because we are now doing multiple pairs of variables in a single series, we would add a helper function that can be called like so: check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR in the earliest step. Perhaps something like this. check_var_migration () { old_name=$1 new_name=$2 eval "old_isset=\${${old_name}:+isset}" eval "new_isset=\${${new_name}:+isset}" case "$old_isset,$new_isset" in isset,) echo >&2 "error: $old_name is now $new_name" exit 1 ;; isset,isset) # enable this, once $old_name no longer is valid anywhere # echo >&2 "warning: $old_name is now $new_name" # echo >&2 "hint: remove $old_name" ;; esac }
[PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give an error when the old variable is set to let people know they need to update their environment to use the new variable. If the new variable is also set, just give a warning so they can eventually remove the old variable. Signed-off-by: Ben Peart --- config.c| 2 +- t/README| 4 t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 2 +- t/test-lib.sh | 11 +++ 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 56a417439c..47165f7eab 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index b3b4d83eaf..f6a856f24c 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -6,7 +6,7 @@ test_description='split index mode tests' # We need total control of index splitting here sane_unset GIT_TEST_SPLIT_INDEX -sane_unset GIT_FSMONITOR_TEST +sane_unset GIT_TEST_FSMONITOR test_expect_success 'enable split index' ' git config splitIndex.maxPercentChange 100 && diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. # # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..0ef111d808 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -140,6 +140,17 @@ then export GIT_INDEX_VERSION fi +if test -n "$GIT_FSMONITOR_TEST" +then + if test -n "$GIT_TEST_FSMONITOR" + then + echo "warning: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR" + else + echo "error: the GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR" + exit 1 + fi +fi + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null || -- 2.18.0.windows.1
[PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give an error when the old variable is set to let people know they need to update their environment to use the new variable. If the new variable is also set, just give a warning so they can eventually remove the old variable. Signed-off-by: Ben Peart --- Makefile | 6 +++--- t/README | 4 t/test-lib.sh | 15 +-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/t/README b/t/README index 47165f7eab..9b13f6d12e 100644 --- a/t/README +++ b/t/README @@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor code path for utilizing a file system monitor to speed up detecting new or changed files. +GIT_TEST_INDEX_VERSION= exercises the index read/write code path +for the index version specified. Can be set to any valid version +(currently 2, 3, or 4). + Naming Tests diff --git a/t/test-lib.sh b/t/test-lib.sh index 0ef111d808..5f5f0f4b55 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,12 +134,23 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi +if test -n "$TEST_GIT_INDEX_VERSION" +then + if test -n "$GIT_TEST_INDEX_VERSION" + then + echo "warning: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION" + else + echo "error: the TEST_GIT_INDEX_VERSION variable has been renamed to GIT_TEST_INDEX_VERSION" + exit 1 + fi +fi + if test -n "$GIT_FSMONITOR_TEST" then if test -n "$GIT_TEST_FSMONITOR" -- 2.18.0.windows.1
[PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with the other GIT_TEST_ special setups and properly document its use. Add logic in t/test-lib.sh to give an error when the old variable is set to let people know they need to update their environment to use the new variable. If the new variable is also set, just give a warning so they can eventually remove the old variable. Signed-off-by: Ben Peart --- preload-index.c | 3 ++- t/README| 3 +++ t/t7519-status-fsmonitor.sh | 4 ++-- t/test-lib.sh | 11 +++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/preload-index.c b/preload-index.c index 71cd2437a3..a850e197c2 100644 --- a/preload-index.c +++ b/preload-index.c @@ -5,6 +5,7 @@ #include "pathspec.h" #include "dir.h" #include "fsmonitor.h" +#include "config.h" #ifdef NO_PTHREADS static void preload_index(struct index_state *index, @@ -84,7 +85,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && getenv("GIT_FORCE_PRELOAD_TEST")) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_TEST_PRELOAD_INDEX", 0)) threads = 2; if (threads < 2) return; diff --git a/t/README b/t/README index 9b13f6d12e..5670c7aad0 100644 --- a/t/README +++ b/t/README @@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write code path for the index version specified. Can be set to any valid version (currently 2, 3, or 4). +GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path +by overriding the minimum number of cache entries required per thread. + Naming Tests diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index d77012ea6d..8308d6d5b1 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -245,9 +245,9 @@ do git config core.preloadIndex $preload_val && if test $preload_val = true then - GIT_FORCE_PRELOAD_TEST=$preload_val; export GIT_FORCE_PRELOAD_TEST + GIT_TEST_PRELOAD_INDEX=$preload_val; export GIT_TEST_PRELOAD_INDEX else - unset GIT_FORCE_PRELOAD_TEST + sane_unset GIT_TEST_PRELOAD_INDEX fi ' diff --git a/t/test-lib.sh b/t/test-lib.sh index 5f5f0f4b55..3f447b8ddc 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -162,6 +162,17 @@ then fi fi +if test -n "$GIT_FORCE_PRELOAD_TEST" +then + if test -n "$GIT_TEST_PRELOAD_INDEX" + then + echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX" + else + echo "error: the GIT_FORCE_PRELOAD_TEST variable has been renamed to GIT_TEST_PRELOAD_INDEX" + exit 1 + fi +fi + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null || -- 2.18.0.windows.1
Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/13/2018 2:54 PM, Ævar Arnfjörð Bjarmason wrote: On Thu, Sep 13 2018, Ben Peart wrote: diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 9028b47d92..545438c820 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests I've seen this & will watch out for it, but still, when I'm updating to "next" in a couple of months I may not be tracking the exact state of the integration of this patch, and then running with GIT_FSMONITOR_TEST=... will suddenly be a noop. So maybe something like this to test-lib.sh as well (or directly in config.c): if test -n "$GIT_FSMONITOR_TEST" then echo "The GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR" exit 1 fi Maybe I'm being too nitpicky and there's only two of us who run the test with this anyway, and we can deal with it. I agree that there are probably only 2 people in the world who ever used this but I'm happy to add the additional test to make it obvious it has been renamed. It just rubs me the wrong way that we have a test mode that silently stops being picked up because a command-line option or env variable got renamed, especially since we've had it for 4 stable releases, especially since it's so easy for us to avoid that confusion (just die), v.s. potential time wasted downstream (wondering why fsmonitor stuff broke on $SOME_OS even though we're testing for it during package build...).
Re: [PATCH v1] preload-index: update GIT_FORCE_PRELOAD_TEST support
Ben Peart writes: > Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD for consistency with the > other GIT_TEST_ special setups and properly document its use. > > Signed-off-by: Ben Peart > --- > Among the two unrelated changes that are not mentioned in the proposed log message, the change to the test to use sane_unset is probably OK, but replacing a call to getenv() with git_env_bool() without making sure necessary header file(s) are included would break the build. I _think_ config.h is the header to include; I didn't try it, though. Thanks. > Notes: > Base Ref: v2.19.0 > Web-Diff: https://github.com/benpeart/git/commit/dcd201b920 > Checkout: git fetch https://github.com/benpeart/git git-test-preload-v1 > && git checkout dcd201b920 > > preload-index.c | 2 +- > t/README| 3 +++ > t/t7519-status-fsmonitor.sh | 4 ++-- > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/preload-index.c b/preload-index.c > index 71cd2437a3..cc8a7333c2 100644 > --- a/preload-index.c > +++ b/preload-index.c > @@ -84,7 +84,7 @@ static void preload_index(struct index_state *index, > return; > > threads = index->cache_nr / THREAD_COST; > - if ((index->cache_nr > 1) && (threads < 2) && > getenv("GIT_FORCE_PRELOAD_TEST")) > + if ((index->cache_nr > 1) && (threads < 2) && > git_env_bool("GIT_TEST_PRELOAD", 0)) > threads = 2; > if (threads < 2) > return; > diff --git a/t/README b/t/README > index 9028b47d92..73fb09560f 100644 > --- a/t/README > +++ b/t/README > @@ -319,6 +319,9 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon > pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_PRELOAD= exercises the preload-index code path by > +overriding the minimum number of cache entries required per thread. > + > Naming Tests > > > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh > index 756beb0d8e..9b703d49b5 100755 > --- a/t/t7519-status-fsmonitor.sh > +++ b/t/t7519-status-fsmonitor.sh > @@ -245,9 +245,9 @@ do > git config core.preloadIndex $preload_val && > if test $preload_val = true > then > - GIT_FORCE_PRELOAD_TEST=$preload_val; export > GIT_FORCE_PRELOAD_TEST > + GIT_TEST_PRELOAD=$preload_val; export GIT_TEST_PRELOAD > else > - unset GIT_FORCE_PRELOAD_TEST > + sane_unset GIT_TEST_PRELOAD > fi > ' > > > base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
On Thu, Sep 13 2018, Ben Peart wrote: > diff --git a/config.c b/config.c > index 3461993f0a..3555c63f28 100644 > --- a/config.c > +++ b/config.c > @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) > int git_config_get_fsmonitor(void) > { > if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) > - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); > + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); > > if (core_fsmonitor && !*core_fsmonitor) > core_fsmonitor = NULL; > diff --git a/t/README b/t/README > index 9028b47d92..545438c820 100644 > --- a/t/README > +++ b/t/README > @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon > pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor > +code path for utilizing a file system monitor to speed up detecting > +new or changed files. > + > Naming Tests > I've seen this & will watch out for it, but still, when I'm updating to "next" in a couple of months I may not be tracking the exact state of the integration of this patch, and then running with GIT_FSMONITOR_TEST=... will suddenly be a noop. So maybe something like this to test-lib.sh as well (or directly in config.c): if test -n "$GIT_FSMONITOR_TEST" then echo "The GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR" exit 1 fi Maybe I'm being too nitpicky and there's only two of us who run the test with this anyway, and we can deal with it. It just rubs me the wrong way that we have a test mode that silently stops being picked up because a command-line option or env variable got renamed, especially since we've had it for 4 stable releases, especially since it's so easy for us to avoid that confusion (just die), v.s. potential time wasted downstream (wondering why fsmonitor stuff broke on $SOME_OS even though we're testing for it during package build...).
[PATCH v2] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with the other GIT_TEST_ special setups and properly document its use. Signed-off-by: Ben Peart --- Notes: Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/e26ccb9004 Checkout: git fetch https://github.com/benpeart/git git-test-index-version-v2 && git checkout e26ccb9004 ### Interdiff (v1..v2): diff --git a/Makefile b/Makefile index 5a969f5830..9e84ef02f7 100644 --- a/Makefile +++ b/Makefile @@ -400,7 +400,7 @@ all:: # (defaults to "man") if you want to have a different default when # "git help" is called without a parameter specifying the format. # -# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite +# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. # @@ -2599,8 +2599,8 @@ endif ifdef GIT_INTEROP_MAKE_OPTS @echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+ endif -ifdef TEST_GIT_INDEX_VERSION - @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+ +ifdef GIT_TEST_INDEX_VERSION + @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+ endif @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi diff --git a/read-cache.c b/read-cache.c index d140ce9989..7b1354d759 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1570,45 +1570,26 @@ static unsigned int get_index_format_default(void) char *envversion = getenv("GIT_INDEX_VERSION"); char *endp; int value; - unsigned int version = -1; + unsigned int version = INDEX_FORMAT_DEFAULT; - if (envversion) { - version = strtoul(envversion, , 10); - if (*endp || - version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" - "Using version %i"), INDEX_FORMAT_DEFAULT); - version = INDEX_FORMAT_DEFAULT; - } - } - - if (version == -1) { - if (!git_config_get_int("index.version", )) { + if (!envversion) { + if (!git_config_get_int("index.version", )) version = value; if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { warning(_("index.version set, but the value is invalid.\n" "Using version %i"), INDEX_FORMAT_DEFAULT); - version = INDEX_FORMAT_DEFAULT; - } + return INDEX_FORMAT_DEFAULT; } + return version; } - if (version == -1) { - envversion = getenv("GIT_TEST_INDEX_VERSION"); - if (envversion) { version = strtoul(envversion, , 10); if (*endp || version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { - warning(_("GIT_TEST_INDEX_VERSION set, but the value is invalid.\n" + warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n" "Using version %i"), INDEX_FORMAT_DEFAULT); version = INDEX_FORMAT_DEFAULT; } - } - } - - if (version == -1) - version = INDEX_FORMAT_DEFAULT; - return version; } diff --git a/t/README b/t/README index f872638a78..fb6359b17b 100644 --- a/t/README +++ b/t/README @@ -320,8 +320,7 @@ path where deltas larger than this limit require extra memory allocation for bookkeeping. GIT_TEST_INDEX_VERSION= exercises the index read/write code path -for the index version specified. Can be set to any valid version -but the non-default version 4 is probably the most beneficial. +for the index version specified. Can be set to any valid version. Naming Tests diff --git a/t/test-lib.sh b/t/test-lib.sh index 44288cbb59..31698c01c4 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -134,9 +134,9 @@ export EDITOR GIT_TRACE_BARE=1 export GIT_TRACE_BARE -if test -n "${TEST_GIT_INDEX_VERSION:+isset}" +if test -n "${GIT_TEST_INDEX_VERSION:+isset}" then - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION" + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION" export GIT_INDEX_VERSION fi ### Patches Makefile | 6 +++--- t/README | 5 - t/test-lib.sh | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile
Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the > other GIT_TEST_ special setups and properly document its use. Makes sense. Thanks for such an attention to detail. > > Signed-off-by: Ben Peart > --- > > Notes: > Base Ref: v2.19.0 > Web-Diff: https://github.com/benpeart/git/commit/311484a684 > Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 > && git checkout 311484a684 > > config.c| 2 +- > t/README| 4 > t/t1700-split-index.sh | 2 +- > t/t7519-status-fsmonitor.sh | 2 +- > 4 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/config.c b/config.c > index 3461993f0a..3555c63f28 100644 > --- a/config.c > +++ b/config.c > @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) > int git_config_get_fsmonitor(void) > { > if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) > - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); > + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); > > if (core_fsmonitor && !*core_fsmonitor) > core_fsmonitor = NULL; > diff --git a/t/README b/t/README > index 9028b47d92..545438c820 100644 > --- a/t/README > +++ b/t/README > @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon > pack-objects code > path where deltas larger than this limit require extra memory > allocation for bookkeeping. > > +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor > +code path for utilizing a file system monitor to speed up detecting > +new or changed files. > + > Naming Tests > > > diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh > index b3b4d83eaf..f6a856f24c 100755 > --- a/t/t1700-split-index.sh > +++ b/t/t1700-split-index.sh > @@ -6,7 +6,7 @@ test_description='split index mode tests' > > # We need total control of index splitting here > sane_unset GIT_TEST_SPLIT_INDEX > -sane_unset GIT_FSMONITOR_TEST > +sane_unset GIT_TEST_FSMONITOR > > test_expect_success 'enable split index' ' > git config splitIndex.maxPercentChange 100 && > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh > index 756beb0d8e..d77012ea6d 100755 > --- a/t/t7519-status-fsmonitor.sh > +++ b/t/t7519-status-fsmonitor.sh > @@ -8,7 +8,7 @@ test_description='git status with file system watcher' > # To run the entire git test suite using fsmonitor: > # > # copy t/t7519/fsmonitor-all to a location in your path and then set > -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. > +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. > # > > # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' > > base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
[PATCH v1] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD for consistency with the other GIT_TEST_ special setups and properly document its use. Signed-off-by: Ben Peart --- Notes: Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/dcd201b920 Checkout: git fetch https://github.com/benpeart/git git-test-preload-v1 && git checkout dcd201b920 preload-index.c | 2 +- t/README| 3 +++ t/t7519-status-fsmonitor.sh | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/preload-index.c b/preload-index.c index 71cd2437a3..cc8a7333c2 100644 --- a/preload-index.c +++ b/preload-index.c @@ -84,7 +84,7 @@ static void preload_index(struct index_state *index, return; threads = index->cache_nr / THREAD_COST; - if ((index->cache_nr > 1) && (threads < 2) && getenv("GIT_FORCE_PRELOAD_TEST")) + if ((index->cache_nr > 1) && (threads < 2) && git_env_bool("GIT_TEST_PRELOAD", 0)) threads = 2; if (threads < 2) return; diff --git a/t/README b/t/README index 9028b47d92..73fb09560f 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,9 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_PRELOAD= exercises the preload-index code path by +overriding the minimum number of cache entries required per thread. + Naming Tests diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..9b703d49b5 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -245,9 +245,9 @@ do git config core.preloadIndex $preload_val && if test $preload_val = true then - GIT_FORCE_PRELOAD_TEST=$preload_val; export GIT_FORCE_PRELOAD_TEST + GIT_TEST_PRELOAD=$preload_val; export GIT_TEST_PRELOAD else - unset GIT_FORCE_PRELOAD_TEST + sane_unset GIT_TEST_PRELOAD fi ' base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 -- 2.18.0.windows.1
[PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the other GIT_TEST_ special setups and properly document its use. Signed-off-by: Ben Peart --- Notes: Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/311484a684 Checkout: git fetch https://github.com/benpeart/git git-test-fsmonitor-v1 && git checkout 311484a684 config.c| 2 +- t/README| 4 t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", _fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 9028b47d92..545438c820 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index b3b4d83eaf..f6a856f24c 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -6,7 +6,7 @@ test_description='split index mode tests' # We need total control of index splitting here sane_unset GIT_TEST_SPLIT_INDEX -sane_unset GIT_FSMONITOR_TEST +sane_unset GIT_TEST_FSMONITOR test_expect_success 'enable split index' ' git config splitIndex.maxPercentChange 100 && diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 756beb0d8e..d77012ea6d 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,7 +8,7 @@ test_description='git status with file system watcher' # To run the entire git test suite using fsmonitor: # # copy t/t7519/fsmonitor-all to a location in your path and then set -# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests. +# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests. # # Note, after "git reset --hard HEAD" no extensions exist other than 'TREE' base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 -- 2.18.0.windows.1
[PATCH v5 03/12] t0000: update tests for SHA-256
Test t tests the "basics of the basics" and as such, checks that we have various fixed hard-coded object IDs. The tests relying on these assertions have been marked with the SHA1 prerequisite, as they will obviously not function in their current form with SHA-256. Use the test_oid helper to update these assertions and provide values for both SHA-1 and SHA-256. These object IDs were synthesized using a set of scripts that created the objects for both SHA-1 and SHA-256 using the same method to ensure that they are indeed the correct values. Signed-off-by: brian m. carlson --- t/t-basic.sh | 163 +-- 1 file changed, 102 insertions(+), 61 deletions(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index a9dc534048..391f910c6a 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -861,6 +861,47 @@ test_expect_success 'test_oid can look up data for SHA-256' ' # Basics of the basics +test_oid_cache <<\EOF +path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154 +path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d + +path0s sha1:15a98433ae33114b085f3eb3bb03b832b3180a01 +path0s sha256:3a24cc53cf68edddac490bbf94a418a52932130541361f685df685e41dd6c363 + +path2f sha1:3feff949ed00a62d9f7af97c15cd8a30595e7ac7 +path2f sha256:2a7f36571c6fdbaf0e3f62751a0b25a3f4c54d2d1137b3f4af9cb794bb498e5f + +path2s sha1:d8ce161addc5173867a3c3c730924388daedbc38 +path2s sha256:18fd611b787c2e938ddcc248fabe4d66a150f9364763e9ec133dd01d5bb7c65a + +path2d sha1:58a09c23e2ca152193f2786e06986b7b6712bdbe +path2d sha256:00e4b32b96e7e3d65d79112dcbea53238a22715f896933a62b811377e2650c17 + +path3f sha1:0aa34cae68d0878578ad119c86ca2b5ed5b28376 +path3f sha256:09f58616b951bd571b8cb9dc76d372fbb09ab99db2393f5ab3189d26c45099ad + +path3s sha1:8599103969b43aff7e430efea79ca4636466794f +path3s sha256:fce1aed087c053306f3f74c32c1a838c662bbc4551a7ac2420f5d6eb061374d0 + +path3d sha1:21ae8269cacbe57ae09138dcc3a2887f904d02b3 +path3d sha256:9b60497be959cb830bf3f0dc82bcc9ad9e925a24e480837ade46b2295e47efe1 + +subp3f sha1:00fb5908cb97c2564a9783c0c64087333b3b464f +subp3f sha256:a1a9e16998c988453f18313d10375ee1d0ddefe757e710dcae0d66aa1e0c58b3 + +subp3s sha1:6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c +subp3s sha256:81759d9f5e93c6546ecfcadb560c1ff057314b09f93fe8ec06e2d8610d34ef10 + +subp3d sha1:3c5e5399f3a333eddecce7a9b9465b63f65f51e2 +subp3d sha256:76b4ef482d4fa1c754390344cf3851c7f883b27cf9bc999c6547928c46aeafb7 + +root sha1:087704a96baf1c2d1c869a8b084481e121c88b5b +root sha256:9481b52abab1b2ffeedbf9de63ce422b929f179c1b98ff7bee5f8f1bc0710751 + +simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a +simpletree sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5 +EOF + # updating a new file without --add should fail. test_expect_success 'git update-index without --add should fail adding' ' test_must_fail git update-index should-be-empty @@ -876,8 +917,8 @@ test_expect_success 'writing tree out with git write-tree' ' ' # we know the shape and contents of the tree and know the object ID for it. -test_expect_success SHA1 'validate object ID of a known tree' ' - test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a +test_expect_success 'validate object ID of a known tree' ' + test "$tree" = "$(test_oid simpletree)" ' # Removing paths. @@ -919,16 +960,16 @@ test_expect_success 'showing stage with git ls-files --stage' ' git ls-files --stage >current ' -test_expect_success SHA1 'validate git ls-files output for a known tree' ' - cat >expected <<-\EOF && - 100644 f87290f8eb2cbbea7857214459a0739927eab154 0 path0 - 12 15a98433ae33114b085f3eb3bb03b832b3180a01 0 path0sym - 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0 path2/file2 - 12 d8ce161addc5173867a3c3c730924388daedbc38 0 path2/file2sym - 100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0 path3/file3 - 12 8599103969b43aff7e430efea79ca4636466794f 0 path3/file3sym - 100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0 path3/subp3/file3 - 12 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0 path3/subp3/file3sym +test_expect_success 'validate git ls-files output for a known tree' ' + cat >expected <<-EOF && + 100644 $(test_oid path0f) 0 path0 + 12 $(test_oid path0s) 0 path0sym + 100644 $(test_oid path2f) 0 path2/file2 + 12 $(test_oid path2s) 0 path2/file2sym + 100644 $(test_oid path3f) 0 path3/file3 + 12 $(test_oid path3s) 0 path3/file3sym + 100644 $(test_oid subp3f) 0 path3/subp3/file3 + 12 $(test_oid subp3s) 0 path3/subp3/file3sym EOF test_cmp expected current ' @@ -937,20 +978,20 @@ test_exp
Re: [PATCH] submodule.sh update --remote: default to oid instead of master
Stefan Beller wrote: > On Wed, Sep 5, 2018 at 4:10 PM Jonathan Nieder wrote: >> Broader comment: do you think people will be surprised by this new >> behavior? Is there anything special we'd need to do to call it out >> (e.g., print a warning or put something in release notes)? > > I guess. Not sure how to approach this best. Maybe we can > extend the output of 'submodule update' to print that branch names > instead of hashes for the configured case and keep printing hashes > only for this case. Although that would not help someone who relies > on the default solely. Thinking more out loud: often the simplest migration path involves multiple steps: 1. Warn in the case that is going to change, with no behavior change yet. 2. Treat the case that will change as an error. This should help flush out cases where people were relying on the old behavior. 3. Introduce the new behavior. Warn that old versions of Git don't support it yet. 4. Eliminate the warning. You're all clear now. Sometimes some of these steps can be combined. Another possible approach is to measure. For example, is there some way to find out how many people are relying in this "git submodule update --remote" defaulting behavior? One example of this approach is to make the change (all in one step) in "next" and deploy to some relevant subpopulation and see if anyone screams. By making the change in "next" instead of something with more stability guarantees, you get the ability to roll back quickly. There are other tools at our disposal --- e.g. command-line flags, config, other kinds of research. Here my first instinct would be to say this should be a command-line flag. To start out, we can keep the historical behavior as a default, but introduce a command-line option for the new behavior. This way, people can pass the negation of that command-line option if they want the older behavior, throughout the transition. For example (please ignore names): Step 0: introduce git submodule update --remote --default-to-master; # current behavior git submodule update --remote --no-default-to-master; # new behavior and treat plain "git submodule update --remote" as --default-to-master. Step 1: when neither --default-to-master nor --no-default-to-master has been passed, warn when encountering a submodule with no branch and treat it as "master". Step 2: when neither --default-to-master nor --no-default-to-master has been passed, error out when encountering a submodule with no branch. Step 3: when neither --default-to-master nor --no-default-to-master has been passed, warn when encountering a submodule with no branch and treat it as pinned. Step 4: eliminate the warning. What do you think? Thanks, Jonathan
Re: [PATCH] submodule.sh update --remote: default to oid instead of master
On Wed, Sep 5, 2018 at 4:10 PM Jonathan Nieder wrote: > > Stefan Beller wrote: > > > Subject: submodule.sh update --remote: default to oid instead of master > > Yay! > > Nit: it wasn't clear to me at first what default this subject line was > referring to. Perhaps: > > submodule update --remote: skip GITLINK update when no branch is set > > [...] > > --- a/Documentation/gitmodules.txt > > +++ b/Documentation/gitmodules.txt > > @@ -50,11 +50,12 @@ submodule..update:: > > > > submodule..branch:: > [...] > > + If the option is not specified, do not update to any branch but > > + the object id of the remote. > > Likewise: how about something like > > If not set, the default is for `git submodule update --remote` > to update the submodule to the superproject's recorded SHA-1. ... recorded object id. sounds good. > > + git add .gitmodules && > > + git commit --allow-empty -m "submodules: pin in superproject > > branch" > > + ) && > > I wonder if we can do simpler by using -C + some helpers: something like > > git config --unset -f super/.gitmodules ... && > test_commit -C submodule ... && > git -C super submodule update ... && > test_cmp_rev ... > > Unfortunately test_cmp_rev doesn't accept a -C argument. and the lack of fortune goes further, as test_cmp_rev needs to have 2 revisions in the same repository, i.e. both need to exist, which is not the case. > Broader comment: do you think people will be surprised by this new > behavior? Is there anything special we'd need to do to call it out > (e.g., print a warning or put something in release notes)? I guess. Not sure how to approach this best. Maybe we can extend the output of 'submodule update' to print that branch names instead of hashes for the configured case and keep printing hashes only for this case. Although that would not help someone who relies on the default solely.
Re: [PATCH] submodule.sh update --remote: default to oid instead of master
Stefan Beller wrote: > Subject: submodule.sh update --remote: default to oid instead of master Yay! Nit: it wasn't clear to me at first what default this subject line was referring to. Perhaps: submodule update --remote: skip GITLINK update when no branch is set [...] > --- a/Documentation/gitmodules.txt > +++ b/Documentation/gitmodules.txt > @@ -50,11 +50,12 @@ submodule..update:: > > submodule..branch:: [...] > + If the option is not specified, do not update to any branch but > + the object id of the remote. Likewise: how about something like If not set, the default is for `git submodule update --remote` to update the submodule to the superproject's recorded SHA-1. [...] > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -568,16 +568,19 @@ cmd_update() > if test -n "$remote" > then > branch=$(git submodule--helper remote-branch "$sm_path") > - if test -z "$nofetch" > + if test -n "$branch" > then > - # Fetch remote before determining tracking $sha1 > - fetch_in_submodule "$sm_path" $depth || > - die "$(eval_gettext "Unable to fetch in > submodule path '\$sm_path'")" > + if test -z "$nofetch" > + then > + # Fetch remote before determining > tracking $sha1 > + fetch_in_submodule "$sm_path" $depth || Makes sense. If $sha1 isn't available in the submodule, it will fetch again later. [...] > --- a/t/t7406-submodule-update.sh > +++ b/t/t7406-submodule-update.sh > @@ -260,6 +260,28 @@ test_expect_success 'submodule update --remote should > fetch upstream changes wit > ) > ' > > +test_expect_success 'submodule update --remote should not fetch upstream > when no branch is set' ' > + ( > + cd super && > + test_might_fail git config --unset -f .gitmodules > submodule."submodule".branch && Not about this patch: the quoting here is strange. > + git add .gitmodules && > + git commit --allow-empty -m "submodules: pin in superproject > branch" > + ) && I wonder if we can do simpler by using -C + some helpers: something like git config --unset -f super/.gitmodules ... && test_commit -C submodule ... && git -C super submodule update ... && test_cmp_rev ... Unfortunately test_cmp_rev doesn't accept a -C argument. Broader comment: do you think people will be surprised by this new behavior? Is there anything special we'd need to do to call it out (e.g., print a warning or put something in release notes)? Thanks, Jonathan
[PATCH] submodule.sh update --remote: default to oid instead of master
gitmodules(5) sayeth: submodule..branch A remote branch name for tracking updates in the upstream submodule. If the option is not specified, it defaults to master. This doesn't allow having a "pinned" submodule that should not be updated from upstream. We should change this to have no default --- if branch is not specified, don't update that submodule, just like in Gerrit's corresponding feature[1]. [1] https://gerrit-review.googlesource.com/Documentation/user-submodules.html#_defining_the_submodule_branch Signed-off-by: Stefan Beller --- Documentation/gitmodules.txt | 11 ++- git-submodule.sh | 19 +++ t/t7406-submodule-update.sh | 22 ++ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 4d63def2069..3b8739f8294 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -50,11 +50,12 @@ submodule..update:: submodule..branch:: A remote branch name for tracking updates in the upstream submodule. - If the option is not specified, it defaults to 'master'. A special - value of `.` is used to indicate that the name of the branch in the - submodule should be the same name as the current branch in the - current repository. See the `--remote` documentation in - linkgit:git-submodule[1] for details. + A special value of `.` is used to indicate that the name of the + branch in the submodule should be the same name as the current + branch in the current repository. See the `--remote` documentation + in linkgit:git-submodule[1] for details. + If the option is not specified, do not update to any branch but + the object id of the remote. submodule..fetchRecurseSubmodules:: This option can be used to control recursive fetching of this diff --git a/git-submodule.sh b/git-submodule.sh index f7fd80345cd..342050ae934 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -568,16 +568,19 @@ cmd_update() if test -n "$remote" then branch=$(git submodule--helper remote-branch "$sm_path") - if test -z "$nofetch" + if test -n "$branch" then - # Fetch remote before determining tracking $sha1 - fetch_in_submodule "$sm_path" $depth || - die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" + if test -z "$nofetch" + then + # Fetch remote before determining tracking $sha1 + fetch_in_submodule "$sm_path" $depth || + die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")" + fi + remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote) + sha1=$(sanitize_submodule_env; cd "$sm_path" && + git rev-parse --verify "${remote_name}/${branch}") || + die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")" fi - remote_name=$(sanitize_submodule_env; cd "$sm_path" && get_default_remote) - sha1=$(sanitize_submodule_env; cd "$sm_path" && - git rev-parse --verify "${remote_name}/${branch}") || - die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")" fi if ! $(git config -f "$(git rev-parse --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 10dc91620a6..f04884743fd 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -260,6 +260,28 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit ) ' +test_expect_success 'submodule update --remote should not fetch upstream when no branch is set' ' + ( + cd super && + test_might_fail git config --unset -f .gitmodules submodule."submodule".branch && + git add .gitmodules && + git commit --allow-empty -m "submodules: pin in superproject branch" +
[PATCH 0/2] --no-deref and --stdin compatibility for update-ref
Currently, the --no-deref and --stdin options of update-ref cannot be used together (the code aborts immediately with a usage message), though it makes sense to do so and is easier than repeatedly specifying on stdin that each ref should not be dereferenced. Also, the documentation for the --no-deref option has a few problems, making it unclear what it is and isn't compatible with. The first patch is just a minor code fixup that the second lightly depends on. The second patch has the changes to fix the issues stated above. Elijah Newren (2): update-ref: fix type of update_flags variable to match its usage update-ref: allow --no-deref with --stdin Documentation/git-update-ref.txt | 2 +- builtin/update-ref.c | 25 ++--- t/t1400-update-ref.sh| 31 +++ 3 files changed, 46 insertions(+), 12 deletions(-) -- 2.19.0.rc2.2.g1aedc61e22
[PATCH 2/2] update-ref: allow --no-deref with --stdin
If passed both --no-deref and --stdin, update-ref would error out with a general usage message that did not at all suggest these options were incompatible. The manpage for update-ref did suggest through its synopsis line that --no-deref and --stdin were incompatible, but it sadly also incorrectly suggested that -d and --no-deref were incompatible. So the help around the --no-deref option is buggy in a few ways. The --stdin option did provide a different mechanism for avoiding dereferencing symbolic-refs: adding a line reading option no-deref before every other directive in the input. (Technically, if the user wants to do the extra work of first determining which refs they want to update or delete are symbolic, then they only need to put the extra "option no-deref" lines before the updates of those refs. But in some cases, that's more work than just adding the "option no-deref" before every other directive.) It's easier to allow the user to just pass --no-deref along with --stdin in order to tell update-ref that the user doesn't want any symbolic ref to be dereferenced. It also makes the update-ref documentation simpler. Implement that, and update the documentation to match. Signed-off-by: Elijah Newren --- Documentation/git-update-ref.txt | 2 +- builtin/update-ref.c | 23 +-- t/t1400-update-ref.sh| 31 +++ 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt index bc8fdfd469..fda8516677 100644 --- a/Documentation/git-update-ref.txt +++ b/Documentation/git-update-ref.txt @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely SYNOPSIS [verse] -'git update-ref' [-m ] (-d [] | [--no-deref] [--create-reflog] [] | --stdin [-z]) +'git update-ref' [-m ] [--no-deref] (-d [] | [--create-reflog] [] | --stdin [-z]) DESCRIPTION --- diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 54fac01f21..2d8f7f0578 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -15,6 +15,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static unsigned int update_flags; +static unsigned int default_flags; static unsigned create_reflog_flag; static const char *msg; @@ -205,7 +206,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction, msg, )) die("%s", err.buf); - update_flags = 0; + update_flags = default_flags; free(refname); strbuf_release(); @@ -237,7 +238,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction, msg, )) die("%s", err.buf); - update_flags = 0; + update_flags = default_flags; free(refname); strbuf_release(); @@ -273,7 +274,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction, update_flags, msg, )) die("%s", err.buf); - update_flags = 0; + update_flags = default_flags; free(refname); strbuf_release(); @@ -302,7 +303,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction, update_flags, )) die("%s", err.buf); - update_flags = 0; + update_flags = default_flags; free(refname); strbuf_release(); @@ -357,7 +358,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) const char *refname, *oldval; struct object_id oid, oldoid; int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0; - unsigned int flags = 0; int create_reflog = 0; struct option options[] = { OPT_STRING( 'm', NULL, , N_("reason"), N_("reason of the update")), @@ -378,6 +378,11 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) create_reflog_flag = create_reflog ? REF_FORCE_CREATE_REFLOG : 0; + if (no_deref) { + default_flags = REF_NO_DEREF; + update_flags = default_flags; + } + if (read_stdin) { struct strbuf err = STRBUF_INIT; struct ref_transaction *transaction; @@ -385,7 +390,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) transaction = ref_transaction_begin(); if (!transaction) die("%s", err.buf); - if (delete || no_deref || argc > 0) + if (delete || argc > 0) usage_with_options(git_update_ref_usage, options); if (end_null) line_termination = '\0'; @@ -427,8 +432,6 @@ int cmd_update_ref(int arg
[PATCH 1/2] update-ref: fix type of update_flags variable to match its usage
The ref_transaction_*() family of functions expect a flags parameter which is of type unsigned int. Make the update_flags variable, which is passed as that parameter, be of the same type. Signed-off-by: Elijah Newren --- builtin/update-ref.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 4fa3c0a86f..54fac01f21 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = { }; static char line_termination = '\n'; -static int update_flags; +static unsigned int update_flags; static unsigned create_reflog_flag; static const char *msg; -- 2.19.0.rc2.2.g1aedc61e22