* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: brian m. carlson @ 2017-01-25 23:19 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125213544.eelk4pjhrhshi6zh@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 938 bytes --]
On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
>
> > > The need for the extensions could be replaced with a small amount of
> > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > so were negative, however.
> >
> > Quite frankly, it is annoying to be forced to install the extensions. I
> > would much rather have the small amount of Ruby code in Git's repository.
>
> Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> I saw the actual Ruby code, though. :)
I've sent the patch before, but I can send it again. It's relatively
small and self-contained. I'm also happy to be responsible for
maintaining it.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH v2 25/27] attr: store attribute stack in attr_check structure
From: Brandon Williams @ 2017-01-25 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, pclouds
In-Reply-To: <20170125215426.GB83343@google.com>
On 01/25, Brandon Williams wrote:
> On 01/25, Junio C Hamano wrote:
> > Brandon Williams <bmwill@google.com> writes:
> >
> > >> In my mind, the value of having a constant check_attr is primarily
> > >> that it gives us a stable pointer to serve as a hashmap key,
> > >> i.e. the identifier for each call site, in a later iteration.
> > >
> > > We didn't really discuss this notion of having the pointer be a key into
> > > a hashmap, what sort of information are you envisioning being stored in
> > > this sort of hashmap?
> >
> > The "entries relevant to this attr_check() call, that is specific to
> > the <check_attr instance, the thread> tuple" (aka "what used to be
> > called the global attr_stack") we discussed would be the primary
> > example. A thread is likely be looping in a caller that has many
> > paths inside a directory, calling a function that has a call to
> > attr_check() for each path. Having something that can use to
> > identify the check_attr instance in a stable way, even when the
> > inner function is called and returns many times, would allow us to
> > populate the "attr stack" just once for the thread when it enters a
> > directory for the first time (remember, another thread may be
> > executing the same codepath, checking for paths in a different
> > directory) and keep using it. There may be other mechanisms you can
> > come up with, so I wouldn't say it is the only valid way, but it is
> > a way. That is why I said:
> >
> > >> But all of the above comes from my intuition, and I'll very much
> > >> welcome to be proven wrong with an alternative design, or better
> > >> yet, a working code based on an alternative design ;-).
> >
> > near the end of my message.
> >
> > > One issue I can see with this is that the
> > > functions which have a static attr_check struct would still not be thread
> > > safe if the initialization of the structure isn't surrounded by a mutex
> > > itself. ie
> >
> > Yes, that goes without saying. That is why I suggested Stefan to do
> > not this:
> >
> > > static struct attr_check *check;
> > >
> > > if (!check)
> > > init(check);
> > >
> > > would need to be:
> > >
> > > lock()
> > > if (!check)
> > > init(check);
> > > unlock();
> >
> > but this:
> >
> > static struct attr_check *check;
> > init(&check);
> >
> > and hide the lock/unlock gymnastics inside the API. I thought that
> > already was in what you inherited from him and started your work
> > on top of?
>
> I essentially built off of the series you had while using Stefan's
> patches as inspiration, but I don't believe the kind of mechanism you
> are describing existed in Stefan's series. His series had a single lock
> for the entire system, only allowing a single caller to be in it at any
> given time. This definitely isn't ideal, hence why I picked it up.
>
> Implementation aside I want to try and nail down what the purpose of
> this refactor is. There are roughly two notions of being "thread-safe".
>
> 1. The first is that the subsystem itself is thread safe, that is
> multiple threads can be executing inside the subsystem without stepping
> on each others work.
>
> 2. The second is that the object itself is thread safe or that multiple
> threads can use the same object.
>
> I thought that the main purpose of this was to achieve (1) since
> currently that is not the case.
Ok, so I discovered a very good reason why we should do as Stefan
originally did and split the question and answer (beyond the reasoning
for using the reference as a hashkey).
One motivation behind making this API thread-safe is that we can use it
in pathspec code to match against attributes. This means that a
pathspec structure will contain an attr_check member describing the
attributes that a pathspec item is interested in. Then the pathspec
structure is passed to match_pathspec() as a const pointer. To me, when
passing something as 'const' I expect none of the members should change
at all. The struct should remain exactly in the same form as before I
invoked the function.
Requiring the attr_check structure to be modified in the process of a
check_attrs() call would violate this "contract" when calling
match_pathspec() as the attr_check structure would have modified state.
The compiler wouldn't catch this as the "const" modifier isn't passed on
to struct members.
--
Brandon Williams
^ permalink raw reply
* Re: diff --color-words breaks spacing
From: Jeff King @ 2017-01-25 23:20 UTC (permalink / raw)
To: Phil Hord; +Cc: Git
In-Reply-To: <CABURp0ryBtLALEnzRdoeYeUUdrzBxiT3+yTvW6B=vpqMG3p1Rw@mail.gmail.com>
On Tue, Jan 24, 2017 at 09:39:31AM -0800, Phil Hord wrote:
> I noticed some weird spacing when comparing files with git diff
> --color-words. The space before a colored word disappears sometimes.
I _think_ this is working as designed, though it is a bit tricky (and it
may be possible to make it better).
> echo "FOO foo; foo = bar" > a
> echo "FOO foo = baz" > b
> git diff --color-words --no-index a b
> FOOfoo; foo = barbaz
It might be easier to see with --word-diff, which uses the same code but
marks it with punctuation:
$ git diff --word-diff
FOO[-foo;-] foo = [-bar-]{+baz+}
The key thing is that what you are seeing is the post-image, plus
word-based annotations. And the post-image does not have that space in
its context, so we would not expect to see:
FOO [-foo;-] foo = [-bar-]{+baz+}
(you would if we showed the pre-image plus annotations; but then I think
you'd probably end up with the opposite problem when text was added).
However, we also don't see the space in the removed part. I.e., I think:
FOO[- foo;-] foo = [-bar-]{+baz+}
would be correct. But I think because of the way word-diff is
implemented, a non-word character will never be part of the annotation.
The way it works is basically to break the string down on word
boundaries, so we have:
pre: FOO, foo;, foo, =, bar
post: FOO, foo, =, baz
as a set of tokens. We stick each of those on their own line and feed
them back to xdiff, so we get a diff like:
@@ -1,5 +1,4 @@
FOO
-foo;
foo
=
-bar
+baz
and then walk through the post-image buffer, either inserting chunks of
context from the original buffer, or the changed lines. But the changed
lines themselves do not include the non-word characters, so we have no
idea that a space went away.
-Peff
^ permalink raw reply
* Re: [PATCH] Retire the `relink` command
From: Eric Wong @ 2017-01-25 23:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <10319c47ff3f7222c3a601827ebd9398861d509d.1485363528.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> Back in the olden days, when all objects were loose and rubber boots were
> made out of wood, it made sense to try to share (immutable) objects
> between repositories.
>
> Ever since the arrival of pack files, it is but an anachronism.
>
> Let's move the script to the contrib/examples/ directory and no longer
> offer it.
On the other hand, we have no idea if there are still people
using it for whatever reason...
I suggest we have a deprecation period where:
1) there is warning message when it's run
2) a note in the manpage indicating it's to-be-removed
3) ...then wait a few distro LTS cycles to remove it entirely
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 23:25 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Segev Finer
In-Reply-To: <20170125224043.jxjc4avuy4ztnkm4@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 25, 2017 at 02:35:36PM -0800, Junio C Hamano wrote:
>
>> -- >8 --
>> Subject: [PATCH] connect: core.sshvariant to correct misidentification
>
> I have been watching this discussion from the sidelines, and I agree
> that this direction is a big improvement.
> ...
> IIRC, the "const" in git_config_get_string_const is only about avoiding
> an annoying cast. The result is still allocated and needs freed. Since
> you are not keeping the value after the function returns, I think you
> could just use git_config_get_value().
It is too late for today's pushout (I have this topic near the tip
of 'pu', so it is possible to tweak and redo only 'pu' branch, but I
generally hate tweaking things after 15:00 my time), but I'll fix
that before the topic hits 'jch' (which is a bit more than 'next'
and that is what I use for everyday work).
Thanks.
^ permalink raw reply
* Re: [PATCH 4/5] revision.c: refactor ref selection handler after --exclude
From: Junio C Hamano @ 2017-01-25 23:25 UTC (permalink / raw)
To: Jacob Keller
Cc: Jeff King, Nguyễn Thái Ngọc Duy,
Git mailing list
In-Reply-To: <CA+P7+xovOx9ebo6MU0e4v==+76jtoMXz45+LnBPFifHbjqFU4w@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> On Wed, Jan 25, 2017 at 1:27 PM, Jeff King <peff@peff.net> wrote:
>> On Wed, Jan 25, 2017 at 03:57:18PM -0500, Jeff King wrote:
>>
>>> IOW, the ref-selector options build up until a group option is given,
>>> which acts on the built-up options (over that group) and then resets the
>>> built-up options. Doing "--unrelated" as above is orthogonal (though I
>>> think in practice nobody would do that, because it's hard to read).
>>
>> So here's what I would have expected your series to look more like (with
>> probably one patch adding clear_ref_selection_options, and the other
>> adding the decorate stuff):
>>
>
> I agree that this is how I would have expected it to work as well.
That makes three of us ;-)
^ permalink raw reply
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: Jeff King @ 2017-01-25 23:30 UTC (permalink / raw)
To: brian m. carlson, Johannes Schindelin, git
In-Reply-To: <20170125231926.usufhlugjotjw5zw@genre.crustytoothpaste.net>
On Wed, Jan 25, 2017 at 11:19:26PM +0000, brian m. carlson wrote:
> On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> > On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
> >
> > > > The need for the extensions could be replaced with a small amount of
> > > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > > so were negative, however.
> > >
> > > Quite frankly, it is annoying to be forced to install the extensions. I
> > > would much rather have the small amount of Ruby code in Git's repository.
> >
> > Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> > I saw the actual Ruby code, though. :)
>
> I've sent the patch before, but I can send it again. It's relatively
> small and self-contained. I'm also happy to be responsible for
> maintaining it.
Ah, it's:
http://public-inbox.org/git/1413070656-241955-5-git-send-email-sandals@crustytoothpaste.net/
(and note there is some surrounding discussion there).
The code is not _too_ bad. The main thing is that it would have to be
kept up to date with changes to asciidoc.conf's version of the linkgit
macro. But it's not like that changes frequently.
-Peff
^ permalink raw reply
* [PATCHv2] submodule update: run custom update script for initial populating as well
From: Stefan Beller @ 2017-01-25 23:37 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, judge.packham, olsonse, Stefan Beller
In-Reply-To: <CAGZ79kbi-cweTe-ydcRYFFnGT4EmN+O1TZDiGYFyg8-Ex8abUw@mail.gmail.com>
In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
2011-02-17), all actions were defaulted to checkout for populating
a submodule initially, because merging or rebasing makes no sense
in that situation.
Other commands however do make sense, such as the custom command
that was added later (6cb5728c43, submodule update: allow custom
command to update submodule working tree, 2013-07-03).
I am unsure about the "none" command, as I can see an initial
checkout there as a useful thing. On the other hand going strictly
by our own documentation, we should do nothing in case of "none"
as well, because the user asked for it.
Reported-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 5 ++++-
t/t7406-submodule-update.sh | 15 +++++++++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9788175979..63fc4fe9bc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -607,7 +607,10 @@ cmd_update()
if test $just_cloned -eq 1
then
subsha1=
- update_module=checkout
+ case "$update_module" in
+ merge | rebase | none)
+ update_module=checkout ;;
+ esac
else
subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 725bbed1f8..2f83243c7d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -441,6 +441,19 @@ test_expect_success 'submodule update - command in .git/config catches failure -
test_i18ncmp actual expect
'
+test_expect_success 'submodule update - command run for initial population of submodule' '
+ cat <<-\ EOF >expect
+ Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
+ EOF
+ (
+ cd super &&
+ rm -rf submodule
+ test_must_fail git submodule update >../actual
+ )
+ test_cmp expect actual
+ git -C super submodule update --checkout
+'
+
cat << EOF >expect
Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
Failed to recurse into submodule path '../super'
@@ -493,6 +506,7 @@ test_expect_success 'submodule init picks up merge' '
'
test_expect_success 'submodule update --merge - ignores --merge for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
@@ -505,6 +519,7 @@ test_expect_success 'submodule update --merge - ignores --merge for new submod
'
test_expect_success 'submodule update --rebase - ignores --rebase for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCHv3 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Brandon Williams @ 2017-01-25 23:39 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, peff
In-Reply-To: <20170125230450.4393-1-sbeller@google.com>
On 01/25, Stefan Beller wrote:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
>
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
>
> An alternative I considered to do this work lazily, i.e. when resolving
> "../.git/modules/nested", we would notice the ".git" being a gitfile
> linking to another path. That seemed to be robuster by design, but harder
> to get the implementation right. Maybe we have to do that anyway once we
> try to have submodules and worktrees working nicely together, but for now
> just produce 'correct' (i.e. direct) pointers.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
Looks good to me!
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] tag: add tag.createReflog option
From: Cornelius Weig @ 2017-01-25 23:40 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: git, novalis, pclouds
In-Reply-To: <xmqqy3xy7nq0.fsf@gitster.mtv.corp.google.com>
>
> It may have been obvious, but to be explicit for somebody new,
> !prefixcmp() corresponds to starts_with(). IOW, we changed the
> meaning of the return value when moving from cmp-lookalike (where 0
> means "equal") to "does it start with this string?" bool (where 1
> means "yes").
>
I see. It reads much better that way!
I re-did all the changes from Jeff's patch, but some tests are breaking
now. I will have to mend that tomorrow, because it's already too late in
my timezone.
Thanks a lot for your support m(_ _)m
^ permalink raw reply
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: brian m. carlson @ 2017-01-25 23:41 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125232959.zdbf3n3ey7qtnv7j@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On Wed, Jan 25, 2017 at 06:30:00PM -0500, Jeff King wrote:
> On Wed, Jan 25, 2017 at 11:19:26PM +0000, brian m. carlson wrote:
>
> > On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> > > On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
> > >
> > > > > The need for the extensions could be replaced with a small amount of
> > > > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > > > so were negative, however.
> > > >
> > > > Quite frankly, it is annoying to be forced to install the extensions. I
> > > > would much rather have the small amount of Ruby code in Git's repository.
> > >
> > > Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> > > I saw the actual Ruby code, though. :)
> >
> > I've sent the patch before, but I can send it again. It's relatively
> > small and self-contained. I'm also happy to be responsible for
> > maintaining it.
>
> Ah, it's:
>
> http://public-inbox.org/git/1413070656-241955-5-git-send-email-sandals@crustytoothpaste.net/
>
> (and note there is some surrounding discussion there).
>
> The code is not _too_ bad. The main thing is that it would have to be
> kept up to date with changes to asciidoc.conf's version of the linkgit
> macro. But it's not like that changes frequently.
Yes. I think I can actually simplify it some more, since we always seem
to use the argument to linkgit, so I'll send out a simplified patch in a
few minutes.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCHv2] submodule update: run custom update script for initial populating as well
From: Brandon Williams @ 2017-01-25 23:41 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git, judge.packham, olsonse
In-Reply-To: <20170125233742.30370-1-sbeller@google.com>
On 01/25, Stefan Beller wrote:
> In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
> 2011-02-17), all actions were defaulted to checkout for populating
> a submodule initially, because merging or rebasing makes no sense
> in that situation.
>
> Other commands however do make sense, such as the custom command
> that was added later (6cb5728c43, submodule update: allow custom
> command to update submodule working tree, 2013-07-03).
>
> I am unsure about the "none" command, as I can see an initial
> checkout there as a useful thing. On the other hand going strictly
> by our own documentation, we should do nothing in case of "none"
> as well, because the user asked for it.
>
> Reported-by: Han-Wen Nienhuys <hanwen@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> git-submodule.sh | 5 ++++-
> t/t7406-submodule-update.sh | 15 +++++++++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 9788175979..63fc4fe9bc 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -607,7 +607,10 @@ cmd_update()
> if test $just_cloned -eq 1
> then
> subsha1=
> - update_module=checkout
> + case "$update_module" in
> + merge | rebase | none)
> + update_module=checkout ;;
> + esac
> else
> subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
> git rev-parse --verify HEAD) ||
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 725bbed1f8..2f83243c7d 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -441,6 +441,19 @@ test_expect_success 'submodule update - command in .git/config catches failure -
> test_i18ncmp actual expect
> '
>
> +test_expect_success 'submodule update - command run for initial population of submodule' '
> + cat <<-\ EOF >expect
> + Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
> + EOF
> + (
> + cd super &&
> + rm -rf submodule
> + test_must_fail git submodule update >../actual
> + )
> + test_cmp expect actual
> + git -C super submodule update --checkout
> +'
You can probably get away without the subshell:
rm -rf super/submodule
test_must_fail git -C super submodule upsate >actual
> +
> cat << EOF >expect
> Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
> Failed to recurse into submodule path '../super'
> @@ -493,6 +506,7 @@ test_expect_success 'submodule init picks up merge' '
> '
>
> test_expect_success 'submodule update --merge - ignores --merge for new submodules' '
> + test_config -C super submodule.submodule.update checkout &&
> (cd super &&
> rm -rf submodule &&
> git submodule update submodule &&
> @@ -505,6 +519,7 @@ test_expect_success 'submodule update --merge - ignores --merge for new submod
> '
>
> test_expect_success 'submodule update --rebase - ignores --rebase for new submodules' '
> + test_config -C super submodule.submodule.update checkout &&
> (cd super &&
> rm -rf submodule &&
> git submodule update submodule &&
> --
> 2.11.0.495.g04f60290a0.dirty
>
--
Brandon Williams
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Junio C Hamano @ 2017-01-25 23:46 UTC (permalink / raw)
To: Jacob Keller; +Cc: Hilco Wijbenga, Git Users
In-Reply-To: <CA+P7+xo9WBwHjAXeUTn4bh=F6hvw1gA-79h-GmwQoeRpeLj2jQ@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
>> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
>> my example as simple as possible. In reality, it's Maven's POM files
>> (pom.xml).
>>
>> So there is no setting for any of this? There is no way to switch off
>> auto merging for certain files?
>
> Not really sure, but a quick google search revealed
> https://github.com/ralfth/pom-merge-driver
>
> Maybe that will help you?
Its readme seems to indicate that it is either solving a different
problem, or solving the same problem with the opposite goal in mind,
in that its goal seems to be to forcibly resolve what textually does
not resolve cleanly by choosing sides with an arbitrary policy, so
that humans do not have to get involved when they ordinarily would
have to.
Hilco's goal sounded to me the opposite---to force conflict even
when the two histories did what textually does resolve cleanly and
require humans to get involved even when they ordinarily wouldn't
have to.
^ permalink raw reply
* [PATCHv3] submodule update: run custom update script for initial populating as well
From: Stefan Beller @ 2017-01-25 23:48 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, judge.packham, olsonse, Stefan Beller
In-Reply-To: <20170125234158.GE83343@google.com>
In 1b4735d9f3 (submodule: no [--merge|--rebase] when newly cloned,
2011-02-17), all actions were defaulted to checkout for populating
a submodule initially, because merging or rebasing makes no sense
in that situation.
Other commands however do make sense, such as the custom command
that was added later (6cb5728c43, submodule update: allow custom
command to update submodule working tree, 2013-07-03).
I am unsure about the "none" command, as I can see an initial
checkout there as a useful thing. On the other hand going strictly
by our own documentation, we should do nothing in case of "none"
as well, because the user asked for it.
Reported-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Thanks, Brandon, for spotting the unneeded subshell.
I also fixed the && chaining along the way.
Thanks,
Stefan
git-submodule.sh | 5 ++++-
t/t7406-submodule-update.sh | 12 ++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9788175979..63fc4fe9bc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -607,7 +607,10 @@ cmd_update()
if test $just_cloned -eq 1
then
subsha1=
- update_module=checkout
+ case "$update_module" in
+ merge | rebase | none)
+ update_module=checkout ;;
+ esac
else
subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 725bbed1f8..347857fa7c 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -441,6 +441,16 @@ test_expect_success 'submodule update - command in .git/config catches failure -
test_i18ncmp actual expect
'
+test_expect_success 'submodule update - command run for initial population of submodule' '
+ cat <<-\ EOF >expect
+ Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
+ EOF &&
+ rm -rf super/submodule &&
+ test_must_fail git -C super submodule update >../actual &&
+ test_cmp expect actual &&
+ git -C super submodule update --checkout
+'
+
cat << EOF >expect
Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
Failed to recurse into submodule path '../super'
@@ -493,6 +503,7 @@ test_expect_success 'submodule init picks up merge' '
'
test_expect_success 'submodule update --merge - ignores --merge for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
@@ -505,6 +516,7 @@ test_expect_success 'submodule update --merge - ignores --merge for new submod
'
test_expect_success 'submodule update --rebase - ignores --rebase for new submodules' '
+ test_config -C super submodule.submodule.update checkout &&
(cd super &&
rm -rf submodule &&
git submodule update submodule &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Mike Hommey @ 2017-01-25 23:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqtw8m7ncp.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 25, 2017 at 03:04:38PM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > For instance, after changing my laptop for a new one, I copied my
> > configs, but had some environment differences that broke gpg.
> > With this change applied, the output becomes, on this new machine:
> > gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
> > such file or directory
> > error: gpg failed to sign the data
> > error: unable to sign the tag
> >
> > which makes it clearer what's wrong.
>
> Overall I think this is a good thing to do. Instead of eating the
> status output, showing what we got, especially when we know the
> command failed, would make the bug-hunting of user's environment
> easier, like you showed above.
>
> The only thing in the design that makes me wonder is the filtering
> out based on "[GNUPG:]" prefix. Why do we need to filter them out?
The [GNUPG:] lines are part of the status-fd protocol. They show details
that don't really seem interesting to the user. In fact, they even
contain the signed message (yes, in my case, it turns out gpg was
actually still signing, but returned an error code...).
For instance, in that failed run above, the entire output looks like:
gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
such file or directory
[GNUPG:] ERROR add_keyblock_resource ...
[GNUPG:] KEY_CONSIDERED ...
[GNUPG:] BEGIN_SIGNING H2
[GNUPG:] SIG_CREATED ...
All the [GNUPG:] lines are implementation details that don't seem
particularly useful. A case could be made for the ERROR one, though,
although it's also implementation detail-y.
> Implementation-wise, I'd be happier if we do not add any new
> callsites of strbuf_split(), which is a horrible interface. But
> that is a minor detail.
What would you suggest otherwise?
Mike
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-25 23:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git Users
In-Reply-To: <xmqqh94m7leb.fsf@gitster.mtv.corp.google.com>
On 25 January 2017 at 15:46, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>>> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
>>> my example as simple as possible. In reality, it's Maven's POM files
>>> (pom.xml).
>>>
>>> So there is no setting for any of this? There is no way to switch off
>>> auto merging for certain files?
>>
>> Not really sure, but a quick google search revealed
>> https://github.com/ralfth/pom-merge-driver
>>
>> Maybe that will help you?
>
> Its readme seems to indicate that it is either solving a different
> problem, or solving the same problem with the opposite goal in mind,
> in that its goal seems to be to forcibly resolve what textually does
> not resolve cleanly by choosing sides with an arbitrary policy, so
> that humans do not have to get involved when they ordinarily would
> have to.
>
> Hilco's goal sounded to me the opposite---to force conflict even
> when the two histories did what textually does resolve cleanly and
> require humans to get involved even when they ordinarily wouldn't
> have to.
Yes, unfortunately, you are correct. This seems to do the exact
opposite of what I want.
Before I start learning about custom merge drivers, is what I want
even possible? If yes, would you happen to know some good examples of
(custom) merge drivers? (Python, Ruby, Java are all okay.)
^ permalink raw reply
* [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: brian m. carlson @ 2017-01-26 0:13 UTC (permalink / raw)
To: git; +Cc: Jeff King, Johannes Schindelin, Øyvind A. Holm
In-Reply-To: <20170125234101.n2pzrp77df4zycv7@genre.crustytoothpaste.net>
AsciiDoc uses a configuration file to implement macros like linkgit,
while Asciidoctor uses Ruby extensions. Implement a Ruby extension that
implements the linkgit macro for Asciidoctor in the same way that
asciidoc.conf does for AsciiDoc. Adjust the Makefile to use it by
default.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
Documentation/Makefile | 5 +----
Documentation/asciidoctor-extensions.rb | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 4 deletions(-)
create mode 100644 Documentation/asciidoctor-extensions.rb
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 19c42eb60..d1b7a6865 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -179,10 +179,7 @@ ASCIIDOC = asciidoctor
ASCIIDOC_CONF =
ASCIIDOC_HTML = xhtml5
ASCIIDOC_DOCBOOK = docbook45
-ifdef ASCIIDOCTOR_EXTENSIONS_LAB
-ASCIIDOC_EXTRA = -I$(ASCIIDOCTOR_EXTENSIONS_LAB) -rasciidoctor/extensions -rman-inline-macro
-endif
-ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions -alitdd='&\#x2d;&\#x2d;'
DBLATEX_COMMON =
endif
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
new file mode 100644
index 000000000..09f7088ee
--- /dev/null
+++ b/Documentation/asciidoctor-extensions.rb
@@ -0,0 +1,28 @@
+require 'asciidoctor'
+require 'asciidoctor/extensions'
+
+module Git
+ module Documentation
+ class LinkGitProcessor < Asciidoctor::Extensions::InlineMacroProcessor
+ use_dsl
+
+ named :chrome
+
+ def process(parent, target, attrs)
+ if parent.document.basebackend? 'html'
+ prefix = parent.document.attr('git-relative-html-prefix')
+ %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>\n)
+ elsif parent.document.basebackend? 'docbook'
+ %(<citerefentry>
+<refentrytitle>#{target}</refentrytitle><manvolnum>#{attrs[1]}</manvolnum>
+</citerefentry>
+)
+ end
+ end
+ end
+ end
+end
+
+Asciidoctor::Extensions.register do
+ inline_macro Git::Documentation::LinkGitProcessor, :linkgit
+end
--
2.11.0
^ permalink raw reply related
* Re: merge maintaining history
From: David J. Bakeman @ 2017-01-26 0:31 UTC (permalink / raw)
To: Jakub Narębski, Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <38ca43cb-2fc7-0448-352f-7d9413f815c5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3731 bytes --]
On 01/20/2017 03:37 AM, Jakub Narębski wrote:
> W dniu 19.01.2017 o 22:42, Junio C Hamano pisze:
>> "David J. Bakeman" <nakuru@comcast.net> writes:
>
> [...]
>>> Thanks I think that's close but it's a little more complicated I think
>>> :<( I don't know if this diagram will work but lets try.
>>>
>>> original A->B->C->D->E->F
>>> \
>>> first branch b->c->d->e
>>>
>>> new repo e->f->g->h
>>>
>>> Now I need to merge h to F without loosing b through h hopefully. Yes e
>>> was never merged back to the original repo and it's essentially gone now
>>> so I can't just merge to F or can I?
>> With the picture, I think you mean 'b' is forked from 'B' and the
>> first branch built 3 more commits on top, leading to 'e'.
>>
>> You say "new repo" has 'e' thru 'h', and I take it to mean you
>> started developing on top of the history that leads to 'e' you built
>> in the first branch, and "new repo" has the resulting history that
>> leads to 'h'.
>>
>> Unless you did something exotic and non-standard, commit 'e' in "new
>> repo" would be exactly the same as 'e' sitting on the tip of the
>> "first branch", so the picture would be more like:
>>
>>> original A->B->C->D->E->F
>>> \
>>> first branch b->c->d->e
>>> \
>>> new repo f->g->h
>> no?
> On the other hand Git has you covered even if you did something
> non-standard, like starting new repo from the _state_ of 'e', that
> is you have just copied files and created new repository, having
> 'e' (or actually 'e*') as an initial commit.
>
> original A<-B<-C<-D<-E<-F
> \
> first branch b<-c<-d<-e
>
> new repo e*<-f<-g<-h
>
> Note that arrows are in reverse direction, as it is newer commit
> pointing to its parents, not vice versa.
>
> Assuming that you have everything in a single repository, by adding
> both original and new repo as "remotes", you can use 'git replace'
> command to replace 'e*' with 'e'.
>
> original A<-B<-C<-D<-E<-F
> \
> first branch b<-c<-d<-e
> \
> new repo \-f<-g<-h
> (with refs/replace)
>
>> Then merging 'h' into 'F' will pull everything you did since
>> you diverged from the history that leads to 'F', resulting in a
>> history of this shape:
>>
>>> original A->B->C->D->E->F----------M
>>> \ /
>>> first branch b->c->d->e /
>>> \ /
>>> new repo f->g->h
> Then you would have the above history in repositories that fetched
> refs/replace/*, and the one below if replacement info is absent:
>
> original A<-B<-C<-D<-E<-F<-----------M
> \ /
> first branch b<-c<-d<-e /
> /
> new repo e*<-f->g->h
>
> But as Junio said it is highly unlikely that you are in this situation.
>
> HTH
OK so what I've done so far is to clone the original then I added
another remote connected to new repo. Then I did git merge newrepo. It
did a bunch of stuff that flashed by really fast and then reported a
conflict. Now if I do a git st there are a bunch of files that seem to
be already added to a commit and all the files with conflicts which it's
says need to be fixed and added.
I'm still learning git even after using it for several years. I've
never really seen this before. So the already added files are the ones
that git was able to merge mechanically? If so can I diff those changes
some way? Would I have to un add (reset HEAD) all those files to see
the diffs? Would it have assumed that my changes are to be preferred?
Thanks again for all the great help!
[-- Attachment #2: nakuru.vcf --]
[-- Type: text/x-vcard, Size: 248 bytes --]
begin:vcard
fn:David J. Bakeman
n:Bakeman;David J.
org:Nakuru Software Inc.
adr:;;1504 North 57th Street;Seattle;WA;98103;USA
email;internet:nakuru@comcast.net
tel;work:(206)545-0609
tel;fax:(206)600-6957
x-mozilla-html:TRUE
version:2.1
end:vcard
^ permalink raw reply
* [PATCH] Revert "push: change submodule default to check when submodules exist"
From: Stefan Beller @ 2017-01-26 0:33 UTC (permalink / raw)
To: gitster; +Cc: git, hvoigt, dborowitz, Stefan Beller
This reverts commit 04a1d8b1ae5eeecb90675cfaca2850bf26269485.
In the previous commit we set push.recurseSubmodules to "check"
by default. This check is done by walking all remote refs that are known.
For remotes we only store the refs/heads/* (and tags), which doesn't
include all commits. In e.g. Gerrit commits often end up at refs/changes/*
(that we do not store) when pushing to refs/for/master (which we also do
not store). So a workflow such as the following still fails:
$ git -C <submodule> push origin HEAD:refs/for/master
$ git push origin HEAD:refs/for/master
The following submodule paths contain changes that can
not be found on any remote:
submodule
Please try
git push --recurse-submodules=on-demand
or cd to the path and use
git push
to push them to a remote.
Trying to push with --recurse-submodules={on-demand,check}
would run into the same problem, yielding the same error message.
So changing the default to check for submodules is clearly not working
as intended for Gerrit users. We need another option that works for them.
For now just revert the change of the default.
A future patch to address this problem would be add another option that
treats the submodules differently:
1) check if the submodule needs pushing (as currently done in "check")
2) for those submodules that need pushing we run a command, e.g.
git push with the refspec passed down exactly as is. This would
work for the Gerrit case, as HEAD:refs/for/master is correct
for both the superproject and the submodule.
3) Unlike in "on-demand", we would not check again after performing
step 2), as we would not find the newly pushed things at Gerrit.
Once we have such an option, we can default to "check" again, and the
error message would mention both on-demand as well as this new option.
I'd imagine "on-demand" is what works in branch driven code review
systems such as github; for Gerrit which is based on changes the option
outlined above would work.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
After some thought, my opinion on
sb/push-make-submodule-check-the-default
change. We should not merge it down to master
until we found a good solution.
This applies on top of sb/push-make-submodule-check-the-default
unbreaking existing Gerrit users, explaining why this was a bad idea.
Feel free to either apply this patch or just eject said branch from
next.
Thanks,
Stefan
builtin/push.c | 16 +---------------
t/t5531-deep-submodule-push.sh | 6 +-----
2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 9e0b8dba9a..3bb9d6b7e6 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -3,7 +3,6 @@
*/
#include "cache.h"
#include "refs.h"
-#include "dir.h"
#include "run-command.h"
#include "builtin.h"
#include "remote.h"
@@ -23,7 +22,6 @@ static int deleterefs;
static const char *receivepack;
static int verbosity;
static int progress = -1;
-static int has_submodules_configured;
static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
static enum transport_family family;
@@ -33,15 +31,6 @@ static const char **refspec;
static int refspec_nr;
static int refspec_alloc;
-static void preset_submodule_default(void)
-{
- if (has_submodules_configured || file_exists(git_path("modules")) ||
- (!is_bare_repository() && file_exists(".gitmodules")))
- recurse_submodules = RECURSE_SUBMODULES_CHECK;
- else
- recurse_submodules = RECURSE_SUBMODULES_OFF;
-}
-
static void add_refspec(const char *ref)
{
refspec_nr++;
@@ -506,9 +495,7 @@ static int git_push_config(const char *k, const char *v, void *cb)
const char *value;
if (!git_config_get_value("push.recursesubmodules", &value))
recurse_submodules = parse_push_recurse_submodules_arg(k, value);
- } else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
- /* The submodule.<name>.url is used as a bit to indicate existence */
- has_submodules_configured = 1;
+ }
return git_default_config(k, v, NULL);
}
@@ -565,7 +552,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)
};
packet_trace_identity("push");
- preset_submodule_default();
git_config(git_push_config, &flags);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
set_push_cert_flags(&flags, push_cert);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index e690749e8a..198ce84754 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -65,11 +65,7 @@ test_expect_success 'push fails if submodule commit not on remote' '
git add gar/bage &&
git commit -m "Third commit for gar/bage" &&
# the push should fail with --recurse-submodules=check
- # on the command line. "check" is the default for repos in
- # which submodules are detected by existence of config,
- # .gitmodules file or an internal .git/modules/<submodule-repo>
- git submodule add -f ../submodule.git gar/bage &&
- test_must_fail git push ../pub.git master &&
+ # on the command line...
test_must_fail git push --recurse-submodules=check ../pub.git master &&
# ...or if specified in the configuration..
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: Force Confirmation for Dropping Changed Lines
From: Jacob Keller @ 2017-01-26 0:40 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Junio C Hamano, Git Users
In-Reply-To: <CAE1pOi0CgfxQTygg_i3dc_-_Lb8qgOOk_0hg+goJvm7PyLZseg@mail.gmail.com>
On Wed, Jan 25, 2017 at 3:57 PM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> On 25 January 2017 at 15:46, Junio C Hamano <gitster@pobox.com> wrote:
>> Jacob Keller <jacob.keller@gmail.com> writes:
>>
>>>> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
>>>> my example as simple as possible. In reality, it's Maven's POM files
>>>> (pom.xml).
>>>>
>>>> So there is no setting for any of this? There is no way to switch off
>>>> auto merging for certain files?
>>>
>>> Not really sure, but a quick google search revealed
>>> https://github.com/ralfth/pom-merge-driver
>>>
>>> Maybe that will help you?
>>
>> Its readme seems to indicate that it is either solving a different
>> problem, or solving the same problem with the opposite goal in mind,
>> in that its goal seems to be to forcibly resolve what textually does
>> not resolve cleanly by choosing sides with an arbitrary policy, so
>> that humans do not have to get involved when they ordinarily would
>> have to.
>>
>> Hilco's goal sounded to me the opposite---to force conflict even
>> when the two histories did what textually does resolve cleanly and
>> require humans to get involved even when they ordinarily wouldn't
>> have to.
>
> Yes, unfortunately, you are correct. This seems to do the exact
> opposite of what I want.
>
> Before I start learning about custom merge drivers, is what I want
> even possible? If yes, would you happen to know some good examples of
> (custom) merge drivers? (Python, Ruby, Java are all okay.)
Setting the merge driver to "unset" will do what you want, but it
would leave the current branch as the tentative answer and doesn't
actually make it easy to resolve properly. That would only require
putting "pom.xml merge=unset" in the .gitattributes file.
That might be what you want, but it doesn't actually try to update the
file during the merge so you'd have to hand-fix it yourself.
Thanks,
Jake
^ permalink raw reply
* Re: merge maintaining history
From: Jacob Keller @ 2017-01-26 0:41 UTC (permalink / raw)
To: David J. Bakeman; +Cc: Jakub Narębski, Junio C Hamano, Git mailing list
In-Reply-To: <5889436A.8000707@comcast.net>
On Wed, Jan 25, 2017 at 4:31 PM, David J. Bakeman <nakuru@comcast.net> wrote:
> OK so what I've done so far is to clone the original then I added
> another remote connected to new repo. Then I did git merge newrepo. It
> did a bunch of stuff that flashed by really fast and then reported a
> conflict. Now if I do a git st there are a bunch of files that seem to
> be already added to a commit and all the files with conflicts which it's
> says need to be fixed and added.
> I'm still learning git even after using it for several years. I've
> never really seen this before. So the already added files are the ones
> that git was able to merge mechanically? If so can I diff those changes
> some way? Would I have to un add (reset HEAD) all those files to see
> the diffs? Would it have assumed that my changes are to be preferred?
>
> Thanks again for all the great help!
Try "git diff --cached" to show all the current differences saved in the index.
Thanks,
Jake
^ permalink raw reply
* Re: [RFC 12/14] fetch-pack: do not printf after closing stdout
From: Stefan Beller @ 2017-01-26 0:50 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git@vger.kernel.org
In-Reply-To: <db1db5d0e5563464c09d1678234c9c5e8ae5b2f4.1485381677.git.jonathantanmy@google.com>
On Wed, Jan 25, 2017 at 2:03 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
> In fetch-pack, during a stateless RPC, printf is invoked after stdout is
> closed. Update the code to not do this, preserving the existing
> behavior.
This seems to me as if it could go as an independent
bugfix(?) or refactoring as this seems to be unclear from the code?
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> builtin/fetch-pack.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index ae073ab24..24af3b7c5 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -191,10 +191,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
> printf("connectivity-ok\n");
> fflush(stdout);
> }
> - close(fd[0]);
> - close(fd[1]);
> - if (finish_connect(conn))
> - return 1;
> + if (finish_connect(conn)) {
> + ret = 1;
> + goto cleanup;
> + }
>
> ret = !ref;
>
> @@ -218,11 +218,17 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
> ret = 1;
> }
>
> + if (args.stateless_rpc)
> + goto cleanup;
> +
> while (ref) {
> printf("%s %s\n",
> oid_to_hex(&ref->old_oid), ref->name);
> ref = ref->next;
> }
>
> +cleanup:
> + close(fd[0]);
> + close(fd[1]);
> return ret;
> }
> --
> 2.11.0.483.g087da7b7c-goog
>
^ permalink raw reply
* [PATCH] refs: add option core.logAllRefUpdates = always
From: cornelius.weig @ 2017-01-26 1:16 UTC (permalink / raw)
To: peff; +Cc: gitster, git, novalis, pclouds
In-Reply-To: <20170125213328.meehgxvzuajjgvag@sigill.intra.peff.net>
Hi peff,
you made it easy for me. Most of your patch still applied, only the tests
didn't quite fit. Maybe you can have a look if I've overlooked something, since
you know the changes best?
Thanks for supporting this with your patch!
^ permalink raw reply
* [PATCH] refs: add option core.logAllRefUpdates = always
From: cornelius.weig @ 2017-01-26 1:16 UTC (permalink / raw)
To: peff; +Cc: gitster, git, novalis, pclouds, Cornelius Weig
In-Reply-To: <20170126011654.21729-1-cornelius.weig@tngtech.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) do not typically change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
Documentation/config.txt | 7 +++++--
Documentation/git-tag.txt | 3 ++-
branch.c | 2 +-
builtin/init-db.c | 2 +-
cache.h | 9 +++++++-
config.c | 7 ++++++-
environment.c | 2 +-
refs.c | 15 +++++++++-----
refs/files-backend.c | 6 +++---
t/t1400-update-ref.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 90 insertions(+), 16 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..2117616 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -517,10 +517,13 @@ core.logAllRefUpdates::
"`$GIT_DIR/logs/<ref>`", by appending the new and old
SHA-1, the date/time and the reason of the update, but
only when the file exists. If this configuration
- variable is set to true, missing "`$GIT_DIR/logs/<ref>`"
+ variable is set to `true`, missing "`$GIT_DIR/logs/<ref>`"
file is automatically created for branch heads (i.e. under
refs/heads/), remote refs (i.e. under refs/remotes/),
- note refs (i.e. under refs/notes/), and the symbolic ref HEAD.
+ `refs/heads/`), remote refs (i.e. under `refs/remotes/`),
+ note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..2ac25a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -150,7 +150,8 @@ This option is only applicable when listing tags without annotation lines.
'strip' removes both whitespace and commentary.
--create-reflog::
- Create a reflog for the tag.
+ Create a reflog for the tag. To globally enable reflogs for tags, see
+ `core.logAllRefUpdates` in linkgit:git-config[1].
<tagname>::
The name of the tag to create, delete, or describe.
diff --git a/branch.c b/branch.c
index c431cbf..b955d4f 100644
--- a/branch.c
+++ b/branch.c
@@ -298,7 +298,7 @@ void create_branch(const char *name, const char *start_name,
start_name);
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (!dont_change_ref) {
struct ref_transaction *transaction;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 76d68fa..1d4d6a0 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -262,7 +262,7 @@ static int create_default_files(const char *template_path,
const char *work_tree = get_git_work_tree();
git_config_set("core.bare", "false");
/* allow template config file to override the default */
- if (log_all_ref_updates == -1)
+ if (log_all_ref_updates == LOG_REFS_UNSET)
git_config_set("core.logallrefupdates", "true");
if (needs_work_tree_config(original_git_dir, work_tree))
git_config_set("core.worktree", work_tree);
diff --git a/cache.h b/cache.h
index 00a029a..96eeaaf 100644
--- a/cache.h
+++ b/cache.h
@@ -660,7 +660,6 @@ extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
extern int prefer_symlink_refs;
-extern int log_all_ref_updates;
extern int warn_ambiguous_refs;
extern int warn_on_object_refname_ambiguity;
extern const char *apply_default_whitespace;
@@ -728,6 +727,14 @@ enum hide_dotfiles_type {
};
extern enum hide_dotfiles_type hide_dotfiles;
+enum log_refs_config {
+ LOG_REFS_UNSET = -1,
+ LOG_REFS_NONE = 0,
+ LOG_REFS_NORMAL,
+ LOG_REFS_ALWAYS
+};
+extern enum log_refs_config log_all_ref_updates;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index b680f79..c6b874a 100644
--- a/config.c
+++ b/config.c
@@ -826,7 +826,12 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.logallrefupdates")) {
- log_all_ref_updates = git_config_bool(var, value);
+ if (value && !strcasecmp(value, "always"))
+ log_all_ref_updates = LOG_REFS_ALWAYS;
+ else if (git_config_bool(var, value))
+ log_all_ref_updates = LOG_REFS_NORMAL;
+ else
+ log_all_ref_updates = LOG_REFS_NONE;
return 0;
}
diff --git a/environment.c b/environment.c
index 8a83101..c07fb17 100644
--- a/environment.c
+++ b/environment.c
@@ -21,7 +21,6 @@ int ignore_case;
int assume_unchanged;
int prefer_symlink_refs;
int is_bare_repository_cfg = -1; /* unspecified */
-int log_all_ref_updates = -1; /* unspecified */
int warn_ambiguous_refs = 1;
int warn_on_object_refname_ambiguity = 1;
int ref_paranoia = -1;
@@ -64,6 +63,7 @@ int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
unsigned long pack_size_limit_cfg;
enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
#ifndef PROTECT_HFS_DEFAULT
#define PROTECT_HFS_DEFAULT 0
diff --git a/refs.c b/refs.c
index 9bd0bc1..cd36b64 100644
--- a/refs.c
+++ b/refs.c
@@ -638,12 +638,17 @@ int copy_reflog_msg(char *buf, const char *msg)
int should_autocreate_reflog(const char *refname)
{
- if (!log_all_ref_updates)
+ switch (log_all_ref_updates) {
+ case LOG_REFS_ALWAYS:
+ return 1;
+ case LOG_REFS_NORMAL:
+ return starts_with(refname, "refs/heads/") ||
+ starts_with(refname, "refs/remotes/") ||
+ starts_with(refname, "refs/notes/") ||
+ !strcmp(refname, "HEAD");
+ default:
return 0;
- return starts_with(refname, "refs/heads/") ||
- starts_with(refname, "refs/remotes/") ||
- starts_with(refname, "refs/notes/") ||
- !strcmp(refname, "HEAD");
+ }
}
int is_branch(const char *refname)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f902393..14b17a6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2682,7 +2682,7 @@ static int files_rename_ref(struct ref_store *ref_store,
}
flag = log_all_ref_updates;
- log_all_ref_updates = 0;
+ log_all_ref_updates = LOG_REFS_NONE;
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
commit_ref_update(refs, lock, orig_sha1, NULL, &err)) {
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
@@ -2835,8 +2835,8 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
{
int logfd, result, oflags = O_APPEND | O_WRONLY;
- if (log_all_ref_updates < 0)
- log_all_ref_updates = !is_bare_repository();
+ if (log_all_ref_updates == LOG_REFS_UNSET)
+ log_all_ref_updates = is_bare_repository() ? LOG_REFS_NONE : LOG_REFS_NORMAL;
result = log_ref_setup(refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index d4fb977..a920559 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -93,6 +93,36 @@ test_expect_success 'update-ref creates reflogs with --create-reflog' '
git reflog exists $outside
'
+test_expect_success 'core.logAllRefUpdates=true does not create reflog by default' '
+ test_config core.logAllRefUpdates true &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ test_must_fail git reflog exists $outside
+'
+
+test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '
+ test_config core.logAllRefUpdates always &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ git reflog exists $outside
+'
+
+test_expect_success 'update-ref does not create reflog with --no-create-reflog if core.logAllRefUpdates=always' '
+ test_config core.logAllRefUpdates true &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref --no-create-reflog $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ test_must_fail git reflog exists $outside
+'
+
test_expect_success \
"create $m (by HEAD)" \
"git update-ref HEAD $A &&
@@ -501,6 +531,7 @@ test_expect_success 'stdin does not create reflogs by default' '
'
test_expect_success 'stdin creates reflogs with --create-reflog' '
+ test_when_finished "git update-ref -d $outside" &&
echo "create $outside $m" >stdin &&
git update-ref --create-reflog --stdin <stdin &&
git rev-parse $m >expect &&
@@ -509,6 +540,28 @@ test_expect_success 'stdin creates reflogs with --create-reflog' '
git reflog exists $outside
'
+test_expect_success 'stdin does not create reflog when core.logAllRefUpdates=true' '
+ test_config core.logAllRefUpdates true &&
+ test_when_finished "git update-ref -d $outside" &&
+ echo "create $outside $m" >stdin &&
+ git update-ref --stdin <stdin &&
+ git rev-parse $m >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ test_must_fail git reflog exists $outside
+'
+
+test_expect_success 'stdin creates reflog when core.logAllRefUpdates=always' '
+ test_config core.logAllRefUpdates always &&
+ test_when_finished "git update-ref -d $outside" &&
+ echo "create $outside $m" >stdin &&
+ git update-ref --stdin <stdin &&
+ git rev-parse $m >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ git reflog exists $outside
+'
+
test_expect_success 'stdin succeeds with quoted argument' '
git update-ref -d $a &&
echo "create $a \"$m\"" >stdin &&
--
2.10.2
^ permalink raw reply related
* Re: Force Confirmation for Dropping Changed Lines
From: Junio C Hamano @ 2017-01-26 2:32 UTC (permalink / raw)
To: Jacob Keller; +Cc: Hilco Wijbenga, Git Users
In-Reply-To: <CA+P7+xocc==-8ad-OVTahMDABA0-spDPEw05JTHopfO7Ovj2RQ@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> Setting the merge driver to "unset" will do what you want, but it
> would leave the current branch as the tentative answer and doesn't
> actually make it easy to resolve properly. That would only require
> putting "pom.xml merge=unset" in the .gitattributes file.
Where did you get that "unset" from? If that is this paragraph in
Documentation/gitattributes.txt:
Unset::
Take the version from the current branch as the
tentative merge result, and declare that the merge has
conflicts. This is suitable for binary files that do
not have a well-defined merge semantics.
you'd need to read the beginning of the file to learn how to declare
that an attribute is Unset for the path. merge=unset is setting it
to a string value, i.e. you are doing
String::
3-way merge is performed using the specified custom
merge driver. The built-in 3-way merge driver can be
explicitly specified by asking for "text" driver; the
built-in "take the current branch" driver can be
requested with "binary".
instead, specifying a custom merge driver "unset", which would
require something like
[merge "unset"]
name = feel-free merge driver
driver = filfre %O %A %B %L %P
recursive = binary
in your configuration file.
> That might be what you want, but it doesn't actually try to update the
> file during the merge so you'd have to hand-fix it yourself.
I think you should be able to do something like
$ cat >$HOME/bin/fail-3way <<\EOF
#!/bin/sh
git merge-file "$@"
exit 1
EOF
$ chmod +x $HOME/bin/fail-3way
$ cat >>$HOME/.gitconfig <<\EOF
[merge "fail"]
name = always fail 3-way merge
driver = $HOME/bin/fail-3way %A %O %B
recursive = text
EOF
$ echo pom.xml merge=fail >>.gitattributes
to define a custom merge driver whose name is "fail", that runs the
fail-3way program, which runs the bog standard 3-way merge we use
(so that it will do the best-effort textual merge) but always return
with a non-zero status to signal that the result is conflicting and
needs manual resolution.
^ permalink raw reply
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