* bug: git fetch reports too many unreachable loose objects
From: Josh Wolfe @ 2018-12-08 2:12 UTC (permalink / raw)
To: git
git version 2.19.1
steps to reproduce:
# start in a brand new repo
git init
# create lots of unreachable loose objects
for i in {1..10000}; do git commit-tree -m "$(head -c 12 /dev/urandom
| base64)" "$(git mktree <&-)" <&-; done
# this prints a lot of output and takes a minute or so to run
# trigger git gc to run in the background
git fetch
# Auto packing the repository in background for optimum performance.
# See "git help gc" for manual housekeeping.
# trigger it again
git fetch
# Auto packing the repository in background for optimum performance.
# See "git help gc" for manual housekeeping.
# error: The last gc run reported the following. Please correct the root cause
# and remove .git/gc.log.
# Automatic cleanup will not be performed until the file is removed.
#
# warning: There are too many unreachable loose objects; run 'git
prune' to remove them.
# to manually fix this, run git prune:
git prune
# note that `git gc` does not fix the problem, and appears to do
nothing in this situation:
git gc
According to the `git fetch` output, the `git help gc` docs, and the
`git help prune` docs, I don't think I shouldn't ever have to run `git
prune` manually, so this behavior seems like a bug to me. Please
correct me if this is expected behavior.
In case anyone's wondering why I'm creating unreachable loose objects,
here's the usecase: https://stackoverflow.com/a/50403179/367916 . I
would love a first-class solution to obviate that workaround, but that
is probably a separate issue.
Josh
^ permalink raw reply
* Re: [wishlist] git submodule update --reset-hard
From: Yaroslav Halchenko @ 2018-12-08 2:15 UTC (permalink / raw)
To: git
In-Reply-To: <CAGZ79kbeAd1C-ySnJye-QU5FFf2jygksUsWtEmbvPZ_dQy_3uA@mail.gmail.com>
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
^ permalink raw reply
* Re: bug: git fetch reports too many unreachable loose objects
From: Elijah Newren @ 2018-12-08 2:23 UTC (permalink / raw)
To: josh; +Cc: Git Mailing List
In-Reply-To: <CAKQQkGppdO5pwOpG+sJ0uc2uPW867TZzWzc0ZntPd6MkqsozFQ@mail.gmail.com>
On Fri, Dec 7, 2018 at 6:14 PM Josh Wolfe <josh@okcupid.com> wrote:
>
> git version 2.19.1
> steps to reproduce:
>
> # start in a brand new repo
> git init
>
> # create lots of unreachable loose objects
> for i in {1..10000}; do git commit-tree -m "$(head -c 12 /dev/urandom
> | base64)" "$(git mktree <&-)" <&-; done
> # this prints a lot of output and takes a minute or so to run
>
> # trigger git gc to run in the background
> git fetch
> # Auto packing the repository in background for optimum performance.
> # See "git help gc" for manual housekeeping.
>
> # trigger it again
> git fetch
> # Auto packing the repository in background for optimum performance.
> # See "git help gc" for manual housekeeping.
> # error: The last gc run reported the following. Please correct the root cause
> # and remove .git/gc.log.
> # Automatic cleanup will not be performed until the file is removed.
> #
> # warning: There are too many unreachable loose objects; run 'git
> prune' to remove them.
>
> # to manually fix this, run git prune:
> git prune
>
> # note that `git gc` does not fix the problem, and appears to do
> nothing in this situation:
> git gc
>
>
> According to the `git fetch` output, the `git help gc` docs, and the
> `git help prune` docs, I don't think I shouldn't ever have to run `git
> prune` manually, so this behavior seems like a bug to me. Please
> correct me if this is expected behavior.
Known bug, there are a variety of other ways to trigger it too. See
the threads here for more info:
https://public-inbox.org/git/87inc89j38.fsf@evledraar.gmail.com/
https://public-inbox.org/git/20180716172717.237373-1-jonathantanmy@google.com/
There are probably other threads as well.
^ permalink raw reply
* Re: [wishlist] git submodule update --reset-hard
From: Yaroslav Halchenko @ 2018-12-08 4:21 UTC (permalink / raw)
To: git
In-Reply-To: <20181208021531.GB4633@hopa.kiewit.dartmouth.edu>
[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]
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
[-- Attachment #2: 0001-submodule-Add-reset-hard-option-for-git-submodule-up.patch --]
[-- Type: text/x-diff, Size: 8916 bytes --]
From 170296dc661b4bc3d942917ce27288df52ff650d Mon Sep 17 00:00:00 2001
From: Yaroslav Halchenko <debian@onerussian.com>
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 <debian@onerussian.com>
---
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 <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
+update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge|--reset-hard] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]::
+
--
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.<name>.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 linkgit:git-submodule[1] for their meaning. Note that the
'!command' form is intentionally ignored here for security
reasons.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d38113a31a..31d95c3cd6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1481,6 +1481,7 @@ static void determine_submodule_update_strategy(struct repository *r,
if (just_cloned &&
(out->type == SM_UPDATE_MERGE ||
out->type == SM_UPDATE_REBASE ||
+ out->type == SM_UPDATE_RESET_HARD ||
out->type == SM_UPDATE_NONE))
out->type = SM_UPDATE_CHECKOUT;
@@ -1851,7 +1852,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)
"submodule boundaries")),
OPT_STRING(0, "update", &update,
N_("string"),
- N_("rebase, merge, checkout or none")),
+ N_("rebase, merge, checkout, reset-hard or none")),
OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
N_("reference repository")),
OPT_BOOL(0, "dissociate", &suc.dissociate,
diff --git a/git-submodule.sh b/git-submodule.sh
index 5e608f8bad..b5d6fad983 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -9,7 +9,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)
- or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
+ or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase|--reset-hard] [--[no-]recommend-shallow] [--reference <repository>] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
or: $dashless [--quiet] sync [--recursive] [--] [<path>...]
@@ -483,6 +483,9 @@ cmd_update()
-m|--merge)
update="merge"
;;
+ --reset-hard)
+ update="reset-hard"
+ ;;
--recursive)
recursive=1
;;
@@ -621,6 +624,11 @@ cmd_update()
say_msg="$(eval_gettext "Submodule path '\$displaypath': merged in '\$sha1'")"
must_die_on_failure=yes
;;
+ reset-hard)
+ command="git reset --hard"
+ die_msg="$(eval_gettext "Unable to reset --hard to '\$sha1' in submodule path '\$displaypath'")"
+ say_msg="$(eval_gettext "Submodule path '\$displaypath': was reset --hard to '\$sha1'")"
+ ;;
!*)
command="${update_module#!}"
die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")"
diff --git a/submodule.c b/submodule.c
index 6415cc5580..4580cf0944 100644
--- a/submodule.c
+++ b/submodule.c
@@ -373,6 +373,8 @@ enum submodule_update_type parse_submodule_update_type(const char *value)
return SM_UPDATE_REBASE;
else if (!strcmp(value, "merge"))
return SM_UPDATE_MERGE;
+ else if (!strcmp(value, "reset-hard"))
+ return SM_UPDATE_RESET_HARD;
else if (*value == '!')
return SM_UPDATE_COMMAND;
else
@@ -406,6 +408,8 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
return "checkout";
case SM_UPDATE_MERGE:
return "merge";
+ case SM_UPDATE_RESET_HARD:
+ return "reset-hard";
case SM_UPDATE_REBASE:
return "rebase";
case SM_UPDATE_NONE:
diff --git a/submodule.h b/submodule.h
index a680214c01..f23ac4630e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -29,6 +29,7 @@ enum submodule_update_type {
SM_UPDATE_CHECKOUT,
SM_UPDATE_REBASE,
SM_UPDATE_MERGE,
+ SM_UPDATE_RESET_HARD,
SM_UPDATE_NONE,
SM_UPDATE_COMMAND
};
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index e87164aa8f..2e08e0047c 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -6,7 +6,8 @@
test_description='Test updating submodules
This test verifies that "git submodule update" detaches the HEAD of the
-submodule and "git submodule update --rebase/--merge" does not detach the HEAD.
+submodule and "git submodule update --rebase/--merge/--reset-hard" does
+not detach the HEAD.
'
. ./test-lib.sh
@@ -305,6 +306,20 @@ test_expect_success 'submodule update --merge staying on master' '
)
'
+test_expect_success 'submodule update --reset-hard staying on master' '
+ (cd super/submodule &&
+ git reset --hard HEAD~1
+ ) &&
+ (cd super &&
+ (cd submodule &&
+ compare_head
+ ) &&
+ git submodule update --reset-hard submodule &&
+ cd submodule &&
+ compare_head
+ )
+'
+
test_expect_success 'submodule update - rebase in .git/config' '
(cd super &&
git config submodule.submodule.update rebase
--
2.20.0.rc2.8.g0a3bec4a1c.dirty
^ permalink raw reply related
* Re: [PATCH] commit: abort before commit-msg if empty message
From: Junio C Hamano @ 2018-12-08 5:44 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
In-Reply-To: <20181207224817.231957-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> When a user runs "git commit" without specifying a message, an editor
> appears with advice:
>
> Please enter the commit message for your changes. Lines starting
> with '#' will be ignored, and an empty message aborts the commit.
>
> However, if the user supplies an empty message and has a commit-msg hook
> which updates the message to be non-empty, the commit proceeds to occur,
> despite what the advice states.
When "--no-edit" is given, and when commit-msg fills that blank, the
command should go ahead and record the commit, I think.
An automation where commit-msg is used to produce whatever
appropriate message for the automation is entirely a reasonable
thing to arrange. Of course, you can move the logic to produce an
appropriate message for the automation from commit-msg to the script
that drives the "git commit" and use the output of that logic as the
value for the "-m" option to achieve the same, so in that sense,
there is an escape hatch even if you suddenly start to forbid such a
working set-up, but it nevertheless is unnecessary busywork for those
with such a set-up to adjust to this change.
I actually think in this partcular case, the commit-msg hook that
adds Change-ID to an empty message is BUGGY. If the hook looked at
the message contents and refrains from making an otherwise empty
message to non-empty, there is no need for any change here.
In any case, you'll have plenty of time to make your case after the
rc freeze. I am not so sympathetic to a patch that makes us bend
backwards to support such a buggy hook to e honest.
^ permalink raw reply
* Re: [PATCH 0/4]
From: Junio C Hamano @ 2018-12-08 5:57 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20181207235425.128568-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> A couple days before the 2.19 release we had a bug report about
> broken submodules[1] and reverted[2] the commits leading up to them.
>
> The behavior of said bug fixed itself by taking a different approach[3],
> specifically by a weaker enforcement of having `core.worktree` set in a
> submodule [4].
>
> The revert [2] was overly broad as we neared the release, such that we wanted
> to rather keep the known buggy behavior of always having `core.worktree` set,
> rather than figuring out how to fix the new bug of having 'git submodule update'
> not working in old style repository setups.
>
> This series re-introduces those reverted patches, with no changes in code,
> but with drastically changed commit messages, as those focus on why it is safe
> to re-introduce them instead of explaining the desire for the change.
The above was a bit too cryptic for me to grok, so let me try
rephrasing to see if I got them all correctly.
- three-patch series leading to 984cd77ddb were meant to fix some
bug, but the series itself was buggy and caused problems; we got
rid of them
- the problem 984cd77ddb wanted to fix was fixed differently
without reintroducing the problem three-patch series introduced.
That fix is already with us since 4d6d6ef1fc.
- now these three changes that were problematic in the past is
resent without any update (other than that it has one preparatory
patch to add tests).
Is that what is going on? Obviously I am not getting "the other"
benefit we wanted to gain out of these three patches (because the
above description fails to explain what that is), other than to fix
the issue that was fixed by 4d6d6ef1fc.
Sorry for being puzzled...
> [1] https://public-inbox.org/git/2659750.rG6xLiZASK@twilight
> [2] f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07)
> [3] 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17)
> [4] 74d4731da1 (submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)
>
> Stefan Beller (4):
> submodule update: add regression test with old style setups
> submodule: unset core.worktree if no working tree is present
> submodule--helper: fix BUG message in ensure_core_worktree
> submodule deinit: unset core.worktree
>
> builtin/submodule--helper.c | 4 +++-
> submodule.c | 14 ++++++++++++++
> submodule.h | 2 ++
> t/lib-submodule-update.sh | 5 +++--
> t/t7400-submodule-basic.sh | 5 +++++
> t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
> 6 files changed, 33 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: RFE: git-patch-id should handle patches without leading "diff"
From: Junio C Hamano @ 2018-12-08 6:01 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ævar Arnfjörð Bjarmason, Konstantin Ryabitsev, git
In-Reply-To: <20181207223406.GD73340@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
>> So it seems most sensible to me if this is going to be supported that we
>> go a bit beyond the call of duty and fake up the start of it, namely:
>>
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>>
>> To be:
>>
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>
> Right. We may want to handle diff.mnemonicPrefix as well.
I definitely think under the --stable option, we should pretend as
if the canonical a/ vs b/ prefixes were given with the "diff --git"
header, just like we try to reverse the effect of diff-orderfile,
etc.
I am unsure what the right behaviour under --unstable is, though.
^ permalink raw reply
* Re: [PATCH] mailmap: update brandon williams's email address
From: Junio C Hamano @ 2018-12-08 6:08 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jonathan Nieder, bwilliamseng, git, bwilliams.eng
In-Reply-To: <CAGZ79kYrgpZDqAhg8c11V_qJTCzzw4-qrVN2z_Y_OAeCbWU6dQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>>
>> Brandon Williams wrote:
>>
>> > Signed-off-by: Brandon Williams <bwilliams.eng@gmail.com>
>> > ---
>> > .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.
^ permalink raw reply
* Re: [RFC PATCH 2/3] Documentation/git-rev-list: s/<commit>/<object>/
From: Junio C Hamano @ 2018-12-08 6:24 UTC (permalink / raw)
To: Matthew DeVore
Cc: git, Stefan Beller, git, jeffhost, Jeff King, Stefan Beller,
Jonathan Tan, pclouds
In-Reply-To: <CAMfpvh+6r65OqWpY-geOYHizs526A1004vakKSdxnO=fsahdAg@mail.gmail.com>
Matthew DeVore <matvore@google.com> writes:
> $ git -C pc1 fetch-pack --stdin "file://$(pwd)/srv.bare" <observed.oids
>
> Where observed.oids contains all the blobs that were missing. It tells
> the remote that it already has the "refs/heads/master" commit, which
> means it is excluded. Before, this worked fine, since it didn't mean
> the blobs were excluded, only the commit itself.
So 'master' has some blob in its tree, which is missing from the
repository, in this test? Which means that such a repository is
"corrupt" and does not pass connectivity check by fsck.
I am of two minds. If we claim that we have everything that is
needed to complete the commit sitting at the tip of 'master', I
think it is correct for the other side not to send a blob that is in
'master' (or its ancestors), so your "fix" may (at least
technically) be more correct than the status quo. On the other
hand, if possession of commit 'master' does not defeat an explicit
request for a blob in it, that would actually be a good thing---it
would be a very straight-forward way to recover from such form of
repository corruption.
Fetching isolated objects without walking is also something that
would help backfill a lazily created clone, and I even vaguely
recall an effort to allow objects explicitly requested to be always
fetched regardless of the connectivity, if I am not mistaken (JTan?)
Anyway, thanks for a thoughtful response.
^ permalink raw reply
* Re: [PATCH 2/4] submodule: unset core.worktree if no working tree is present
From: Junio C Hamano @ 2018-12-08 6:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20181207235425.128568-3-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This reintroduces 4fa4f90ccd (submodule: unset core.worktree if no working
> tree is present, 2018-06-12), which was reverted as part of f178c13fda
> (Revert "Merge branch 'sb/submodule-core-worktree'", 2018-09-07).
>
> 4fa4f90ccd was reverted as its followup commit was faulty, but without
> the accompanying change of the followup, we'd have an incomplete workflow
> of setting `core.worktree` again, when it is needed such as checking out
> a revision that contains a submodule.
>
> So re-introduce that commit as-is, focusing on fixing up the followup
I was hoping to hear (given what 0/4 claimed) a clearer explanation
of what this change wants to achieve, but that is lacking.
No need to grumble about an earlier work was that turned out to be
inappropriate for the codebase back then. Repeatedly saying "this
is needed" without giving further explaining why it is so, or
anything like that, would help readers.
Just pretend that the ealier commits and their reversion never
happened, and further pretend that we are doing the best thing that
should happen to our codebase. How would we explain this change,
what the problem it tries to solve and what the solution looks like
in the larger picture?
When removing a working tree of a submodule (e.g. we may be
switching back to an earlier commit in the superproject that
did not have the submodule in question yet), we failed to
unset core.worktree of the submodule's repository. That
caused this and that issues, exhibited by a few new tests
this commit adds.
Make sure that core.worktree gets unset so that a leftover
setting won't cause these issues.
or something like that? I am just guessing by looking at the old
commit's text, as the above two paragraphs and one sentence does not
say much.
> diff --git a/submodule.c b/submodule.c
> index 6415cc5580..d393e947e6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1561,6 +1561,18 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
> return ret;
> }
>
> +void submodule_unset_core_worktree(const struct submodule *sub)
> +{
> + char *config_path = xstrfmt("%s/modules/%s/config",
> + get_git_common_dir(), sub->name);
> +
> + if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
> + warning(_("Could not unset core.worktree setting in submodule '%s'"),
> + sub->path);
> +
> + free(config_path);
> +}
> +
> static const char *get_super_prefix_or_empty(void)
> {
> const char *s = get_super_prefix();
> @@ -1726,6 +1738,8 @@ int submodule_move_head(const char *path,
>
> if (is_empty_dir(path))
> rmdir_or_warn(path);
> +
> + submodule_unset_core_worktree(sub);
> }
> }
> out:
> diff --git a/submodule.h b/submodule.h
> index a680214c01..9e18e9b807 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -131,6 +131,8 @@ int submodule_move_head(const char *path,
> const char *new_head,
> unsigned flags);
>
> +void submodule_unset_core_worktree(const struct submodule *sub);
> +
> /*
> * Prepare the "env_array" parameter of a "struct child_process" for executing
> * a submodule by clearing any repo-specific environment variables, but
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 016391723c..51d4555549 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -709,7 +709,8 @@ test_submodule_recursing_with_args_common() {
> git branch -t remove_sub1 origin/remove_sub1 &&
> $command remove_sub1 &&
> test_superproject_content origin/remove_sub1 &&
> - ! test -e sub1
> + ! test -e sub1 &&
> + test_must_fail git config -f .git/modules/sub1/config core.worktree
> )
> '
> # ... absorbing a .git directory along the way.
^ permalink raw reply
* Re: [PATCH] mailmap: update brandon williams's email address
From: Brandon Williams @ 2018-12-08 6:51 UTC (permalink / raw)
To: gitster; +Cc: sbeller, jrnieder, git
In-Reply-To: <xmqq5zw48s9q.fsf@gitster-ct.c.googlers.com>
On Fri, Dec 7, 2018 at 10:08 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Beller <sbeller@google.com> writes:
>
> > On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
> >>
> >> Brandon Williams wrote:
> >>
> >> > Signed-off-by: Brandon Williams <bwilliams.eng@gmail.com>
> >> > ---
> >> > .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.
>
>
^ permalink raw reply
* Re: [PATCH 3/4] submodule--helper: fix BUG message in ensure_core_worktree
From: Junio C Hamano @ 2018-12-08 6:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20181207235425.128568-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Shortly after f178c13fda (Revert "Merge branch
> 'sb/submodule-core-worktree'", 2018-09-07), we had another series
> that implemented partially the same, ensuring that core.worktree was
> set in a checked out submodule, namely 74d4731da1 (submodule--helper:
> replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13)
>
> As the series 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c',
> 2018-09-17) has different goals than the reverted series 7e25437d35
> (Merge branch 'sb/submodule-core-worktree', 2018-07-18), I'd wanted to
> replay the series on top of it to reach the goal of having `core.worktree`
> correctly set when the submodules worktree is present, and unset when the
> worktree is not present.
>
> The replay resulted in a strange merge conflict highlighting that
> the BUG message was not changed in 74d4731da1 (submodule--helper:
> replace connect-gitdir-workingtree by ensure-core-worktree, 2018-08-13).
>
> Fix the error message.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
Unlike the step 2/4 I commented on, this does explain what this
wants to do and why, at least when looked from sideways. Is the
above saying the same as the following two-liner?
An ealier mistake while rebasing to produce 74d4731da1
failed to update this BUG message. Fix this.
> builtin/submodule--helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index d38113a31a..31ac30cf2f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2045,7 +2045,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
> struct repository subrepo;
>
> if (argc != 2)
> - BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
> + BUG("submodule--helper ensure-core-worktree <path>");
>
> path = argv[1];
^ permalink raw reply
* Re: [PATCH 4/4] submodule deinit: unset core.worktree
From: Junio C Hamano @ 2018-12-08 7:03 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20181207235425.128568-5-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This re-introduces 984cd77ddb (submodule deinit: unset core.worktree,
> 2018-06-18), which was reverted as part of f178c13fda (Revert "Merge
> branch 'sb/submodule-core-worktree'", 2018-09-07)
>
> The whole series was reverted as the offending commit e98317508c
> (submodule: ensure core.worktree is set after update, 2018-06-18)
> was relied on by other commits such as 984cd77ddb.
>
> Keep the offending commit reverted, but its functionality came back via
> 4d6d6ef1fc (Merge branch 'sb/submodule-update-in-c', 2018-09-17), such
> that we can reintroduce 984cd77ddb now.
None of the above three explains the most important thing directly,
so readers fail to grasp what the main theme of the three-patch
series is, without looking at the commits that were reverted
already.
Is the theme of the overall series to make sure core.worktree is set
to point at the working tree when submodule's working tree is
instantiated, and unset it when it is not?
2/4 was also explained (in the original) that it wants to unset and
did so when "move_head" gets called. This one does the unset when a
submodule is deinited. Are these the only two cases a submodule
loses its working tree? If so, the log message for this step should
declare victory by ending with something like
... as we covered the only other case in which a submodule
loses its working tree in the earlier step (i.e. switching
branches of top-level project to move to a commit that did
not have the submodule), this makes the code always maintain
core.worktree correctly unset when there is no working tree
for a submodule.
Thanks. I think I agree with what the series wants to do (if I read
what it wants to do correctly, that is).
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> builtin/submodule--helper.c | 2 ++
> t/lib-submodule-update.sh | 2 +-
> t/t7400-submodule-basic.sh | 5 +++++
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 31ac30cf2f..672b74db89 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1131,6 +1131,8 @@ static void deinit_submodule(const char *path, const char *prefix,
> if (!(flags & OPT_QUIET))
> printf(format, displaypath);
>
> + submodule_unset_core_worktree(sub);
> +
> strbuf_release(&sb_rm);
> }
>
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 51d4555549..5b56b23166 100755
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -235,7 +235,7 @@ reset_work_tree_to_interested () {
> then
> mkdir -p submodule_update/.git/modules/sub1/modules &&
> cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
> - GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1/modules/sub2 config --unset core.worktree
> + # core.worktree is unset for sub2 as it is not checked out
> fi &&
> # indicate we are interested in the submodule:
> git -C submodule_update config submodule.sub1.url "bogus" &&
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 76a7cb0af7..aba2d4d6ee 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -984,6 +984,11 @@ test_expect_success 'submodule deinit should remove the whole submodule section
> rmdir init
> '
>
> +test_expect_success 'submodule deinit should unset core.worktree' '
> + test_path_is_file .git/modules/example/config &&
> + test_must_fail git config -f .git/modules/example/config core.worktree
> +'
> +
> test_expect_success 'submodule deinit from subdirectory' '
> git submodule update --init &&
> git config submodule.example.foo bar &&
^ permalink raw reply
* Re: Difficulty with parsing colorized diff output
From: Jeff King @ 2018-12-08 7:16 UTC (permalink / raw)
To: George King; +Cc: Git Mailing List
In-Reply-To: <799879BD-A2F0-487C-AA05-8054AC62C5BD@gmail.com>
On Fri, Dec 07, 2018 at 07:09:58PM -0500, George King wrote:
> My goal is to detect SGR color sequences, e.g. '\x1b[32m', that exist
> in the source text, and have my highlighter print escaped
> representations of those. For example, I have checked in files that
> are expected test outputs for tools that emit color codes, and diffs
> of those get very confusing.
>
> Figuring out which color codes are from the source text and which were
> added by git is proving very difficult. The obvious solution is to
> turn git diff coloring off, but as far as I can see this also turns
> off all coloring for logs, which is undesirable.
Another gotcha that I don't think you've hit yet: whitespace
highlighting can color arbitrary parts of the line.
Try this, for example:
printf 'foo\n' >old
printf '\t \tfoo\n' >new
git diff --no-index old new
So I'm not sure what you want to do can ever be completely robust. That
said, I'm not opposed to making Git's color output more regular. It
seems like a reasonable cleanup on its own.
(For the Git project itself, we long ago realized that putting raw color
codes into the source is a big pain when working with diffs, and we
instead use tools like t/test-lib-functions.sh's test_decode_color).
> * Context lines do not begin with reset code, but do end with a reset
> code. It would be preferable in my opinion if they had both (like
> every other line), or none at all.
Context lines do have both. It's just that the default color for context
lines is empty. ;)
But yes, I think it might be reasonable to recognize when an opening
color is empty, and turn the closing reset into a noop in that case (or
I guess you are also advocating the opposite: turn an empty color into a
noop \x1b[m or similar).
I think most of the coloring, including context lines, is coming from
diff.c:emit_diff_symbol_from_struct(). Instead of unconditionally
calling:
context = diff_get_color_opt(o, DIFF_CONTEXT);
reset = diff_get_color_opt(o, DIFF_RESET);
I suspect we could have a helper like this:
static const char *diff_get_reset(const char *color)
{
if (!color || !*color)
return "";
return diff_colors[DIFF_RESET];
}
...
context = diff_get_color_opt(o, DIFF_CONTEXT);
reset = diff_get_reset(context);
> * Added lines have excess codes after the plus sign. The entire prefix
> is, `\x1b[32m+\x1b[m\x1b[32m` translating to GREEN PLUS RESET GREEN.
> Emitting codes after the plus sign makes the parsing more complex and
> idiosyncratic.
Yeah, I've run into this one, too (when working on diff-highlight, which
similarly tries to pass-through Git's existing colors).
It's unfortunately not quite trivial, due to some interactions with
whitespace coloring. See this old patch:
https://public-inbox.org/git/20101109220136.GA5617@sigill.intra.peff.net/
and the followup:
https://public-inbox.org/git/20101118064050.GA12825@sigill.intra.peff.net/
> * Add a config feature to turn on log coloring while leaving diff coloring off.
Yes, the fact that --pretty log coloring is tied to diffs is mostly a
historical artifact. I think it would be OK to introduce a color.log
config option that defaults to the value of color.diff if unset. That
would be backwards compatible, but allow you to set them independently
if you wanted to.
-Peff
^ permalink raw reply
* Re: Retrieving a file in git that was deleted and committed
From: Jeff King @ 2018-12-08 7:29 UTC (permalink / raw)
To: biswaranjan panda; +Cc: Bryan Turner, git
In-Reply-To: <CADHAf1bH5Aaw3-5WvoHawjXUXL9B-YNvh+AYU1fpGbUe=c0E+A@mail.gmail.com>
On Fri, Dec 07, 2018 at 01:50:57PM -0800, biswaranjan panda wrote:
> Thanks Jeff and Bryan! However, I am curious that if there were a way
> to tell git blame to skip a commit (the one which added the file again
> and maybe the one which deleted it originally) while it walks back
> through history, then it should just get back the
> entire history right ?
Not easily. ;)
You can feed a set of revisions to git-blame with the "-S" option, but I
don't offhand know how it handles diffs (I think it would have to still
diff each commit against its parent, since history is non-linear, and a
list is inherently linear). You might want to experiment with that.
Other than that, you can play with git-replace to produce a fake
history, as if the deletion never happened. But note that will affect
all commands, not just one particular blame. It might be a neat way to
play with blame, but I doubt I'd leave the replacement in place in the
long term.
-Peff
^ permalink raw reply
* Re: [PATCH] mailmap: update brandon williams's email address
From: Duy Nguyen @ 2018-12-08 8:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, Jonathan Nieder, bwilliamseng, Git Mailing List,
Brandon Williams
In-Reply-To: <xmqq5zw48s9q.fsf@gitster-ct.c.googlers.com>
On Sat, Dec 8, 2018 at 7:09 AM Junio C Hamano <gitster@pobox.com> 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
^ permalink raw reply
* Re: [PATCH] mailmap: update brandon williams's email address
From: Ævar Arnfjörð Bjarmason @ 2018-12-08 12:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, Jonathan Nieder, bwilliamseng, git, bwilliams.eng
In-Reply-To: <xmqq5zw48s9q.fsf@gitster-ct.c.googlers.com>
On Sat, Dec 08 2018, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Fri, Dec 7, 2018 at 1:40 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>>>
>>> Brandon Williams wrote:
>>>
>>> > Signed-off-by: Brandon Williams <bwilliams.eng@gmail.com>
>>> > ---
>>> > .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)
^ permalink raw reply
* Documentation: update "man git-add" to include "[]"
From: Robert P. J. Day @ 2018-12-08 15:00 UTC (permalink / raw)
To: Git Mailing list
Current "man git-add" emphasizes single letter interactive
shortcut commands with "[]".
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
---
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: patch 6: 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
========================================================================
^ permalink raw reply related
* [PATCH v3 1/1] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: tboegi @ 2018-12-08 15:11 UTC (permalink / raw)
To: git, svnpenn, johannes.schindelin; +Cc: Torsten Bögershausen
In-Reply-To: <5bf18396.1c69fb81.20780.2b1d@mx.google.com>
From: Torsten Bögershausen <tboegi@web.de>
A regression for cygwin users was introduced with commit 05b458c,
"real_path: resolve symlinks by hand".
In the the commit message we read:
The current implementation of real_path uses chdir() in order to resolve
symlinks. Unfortunately this isn't thread-safe as chdir() affects a
process as a whole...
The old (and non-thread-save) OS calls chdir()/pwd() had been
replaced by a string operation.
The cygwin layer "knows" that "C:\cygwin" is an absolute path,
but the new string operation does not.
"git clone <url> C:\cygwin\home\USER\repo" fails like this:
fatal: Invalid path '/home/USER/repo/C:\cygwin\home\USER\repo'
The solution is to implement has_dos_drive_prefix(), skip_dos_drive_prefix()
is_dir_sep(), offset_1st_component() and convert_slashes() for cygwin
in the same way as it is done in 'Git for Windows' in compat/mingw.[ch]
Extract the needed code into compat/win32/path-utils.[ch] and use it
for cygwin as well.
Reported-by: Steven Penny <svnpenn@gmail.com>
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Changes since V2:
- Settled on a better name:
The common code is in compat/win32/path-utils.c/h
- Skip the 2 patches which "only" do a cleanup (for a moment)
put those cleanups onto the "todo stack".
- The "DOS" moniker is still used for 2 reasons:
Windows inherited the "drive letter" concept from DOS,
and everybody (tm) familar with the code and the path handling
in Git is used to that wording.
Even if there was a better name, it needed to be addressed
in a patch series different from this one.
Here I want to fix a reported regression.
And, before any cleanup is done, I sould like to ask if anybody
can build the code with VS and confirm that it works, please ?
Thanks for the reviews, testing and comment.
compat/cygwin.c | 19 -------------------
compat/cygwin.h | 2 --
compat/mingw.c | 29 +----------------------------
compat/mingw.h | 20 --------------------
compat/win32/path-utils.c | 28 ++++++++++++++++++++++++++++
compat/win32/path-utils.h | 20 ++++++++++++++++++++
config.mak.uname | 3 ++-
git-compat-util.h | 3 ++-
8 files changed, 53 insertions(+), 71 deletions(-)
delete mode 100644 compat/cygwin.c
delete mode 100644 compat/cygwin.h
create mode 100644 compat/win32/path-utils.c
create mode 100644 compat/win32/path-utils.h
diff --git a/compat/cygwin.c b/compat/cygwin.c
deleted file mode 100644
index b9862d606d..0000000000
--- a/compat/cygwin.c
+++ /dev/null
@@ -1,19 +0,0 @@
-#include "../git-compat-util.h"
-#include "../cache.h"
-
-int cygwin_offset_1st_component(const char *path)
-{
- const char *pos = path;
- /* unc paths */
- if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
- /* skip server name */
- pos = strchr(pos + 2, '/');
- if (!pos)
- return 0; /* Error: malformed unc path */
-
- do {
- pos++;
- } while (*pos && pos[0] != '/');
- }
- return pos + is_dir_sep(*pos) - path;
-}
diff --git a/compat/cygwin.h b/compat/cygwin.h
deleted file mode 100644
index 8e52de4644..0000000000
--- a/compat/cygwin.h
+++ /dev/null
@@ -1,2 +0,0 @@
-int cygwin_offset_1st_component(const char *path);
-#define offset_1st_component cygwin_offset_1st_component
diff --git a/compat/mingw.c b/compat/mingw.c
index 34b3880b29..27e397f268 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -350,7 +350,7 @@ static inline int needs_hiding(const char *path)
return 0;
/* We cannot use basename(), as it would remove trailing slashes */
- mingw_skip_dos_drive_prefix((char **)&path);
+ win_path_utils_skip_dos_drive_prefix((char **)&path);
if (!*path)
return 0;
@@ -2275,33 +2275,6 @@ pid_t waitpid(pid_t pid, int *status, int options)
return -1;
}
-int mingw_skip_dos_drive_prefix(char **path)
-{
- int ret = has_dos_drive_prefix(*path);
- *path += ret;
- return ret;
-}
-
-int mingw_offset_1st_component(const char *path)
-{
- char *pos = (char *)path;
-
- /* unc paths */
- if (!skip_dos_drive_prefix(&pos) &&
- is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
- /* skip server name */
- pos = strpbrk(pos + 2, "\\/");
- if (!pos)
- return 0; /* Error: malformed unc path */
-
- do {
- pos++;
- } while (*pos && !is_dir_sep(*pos));
- }
-
- return pos + is_dir_sep(*pos) - path;
-}
-
int xutftowcsn(wchar_t *wcs, const char *utfs, size_t wcslen, int utflen)
{
int upos = 0, wpos = 0;
diff --git a/compat/mingw.h b/compat/mingw.h
index 8c24ddaa3e..30d9fb3e36 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -443,32 +443,12 @@ HANDLE winansi_get_osfhandle(int fd);
* git specific compatibility
*/
-#define has_dos_drive_prefix(path) \
- (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
-int mingw_skip_dos_drive_prefix(char **path);
-#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
-static inline int mingw_is_dir_sep(int c)
-{
- return c == '/' || c == '\\';
-}
-#define is_dir_sep mingw_is_dir_sep
-static inline char *mingw_find_last_dir_sep(const char *path)
-{
- char *ret = NULL;
- for (; *path; ++path)
- if (is_dir_sep(*path))
- ret = (char *)path;
- return ret;
-}
static inline void convert_slashes(char *path)
{
for (; *path; path++)
if (*path == '\\')
*path = '/';
}
-#define find_last_dir_sep mingw_find_last_dir_sep
-int mingw_offset_1st_component(const char *path);
-#define offset_1st_component mingw_offset_1st_component
#define PATH_SEP ';'
extern char *mingw_query_user_email(void);
#define query_user_email mingw_query_user_email
diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c
new file mode 100644
index 0000000000..6cb9a6a745
--- /dev/null
+++ b/compat/win32/path-utils.c
@@ -0,0 +1,28 @@
+#include "../../git-compat-util.h"
+
+int win_path_utils_skip_dos_drive_prefix(char **path)
+{
+ int ret = has_dos_drive_prefix(*path);
+ *path += ret;
+ return ret;
+}
+
+int win_path_utils_offset_1st_component(const char *path)
+{
+ char *pos = (char *)path;
+
+ /* unc paths */
+ if (!skip_dos_drive_prefix(&pos) &&
+ is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
+ /* skip server name */
+ pos = strpbrk(pos + 2, "\\/");
+ if (!pos)
+ return 0; /* Error: malformed unc path */
+
+ do {
+ pos++;
+ } while (*pos && !is_dir_sep(*pos));
+ }
+
+ return pos + is_dir_sep(*pos) - path;
+}
diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
new file mode 100644
index 0000000000..c931b2a890
--- /dev/null
+++ b/compat/win32/path-utils.h
@@ -0,0 +1,20 @@
+#define has_dos_drive_prefix(path) \
+ (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+int win_path_utils_skip_dos_drive_prefix(char **path);
+#define skip_dos_drive_prefix win_path_utils_skip_dos_drive_prefix
+static inline int win_path_utils_is_dir_sep(int c)
+{
+ return c == '/' || c == '\\';
+}
+#define is_dir_sep win_path_utils_is_dir_sep
+static inline char *win_path_utils_find_last_dir_sep(const char *path)
+{
+ char *ret = NULL;
+ for (; *path; ++path)
+ if (is_dir_sep(*path))
+ ret = (char *)path;
+ return ret;
+}
+#define find_last_dir_sep win_path_utils_find_last_dir_sep
+int win_path_utils_offset_1st_component(const char *path);
+#define offset_1st_component win_path_utils_offset_1st_component
diff --git a/config.mak.uname b/config.mak.uname
index 3ee7da0e23..60876e26f4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -187,7 +187,7 @@ ifeq ($(uname_O),Cygwin)
UNRELIABLE_FSTAT = UnfortunatelyYes
OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
MMAP_PREVENTS_DELETE = UnfortunatelyYes
- COMPAT_OBJS += compat/cygwin.o
+ COMPAT_OBJS += compat/win32/path-utils.o
FREAD_READS_DIRECTORIES = UnfortunatelyYes
endif
ifeq ($(uname_S),FreeBSD)
@@ -527,6 +527,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
+ compat/win32/path-utils.o \
compat/win32/pthread.o compat/win32/syslog.o \
compat/win32/dirent.o
BASIC_CFLAGS += -DWIN32 -DPROTECT_NTFS_DEFAULT=1
diff --git a/git-compat-util.h b/git-compat-util.h
index 09b0102cae..5702556c89 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -193,10 +193,11 @@
#endif
#if defined(__CYGWIN__)
-#include "compat/cygwin.h"
+#include "compat/win32/path-utils.h"
#endif
#if defined(__MINGW32__)
/* pull in Windows compatibility stuff */
+#include "compat/win32/path-utils.h"
#include "compat/mingw.h"
#elif defined(_MSC_VER)
#include "compat/msvc.h"
--
2.19.0.271.gfe8321ec05
^ permalink raw reply related
* [wishlist] submodule.update config
From: Yaroslav Halchenko @ 2018-12-08 15:45 UTC (permalink / raw)
To: git
Relates (but orthogonal) to my other thread
[wishlist] git submodule update --reset-hard
ATM, it possible to specify per submodule update strategy via
configuration variable submodule.SUBMODULE.update where SUBMODULE is the name
of the corresponding submodule. But I see no way to specify default update
strategy for all submodules.
From our conversation in that other thread I have discovered to myself about
existence of submodule.recurse configuration, and there seems to be a few
more (.fetchJobs, .active) where e.g. .active seems to complement per-submodule
submodule.*.active:
yoh@debian:~/proj/misc/git$ git grep '[^.]submodule\.[a-z]' -- Documentation/
Documentation/RelNotes/2.14.0.txt: * Many commands learned to pay attention to submodule.recurse
Documentation/RelNotes/2.15.0.txt: * "git -c submodule.recurse=yes pull" did not work as if the
Documentation/config.txt:include::config/submodule.txt[]
Documentation/config/submodule.txt: update'. If neither submodule.<name>.active or submodule.active are
Documentation/config/submodule.txt: interact with submodules; settings like `submodule.active`
Documentation/config/submodule.txt: submodule.active config option. See linkgit:gitsubmodules[7] for
Documentation/config/submodule.txt: as computed via `submodule.alternateLocation`. Possible values are
Documentation/git-clone.txt: of multiple entries. The resulting clone has `submodule.active` set to
Documentation/git-clone.txt: Defaults to the `submodule.fetchJobs` option.
Documentation/git-submodule.txt:If no path is specified and submodule.active has been configured, submodules
Documentation/git-submodule.txt: Defaults to the `submodule.fetchJobs` option.
Documentation/gitsubmodules.txt:`submodule.foo.path = path/to/bar`.
Documentation/gitsubmodules.txt:The section `submodule.foo.*` in the `.gitmodules` file gives additional
Documentation/gitsubmodules.txt:hints to Git's porcelain layer. For example, the `submodule.foo.url`
Documentation/gitsubmodules.txt: b. if the submodule's path matches the pathspec in `submodule.active`
Documentation/gitsubmodules.txt:submodule's path is excluded in the pathspec in `submodule.active`, the
Documentation/gitsubmodules.txt: git config --global submodule.recurse true
Documentation/gitsubmodules.txt:your working tree. Alternatively you can set 'submodule.recurse' to have
Documentation/technical/api-config.txt:if (!git_configset_get_bool(gm_config, "submodule.frotz.ignore", &b)) {
Documentation/technical/http-protocol.txt: $GIT_URL: http://example.com/git/repo.git/path/submodule.git
Documentation/technical/http-protocol.txt: URL request: http://example.com/git/repo.git/path/submodule.git/info/refs
I wondered, if you think it would be sensible to also add of
submodule.update which would be considered before submodule.SUBMODULE.update
variable possibly defined per submodule. That would be more inline with desire
to use any of the --merge, --rebase (and hopefully soon --reset-hard)
strategies specified as an option for submodule update, where no per-submodule
handling is happening.
Thanks in advance for the consideration!
--
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
^ permalink raw reply
* why doesn't "git reset" mention optional pathspec?
From: Robert P. J. Day @ 2018-12-08 16:05 UTC (permalink / raw)
To: Git Mailing list
from "man git-reset":
SYNOPSIS
git reset [-q] [<tree-ish>] [--] <paths>...
git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
oddly, the third form says nothing about possible "<paths>", even
though i'm pretty sure they're valid in that third case (at least for
"--mixed"). thoughts? is that just an oversight in the man page?
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* Re: "git add -p" versus "git add -i", followed by "p"
From: Robert P. J. Day @ 2018-12-08 16:22 UTC (permalink / raw)
To: Duy Nguyen; +Cc: SZEDER Gábor, Git Mailing List
In-Reply-To: <CACsJy8AkMfZ02b9p2sQi2p=Bw4MDckLYy_cBFVeN2_UY-Z3kCg@mail.gmail.com>
On Sun, 2 Dec 2018, Duy Nguyen wrote:
> On Sun, Dec 2, 2018 at 6:05 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > > Patch update>> 2
> > > staged unstaged path
> > > * 1: unchanged +1/-0 README.md
> > > * 2: unchanged +1/-0 contrib/README
> > > 3: unchanged +1/-0 t/README
> > > Patch update>>
> > >
> > > Here I hit enter. Did you?
> >
> > perhaps i'm just not seeing it, but from "man git-add", it
> > doesn't seem obvious that you would first select the files to work
> > with, then hit a simple CR to get into actual patch mode.
>
> I think it's the same procedure as the "update" step, which
> describes this in more detail. I agree that the "patch" section does
> not make this obvious.
when i get a chance, i'll try to reword it.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* Re: [PATCH v3 1/1] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Steven Penny @ 2018-12-08 16:24 UTC (permalink / raw)
To: tboegi; +Cc: git
In-Reply-To: <20181208151109.2097-1-tboegi@web.de>
On Sat, Dec 8, 2018 at 9:11 AM wrote:
> Changes since V2:
latest patch still fixes original issue - thanks
> - Settled on a better name:
> The common code is in compat/win32/path-utils.c/h
> [...]
> - The "DOS" moniker is still used for 2 reasons:
> Windows inherited the "drive letter" concept from DOS,
> and everybody (tm) familar with the code and the path handling
> in Git is used to that wording.
> Even if there was a better name, it needed to be addressed
> in a patch series different from this one.
> Here I want to fix a reported regression.
i still disagree with this - but i understand if naming argument is out of scope
for thread
> And, before any cleanup is done, I sould like to ask if anybody
> can build the code with VS and confirm that it works, please ?
sorry but i am decidedly *not* interested in doing this. i use cygwin
specifically so that i can avoid VS. hopefully someone else will be able to
test. cheers
^ permalink raw reply
* [PATCH v4 0/7] %(trailers) improvements in pretty format
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181028125025.30952-1-anders@0x63.nu>
Updated since v3:
* multiple 'key=' matches any
* allow overriding implicit 'only' when using key
* minor grammar and spelling fixes
* documentation restructuring
* Helper functions for parsing options
Anders Waldenborg (7):
doc: group pretty-format.txt placeholders descriptions
pretty: allow %(trailers) options with explicit value
pretty: single return path in %(trailers) handling
pretty: allow showing specific trailers
pretty: add support for "valueonly" option in %(trailers)
strbuf: separate callback for strbuf_expand:ing literals
pretty: add support for separator option in %(trailers)
Documentation/pretty-formats.txt | 260 ++++++++++++++++++-------------
pretty.c | 104 ++++++++++---
strbuf.c | 21 +++
strbuf.h | 8 +
t/t4205-log-pretty-formats.sh | 111 +++++++++++++
trailer.c | 25 ++-
trailer.h | 4 +
7 files changed, 400 insertions(+), 133 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
In addition to old %(trailers:only) it is now allowed to write
%(trailers:only=yes)
By itself this only gives (the not quite so useful) possibility to have
users change their mind in the middle of a formatting
string (%(trailers:only=true,only=false)). However, it gives users the
opportunity to override defaults from future options.
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 14 ++++++++++----
pretty.c | 32 +++++++++++++++++++++++++++-----
t/t4205-log-pretty-formats.sh | 18 ++++++++++++++++++
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86d804fe97..d33b072eb2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,10 +225,16 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
-** 'only': omit non-trailer lines from the trailer block.
-** 'unfold': make it behave as if interpret-trailer's `--unfold`
- option was given. E.g., `%(trailers:only,unfold)` unfolds and
- shows all trailer lines.
+** 'only[=val]': select whether non-trailer lines from the trailer
+ block should be included. The `only` keyword may optionally be
+ followed by an equal sign and one of `true`, `on`, `yes` to omit or
+ `false`, `off`, `no` to show the non-trailer lines. If option is
+ given without value it is enabled. If given multiple times the last
+ value is used.
+** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
+ option was given. In same way as to for `only` it can be followed
+ by an equal sign and explicit value. E.g.,
+ `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index b83a3ecd23..26efdba73a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,31 @@ static int match_placeholder_arg(const char *to_parse, const char *candidate,
return 0;
}
+static int match_placeholder_bool_arg(const char *to_parse, const char *candidate,
+ const char **end, int *val)
+{
+ const char *p;
+ if (!skip_prefix(to_parse, candidate, &p))
+ return 0;
+
+ if (match_placeholder_arg(p, "=no", end) ||
+ match_placeholder_arg(p, "=off", end) ||
+ match_placeholder_arg(p, "=false", end)) {
+ *val = 0;
+ return 1;
+ }
+
+ if (match_placeholder_arg(p, "", end) ||
+ match_placeholder_arg(p, "=yes", end) ||
+ match_placeholder_arg(p, "=on", end) ||
+ match_placeholder_arg(p, "=true", end)) {
+ *val = 1;
+ return 1;
+ }
+
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1318,11 +1343,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (match_placeholder_arg(arg, "only", &arg))
- opts.only_trailers = 1;
- else if (match_placeholder_arg(arg, "unfold", &arg))
- opts.unfold = 1;
- else
+ if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
+ !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66ff..63730a4ec0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -578,6 +578,24 @@ test_expect_success '%(trailers:only) shows only "key: value" trailers' '
test_cmp expect actual
'
+test_expect_success '%(trailers:only=yes) shows only "key: value" trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=yes)" >actual &&
+ grep -v patch.description <trailers >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:only=no) shows all trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=no)" >actual &&
+ cat trailers >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:only=no,only=true) shows only "key: value" trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=yes)" >actual &&
+ grep -v patch.description <trailers >expect &&
+ test_cmp expect actual
+'
+
test_expect_success '%(trailers:unfold) unfolds trailers' '
git log --no-walk --pretty="%(trailers:unfold)" >actual &&
{
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox