* Re: [RFCv3 PATCH 00/14] Checkout aware of Submodules!
From: Junio C Hamano @ 2017-02-15 18:34 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, sandals
In-Reply-To: <20170215003423.20245-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Integrate updating the submodules into git checkout, with the same
> safety promises that git-checkout has, i.e. not throw away data unless
> asked to. This is done by first checking if the submodule is at the same
> sha1 as it is recorded in the superproject.
I've so far read only the first half of this series (i.e. the
preparatory part) and haven't reached the place where it starts to
become really interesting, but what I've seen so far looked very
sensible. I also find that comments given by others so far all
raise good points.
^ permalink raw reply
* Re: [PATCH v1] t7400: cleanup "submodule add clone shallow submodule" test
From: Junio C Hamano @ 2017-02-15 18:39 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, sbeller
In-Reply-To: <20170215113325.14393-1-larsxschneider@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
> The test creates a "super" directory that is not removed after the
> test finished. This directory is not used in any subsequent tests and
> should therefore be removed.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> I just noticed that my bug report test does not run properly without this
> patch: http://public-inbox.org/git/20170215111704.78320-1-larsxschneider@gmail.com/
>
> @Junio: I think this patch should be applied regardless of the bug.
Without the other one, this is not strictly needed, but I agree that
it is a good code hygiene to make sure each test cleans up after
itself.
Is this the only one that needs change in the script from that
"hygiene" point of view, or are there others? An alternative that
is also acceptable is to squash this one into the other patch.
> t/t7400-submodule-basic.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index b77cce8e40..08df483280 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1078,6 +1078,7 @@ test_expect_success 'submodule with UTF-8 name' '
> '
>
> test_expect_success 'submodule add clone shallow submodule' '
> + test_when_finished "rm -rf super" &&
> mkdir super &&
> pwd=$(pwd) &&
> (
>
> base-commit: 3b9e3c2cede15057af3ff8076c45ad5f33829436
> --
> 2.11.0
^ permalink raw reply
* Re: [PATCH 07/14] update submodules: introduce is_interesting_submodule
From: Stefan Beller @ 2017-02-15 18:46 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Jonathan Nieder, brian m. carlson,
Junio C Hamano
In-Reply-To: <20170215170441.GB29448@google.com>
On Wed, Feb 15, 2017 at 9:04 AM, Brandon Williams <bmwill@google.com> wrote:
> On 02/14, Stefan Beller wrote:
>> In later patches we introduce the --recurse-submodule flag for commands
>> that modify the working directory, e.g. git-checkout.
>>
>> It is potentially expensive to check if a submodule needs an update,
>> because a common theme to interact with submodules is to spawn a child
>> process for each interaction.
>>
>> So let's introduce a function that pre checks if a submodule needs
>> to be checked for an update.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> submodule.c | 26 ++++++++++++++++++++++++++
>> submodule.h | 8 ++++++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/submodule.c b/submodule.c
>> index c0060c29f2..4c33374ae8 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -551,6 +551,32 @@ void set_config_update_recurse_submodules(int value)
>> config_update_recurse_submodules = value;
>> }
>>
>> +int submodules_interesting_for_update(void)
>> +{
>> + /*
>> + * Update can't be "none", "merge" or "rebase",
>> + * treat any value as OFF, except an explicit ON.
>> + */
>> + return config_update_recurse_submodules == RECURSE_SUBMODULES_ON;
>> +}
>> +
>> +int is_interesting_submodule(const struct cache_entry *ce)
>
> Is there perhaps a more descriptive function name we could use instead
> of "is_interesting"? The problem is that its difficult to know why its
> interesting or for what purpose it is interesting.
I should finish the background story patch first. By 'is_interesting' I mean
* it is active/initialized/"The user expressed interested in the submodule by
setting submodule.<name>.URL
* its submodule.<name>.update strategy is != NONE.
The second point is interesting, as that entertains the thought that we'll pay
attention to the submodule.<name>.update strategy at all and
we may want to also implement rebase/merge eventually.
So I think we'd want to tighten that down to "checkout" only for now.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 02/14] lib-submodule-update.sh: define tests for recursing into submodules
From: Stefan Beller @ 2017-02-15 18:52 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Jonathan Nieder, brian m. carlson,
Junio C Hamano
In-Reply-To: <20170215165107.GA29448@google.com>
On Wed, Feb 15, 2017 at 8:51 AM, Brandon Williams <bmwill@google.com> wrote:
> On 02/14, Stefan Beller wrote:
>> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
>> index 61c54f2098..7c8c557572 100755
>> --- a/t/lib-submodule-update.sh
>> +++ b/t/lib-submodule-update.sh
>> @@ -4,6 +4,7 @@
>> # - New submodule (no_submodule => add_sub1)
>> # - Removed submodule (add_sub1 => remove_sub1)
>> # - Updated submodule (add_sub1 => modify_sub1)
>> +# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
>> # - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
>> # - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
>> # - Submodule replaced by tracked files in directory (add_sub1 =>
>> @@ -19,8 +20,8 @@
>> # / ^
>> # / remove_sub1
>> # /
>> -# add_sub1 /-------O
>> -# | / ^
>> +# add_sub1 /-------O---------O
>> +# | / ^ modify_sub1_recursive
>> # | / modify_sub1
>> # v/
>> # O------O-----------O---------O
>> @@ -73,6 +74,14 @@ create_lib_submodule_repo () {
>> git add sub1 &&
>> git commit -m "Modify sub1" &&
>>
>> + git checkout -b modify_sub1_recursively modify_sub1 &&
>> + git -C sub1 checkout -b "add_nested_sub" &&
>> + git -C sub1 submodule add --branch no_submodule ./. sub2 &&
>
> I thought we were trying to avoid './.' when adding submodules?
>
Yes we should; I'll fix that in a reroll.
It's also still on my long term fix list to remove the ./.
$ git grep 'add \./\.'
lib-submodule-update.sh: git submodule add ./. sub1 &&
t7001-mv.sh: git submodule add ./. sub &&
t7001-mv.sh: git submodule add ./. deep/directory/hierarchy/sub &&
t7507-commit-verbose.sh: git submodule add ./. sub &&
t7800-difftool.sh: git submodule add ./. submod/ule &&
^ permalink raw reply
* Re: [BUG] submodule config does not apply to upper case submodules?
From: Junio C Hamano @ 2017-02-15 18:53 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, sbeller
In-Reply-To: <20170215111704.78320-1-larsxschneider@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
> It looks like as if submodule configs ("submodule.*") for submodules
> with upper case names are ignored.
This observation is surprising, as the second level in three-level
names like "<section>.<name>.<variable>" is designed to be case
sensitive. A code that uses the config API needs to do extra things
to cause the behaviour you showed, i.e. to get submodule.U.update
ignored while submodule.l.update to be honoured. Perhaps somebody
downcases things too aggressively before comparing?
This is worth making it work as expected, needless to say ;-)
^ permalink raw reply
* how are "untracked working tree files" even possible in this case?
From: G. Sylvie Davies @ 2017-02-15 20:36 UTC (permalink / raw)
To: Git Users
Hi,
I have a script that runs the following sequence of commands within a clone:
-----
/usr/bin/git rebase --abort (took 148ms)
/usr/bin/git cherry-pick --abort (took 103ms)
/usr/bin/git clean -d -f -x (took 2007ms)
/usr/bin/git reflog expire --expire=now --all (took 106ms)
/usr/bin/git reset --hard --quiet
181def85d58597dfb28729029b2ad76b9fbb09f5 -- (took 60103ms)
/usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1
(took 1795ms)
Err: '/usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1'
Exit=128
error: The following untracked working tree files would be overwritten by merge:
.gitignore
[...many more files...]
Please move or remove them before you can merge.
Aborting
-----
I don't understand how untracked working tree files are possible after
"git clean -d -f -x" and "git reset --hard" !
I don't have access to this particular repo, but it's around 30GB when
cloned (git directory plus working tree), and around 500,000 files in
the working tree when checked out. Note: the "reset --hard" takes 60
seconds here.
- Sylvie
^ permalink raw reply
* Re: how are "untracked working tree files" even possible in this case?
From: G. Sylvie Davies @ 2017-02-15 20:38 UTC (permalink / raw)
To: Git Users
In-Reply-To: <CAAj3zPx6uP5WbA68Co0yX_yh-e5C+jze2T1hJ0NYS7hHBzgdqg@mail.gmail.com>
On Wed, Feb 15, 2017 at 12:36 PM, G. Sylvie Davies
<sylvie@bit-booster.com> wrote:
> Hi,
>
> I have a script that runs the following sequence of commands within a clone:
>
> -----
> /usr/bin/git rebase --abort (took 148ms)
> /usr/bin/git cherry-pick --abort (took 103ms)
> /usr/bin/git clean -d -f -x (took 2007ms)
> /usr/bin/git reflog expire --expire=now --all (took 106ms)
> /usr/bin/git reset --hard --quiet
> 181def85d58597dfb28729029b2ad76b9fbb09f5 -- (took 60103ms)
> /usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1
> (took 1795ms)
> Err: '/usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1'
> Exit=128
> error: The following untracked working tree files would be overwritten by merge:
> .gitignore
>
> [...many more files...]
>
> Please move or remove them before you can merge.
> Aborting
> -----
>
>
> I don't understand how untracked working tree files are possible after
> "git clean -d -f -x" and "git reset --hard" !
>
> I don't have access to this particular repo, but it's around 30GB when
> cloned (git directory plus working tree), and around 500,000 files in
> the working tree when checked out. Note: the "reset --hard" takes 60
> seconds here.
>
>
p.s. environment is: Git-2.7.4 / Bitbucket-4.10.1 /
Linux-4.4.0-59-generic (amd64)
>
>
> - Sylvie
^ permalink raw reply
* Re: how are "untracked working tree files" even possible in this case?
From: G. Sylvie Davies @ 2017-02-15 20:44 UTC (permalink / raw)
To: G. Sylvie Davies; +Cc: Git Users
In-Reply-To: <CAAj3zPyz8m3nGC_897k9SJOmfqvC2VczxFyphabkDho34nuPUw@mail.gmail.com>
On Wed, Feb 15, 2017 at 12:38 PM, G. Sylvie Davies
<sylvie@bit-booster.com> wrote:
> On Wed, Feb 15, 2017 at 12:36 PM, G. Sylvie Davies
> <sylvie@bit-booster.com> wrote:
>> Hi,
>>
>> I have a script that runs the following sequence of commands within a clone:
>>
>> -----
>> /usr/bin/git rebase --abort (took 148ms)
>> /usr/bin/git cherry-pick --abort (took 103ms)
>> /usr/bin/git clean -d -f -x (took 2007ms)
>> /usr/bin/git reflog expire --expire=now --all (took 106ms)
>> /usr/bin/git reset --hard --quiet
>> 181def85d58597dfb28729029b2ad76b9fbb09f5 -- (took 60103ms)
>> /usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1
>> (took 1795ms)
>> Err: '/usr/bin/git merge --squash 333def1a1513f84c1eb79e5341ed6ebca0d359a1'
>> Exit=128
>> error: The following untracked working tree files would be overwritten by merge:
>> .gitignore
>>
>> [...many more files...]
>>
>> Please move or remove them before you can merge.
>> Aborting
>> -----
>>
>>
>> I don't understand how untracked working tree files are possible after
>> "git clean -d -f -x" and "git reset --hard" !
>>
>> I don't have access to this particular repo, but it's around 30GB when
>> cloned (git directory plus working tree), and around 500,000 files in
>> the working tree when checked out. Note: the "reset --hard" takes 60
>> seconds here.
>>
>>
>
> p.s. environment is: Git-2.7.4 / Bitbucket-4.10.1 /
> Linux-4.4.0-59-generic (amd64)
>
>
Also, one of the "untracked files" that shows up is called
".gitmodules". Could submodules be causing this? (I have no
experience with submodules).
>>
>>
>> - Sylvie
^ permalink raw reply
* Re: [PATCH] mingw: make stderr unbuffered again
From: Johannes Sixt @ 2017-02-15 20:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Jeff Hostetler
In-Reply-To: <alpine.DEB.2.20.1702151312330.3496@virtualbox>
Am 15.02.2017 um 13:32 schrieb Johannes Schindelin:
> On Tue, 14 Feb 2017, Johannes Sixt wrote:
>> Am 14.02.2017 um 15:47 schrieb Johannes Schindelin:
>>> On Mon, 13 Feb 2017, Junio C Hamano wrote:
>>>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>>>> What we forgot was to mark stderr as unbuffered again.
>>
>> I do not see how the earlier patch turned stderr from unbuffered to
>> buffered, as it did not add or remove any setvbuf() call. Can you
>> explain?
>
> [ motivation and history of js/mingw-isatty snipped ]
>
> So instead of "bending" the target HANDLE of the existing stdout/stderr
> (which would *naturally* have kept the buffered/unbuffered nature as-is),
> we now redirect with correct API calls.
Your statement implies that at the time when winansi_init() begins,
stdio is already initialized and the buffered/unbuffered state has been
set for stderr. I would think that this is true.
Then we swap out the file handle underlying stderr in swap_osfhnd()
using dup2(). Why would that change the buffered state of stdio?
> And the patch I provided at the
> bottom of this mail thread reinstates the unbuffered nature of stderr now
> that it gets reopened.
>
> Hopefully that makes it clear why the setvbuf() call is required now, but
> was previously unnecessary?
Unfortunately, no. I do not see how dup2() causes a change in stdio
state. I must be missing something (and that may be a basic
misunderstanding of how stdio is initialized).
-- Hannes
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Philip Oakley @ 2017-02-15 17:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, Johannes Schindelin, git-for-windows, git
In-Reply-To: <xmqqshng5osz.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> There are also a few ideas at the SO answers:
>> http://stackoverflow.com/a/5652323/717355
>
> I vaguely recall that I saw somebody said the same "mark tips of
> topics as good" on the list and answered with why it does not quite
> work, though.
>
I think you may mean
https://public-inbox.org/git/7v8vyam5la.fsf@alter.siamese.dyndns.org/
I think we are thinking of opposite abstractions.
For regular bisect, the assumption (to a first order) is that there is a
single point of infection of a single persistent bug with a well defined
test, and that the goal is to find the point of first infection, as all
other incidents of the bug are in successor commits, which are all infected.
The fail-fix-break again sequence you mentioned in that thread is to my mind
a red herring as it contradicts the normal bisection assumptions (but see
below).
In the next..pu case the abstraction is in the other direction, we have
potentially multiple points of infection (from feature branches), and a
broad test (the whole test suite). In this case I believe we would like to
investigate initially the --first-parent line with a classic bisect for the
first point of failure (obviously including feature branch merges). This
would identify which feature merge, or regular commit, created the first
breakage.
Once the first point of failure has been identified, for the next..pu case,
each of the post-fail second parents of merge commits _could_ then also be
checked (which is a linear search, not a bisection), to identify any
additional feature branches that need attention. This second stage search
would probably be an option, but if the merging sequence onto pu is
generally from good to bad, then the search is likely to be short. At least
for a CI system this 2nd stage could provide useful feedback to the authors
of their mistakes...
I haven't looked back at the actual patches in that thread, so they may not
have followed my expectation of the --multi-bug (TM) search algorithm.
--
Philip
^ permalink raw reply
* Back quote typo in error messages (?)
From: Fabrizio Cucci @ 2017-02-15 21:06 UTC (permalink / raw)
To: git
Hello everyone,
it's been a couple of days that I keep noticing something (very minor)
that my OCD for symmetric things can't really stand.
If you run the following command:
$ git branch --i-dont-exists
you should get:
error: unknown option `i-dont-exists'
Shouldn't the wrong flag be surrounded by two single quotes instead of
a back quote and a single quote?
For the sake of completeness, I'm on Mac running Git 2.10.1.
Thanks,
Fabrizio
^ permalink raw reply
* Re: Back quote typo in error messages (?)
From: Jeff King @ 2017-02-15 21:21 UTC (permalink / raw)
To: Fabrizio Cucci; +Cc: git
In-Reply-To: <CAOxYW4z=bABqhmHWCc9rizykMcGBjDvqLEuqpJ6DtPve5442Fw@mail.gmail.com>
On Wed, Feb 15, 2017 at 09:06:46PM +0000, Fabrizio Cucci wrote:
> it's been a couple of days that I keep noticing something (very minor)
> that my OCD for symmetric things can't really stand.
>
> If you run the following command:
>
> $ git branch --i-dont-exists
>
> you should get:
>
> error: unknown option `i-dont-exists'
>
> Shouldn't the wrong flag be surrounded by two single quotes instead of
> a back quote and a single quote?
Some people use the matched backtick/single-quote to emulate the
non-symmetric start/end quotes used in traditional typography (and in
fact, ``foo'' in languages like asciidoc are typically rendered using
smart-quotes).
So I think what you are seeing is not wrong in the sense of being
unintended by the author of the message. But I do think that git mostly
uses matched double or single quotes in its error messages, and the
non-symmetric quotes are relatively rare. Running:
git grep "\`.*'" -- '*.c' ':!compat'
shows that there are only a few `quoted' cases in the code base (there
are 27 matches, but many of those are false positives, and some are in
comments).
I don't know how much we care about standardizing that punctuation. If
we do, I suspect there is also inconsistency between single-quotes and
double-quotes ('foo' versus "foo", which I think is an American versus
European thing; we seem to mostly use 'foo', though).
-Peff
^ permalink raw reply
* Re: enhance git-add to avoid password being staged or committed?
From: Jeff King @ 2017-02-15 21:26 UTC (permalink / raw)
To: ryenus; +Cc: Git mailing list
In-Reply-To: <CAKkAvawFJwAcn_360O101vvtbUL3Cwfqx_8VLQg_PjWzFVwDVw@mail.gmail.com>
On Wed, Feb 15, 2017 at 10:36:32PM +0800, ryenus wrote:
> This can be an optional feature, once enabled, git-add would check the
> hunk(s) to stage for sensitive information, such as passwords, secret
> tokens, then ask the user for confirmation.
>
> The implementation for secret detection could be regexp pattern(s),
> and/or (trusted?) commands
>
> Alternative solutions might be hooks during commit, push or recieve,
> but it should be the best to do this in the first place during git-add.
There are already hooks for commit and receive to catch things locally
and at publishing time, respectively. It's possible that an "add" hook
could be more useful, but I'd be a lot more convinced if people were
actively doing secret-detection in their commit hooks and had some
specific complaint that could be addressed by having an "add" hook.
-Peff
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Junio C Hamano @ 2017-02-15 21:26 UTC (permalink / raw)
To: Philip Oakley; +Cc: Christian Couder, Johannes Schindelin, git-for-windows, git
In-Reply-To: <7EA15219331242ABB08B9A9AA9F08CBE@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> In the next..pu case the abstraction is in the other direction, we
> have potentially multiple points of infection (from feature branches),
> and a broad test (the whole test suite). In this case I believe we
> would like to investigate initially the --first-parent line with a
> classic bisect for the first point of failure (obviously including
> feature branch merges). This would identify which feature merge, or
> regular commit, created the first breakage.
If you are going first-parent, you would limit the bisection to a
single-strand-of-pearls, and I agree that it is a good strategy to
find which topic branch merge broke the tip of 'pu'.
If we assume that there is no funny interaction among topics that
cancel a breakage brought in by one topic with another breakage by
another topic, then no matter how many broken topics there are, I
agree that we would get to the first broken topic.
A good thing that comes once we assume that topics are more-or-less
independent is that we could rebuild 'pu' minus the broken topic
identified by the above procedure and repeat it to find other broken
topics, still using the --first-parent bisection, because master..pu
is a linear sequence of merges of individual topics.
^ permalink raw reply
* Re: Non-zero exit code without error
From: Jeff King @ 2017-02-15 21:30 UTC (permalink / raw)
To: Serdar Sahin; +Cc: Christian Couder, git
In-Reply-To: <CAL7ZE5y5wgJfkwn0sAwVPtHhEXuX7tUc-pNkOor1WzAx_u3WhA@mail.gmail.com>
On Tue, Feb 14, 2017 at 10:56:02AM +0300, Serdar Sahin wrote:
> Just to see, if GIT server causes some issues, I’ve pushed to repo to
> github public as a private repo, and can reproduce the issue there as
> well.
FWIW, that server will be running roughly the same version of Git that
is on your GitHub Enterprise install.
I doubt the server version is relevant, though. No matter what the
server does, if the client is exiting non-zero without indicating why, I
think the client needs to be fixed.
-Peff
^ permalink raw reply
* Re: Back quote typo in error messages (?)
From: Junio C Hamano @ 2017-02-15 21:31 UTC (permalink / raw)
To: Jeff King; +Cc: Fabrizio Cucci, git
In-Reply-To: <20170215212157.qgscyglgzrd5cplf@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think what you are seeing is not wrong in the sense of being
> unintended by the author of the message. But I do think that git mostly
> uses matched double or single quotes in its error messages, and the
> non-symmetric quotes are relatively rare. Running:
>
> git grep "\`.*'" -- '*.c' ':!compat'
>
> shows that there are only a few `quoted' cases in the code base (there
> are 27 matches, but many of those are false positives, and some are in
> comments).
I did a simpler
$ git grep "\`%s'"
and saw "`git %s' is aliased to `%s'" from builtin/help.c and
"unknown option `%s'" from parse-options.c (and revision.c)
What Fabrizio saw is the one in parse-options.c, so even though the
number of strings in the code is small, they appear everywhere.
I agree that we should standardise them, and we should do so early
in a cycle, because these appear also in .po files. It is too late
for this cycle, obviously.
^ permalink raw reply
* Re: Confusing git messages when disk is full.
From: Jeff King @ 2017-02-15 21:32 UTC (permalink / raw)
To: Jáchym Barvínek; +Cc: git
In-Reply-To: <CABpqov=FE-h_2s=O9fkSjFjgFXSy6hDwc2fu5ijiVvkaLx9f_Q@mail.gmail.com>
On Sun, Feb 12, 2017 at 05:37:30PM +0100, Jáchym Barvínek wrote:
> Hello, I would like to report what I consider a bug in git, I hope I'm
> doing it the right way.
> I was trying to run `git pull` in my repository and got the following
> error: "git pull
> Your configuration specifies to merge with the ref 'refs/heads/master'
> from the remote, but no such ref was fetched."
It sounds like writing FETCH_HEAD failed, and git-pull became
confused that the ref wasn't fetched.
> Which was very confusing to me, I found some answers to what might be the cause
> but none was the right one. The actual cause was that the filesystem
> had no more free space.
> When I cleaned the space, `git pull` then gave the expected answer
> ("Already up-to-date.").
> I think the message is confusing and git should be able to report to
> the user that the cause
> is full disk.
If FETCH_HEAD failed to write because of a full disk (or any other
reason), then the right thing is for "git fetch" to write an error to
stderr, and git-pull should not continue the operation at all.
If we're not doing that, then that is certainly a bug.
-Peff
^ permalink raw reply
* Re: [PATCH] show-branch: fix crash with long ref name
From: Jeff King @ 2017-02-15 21:40 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, git, Maxim Kuvyrkov, Pranit Bauva
In-Reply-To: <CAP8UFD0EfUgfmTB4dj-A+rw79F7SWKxYvatNfR+Nj-8ukWYAQA@mail.gmail.com>
On Tue, Feb 14, 2017 at 10:29:46PM +0100, Christian Couder wrote:
> > I notice Christian's patch added a few tests. I don't know if we'd want
> > to squash them in (I didn't mean to override his patch at all; I was
> > about to send mine out when I noticed his, and I wondered if we wanted
> > to combine the two efforts).
>
> I think it would be nice to have at least one test. Feel free to
> squash mine if you want.
I started to add some tests, but I had second thoughts. It _is_ nice
to show off the fix, but as far as regressions go, this specific case is
unlikely to come up again. What would be more valuable, I think, is a
test script which set up a very long refname (not just 150 bytes or
whatever) and ran it through a series of git commands.
But then you run into all sorts of portability annoyances with pathname
restrictions (you can hack around creation by writing the refname
directly into packed-refs, but most manipulations will want to take the
.lock in the filesystem). So I dunno. It seems like being thorough is a
lot of hassle for not much gain. Being not-thorough is easy, but is
mostly a token that is unlikely to find any real bugs.
So I punted, at least for now.
I see the patches are marked for 'next' in the latest What's Cooking.
If it is not too late in today's integration cycle, here is a re-roll of
patch 3 that squashes in Pranit's suggestion (if it is too late, then
Pranit, you may want to re-send it as a squash on top).
-- >8 --
Subject: [PATCH] show-branch: use skip_prefix to drop magic numbers
We make several starts_with() calls, only to advance
pointers. This is exactly what skip_prefix() is for, which
lets us avoid manually-counted magic numbers.
Helped-by: Pranit Bauva <pranit.bauva@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/show-branch.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 404c4d09a..19756595d 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -275,8 +275,7 @@ static void show_one_commit(struct commit *commit, int no_name)
pp_commit_easy(CMIT_FMT_ONELINE, commit, &pretty);
pretty_str = pretty.buf;
}
- if (starts_with(pretty_str, "[PATCH] "))
- pretty_str += 8;
+ skip_prefix(pretty_str, "[PATCH] ", &pretty_str);
if (!no_name) {
if (name && name->head_name) {
@@ -470,17 +469,14 @@ static void snarf_refs(int head, int remotes)
}
}
-static int rev_is_head(char *head, char *name,
+static int rev_is_head(const char *head, const char *name,
unsigned char *head_sha1, unsigned char *sha1)
{
if (!head || (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
return 0;
- if (starts_with(head, "refs/heads/"))
- head += 11;
- if (starts_with(name, "refs/heads/"))
- name += 11;
- else if (starts_with(name, "heads/"))
- name += 6;
+ skip_prefix(head, "refs/heads/", &head);
+ if (!skip_prefix(name, "refs/heads/", &name))
+ skip_prefix(name, "heads/", &name);
return !strcmp(head, name);
}
@@ -799,8 +795,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
has_head++;
}
if (!has_head) {
- int offset = starts_with(head, "refs/heads/") ? 11 : 0;
- append_one_rev(head + offset);
+ const char *name = head;
+ skip_prefix(name, "refs/heads/", &name);
+ append_one_rev(name);
}
}
--
2.12.0.rc1.541.g3e32dea89
^ permalink raw reply related
* Re: Confusing git messages when disk is full.
From: Junio C Hamano @ 2017-02-15 21:47 UTC (permalink / raw)
To: Jeff King; +Cc: Jáchym Barvínek, git
In-Reply-To: <20170215213221.lnraiktneokpk3mg@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Feb 12, 2017 at 05:37:30PM +0100, Jáchym Barvínek wrote:
> If FETCH_HEAD failed to write because of a full disk (or any other
> reason), then the right thing is for "git fetch" to write an error to
> stderr, and git-pull should not continue the operation at all.
>
> If we're not doing that, then that is certainly a bug.
One suspect would be store_updated_refs(). We do catch failure from
fopen("a") of FETCH_HEAD (it is truncated earlier in the code when
the --append option is not given), but all the writes go through
stdio without error checking.
I wonder if this lazy patch is sufficient. I want to avoid having
to sprinkle
if (fputs("\\n", fp))
error(...);
all over the code.
builtin/fetch.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b5ad09d046..72347f0054 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,7 +868,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
abort:
strbuf_release(¬e);
free(url);
- fclose(fp);
+ if (ferror(fp))
+ rc = -1;
+ if (fclose(fp))
+ rc = -1;
return rc;
}
^ permalink raw reply related
* Re: [PATCH] show-branch: fix crash with long ref name
From: Junio C Hamano @ 2017-02-15 21:50 UTC (permalink / raw)
To: Jeff King; +Cc: Christian Couder, git, Maxim Kuvyrkov, Pranit Bauva
In-Reply-To: <20170215214052.5py4pxkcz4g2bmtk@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I see the patches are marked for 'next' in the latest What's Cooking.
> If it is not too late in today's integration cycle, here is a re-roll of
> patch 3 that squashes in Pranit's suggestion (if it is too late, then
> Pranit, you may want to re-send it as a squash on top).
Thanks.
I think that matches what I queued last night, except for the
Helped-by: line. Will replace.
> -- >8 --
> Subject: [PATCH] show-branch: use skip_prefix to drop magic numbers
>
> We make several starts_with() calls, only to advance
> pointers. This is exactly what skip_prefix() is for, which
> lets us avoid manually-counted magic numbers.
>
> Helped-by: Pranit Bauva <pranit.bauva@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin/show-branch.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index 404c4d09a..19756595d 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -275,8 +275,7 @@ static void show_one_commit(struct commit *commit, int no_name)
> pp_commit_easy(CMIT_FMT_ONELINE, commit, &pretty);
> pretty_str = pretty.buf;
> }
> - if (starts_with(pretty_str, "[PATCH] "))
> - pretty_str += 8;
> + skip_prefix(pretty_str, "[PATCH] ", &pretty_str);
>
> if (!no_name) {
> if (name && name->head_name) {
> @@ -470,17 +469,14 @@ static void snarf_refs(int head, int remotes)
> }
> }
>
> -static int rev_is_head(char *head, char *name,
> +static int rev_is_head(const char *head, const char *name,
> unsigned char *head_sha1, unsigned char *sha1)
> {
> if (!head || (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
> return 0;
> - if (starts_with(head, "refs/heads/"))
> - head += 11;
> - if (starts_with(name, "refs/heads/"))
> - name += 11;
> - else if (starts_with(name, "heads/"))
> - name += 6;
> + skip_prefix(head, "refs/heads/", &head);
> + if (!skip_prefix(name, "refs/heads/", &name))
> + skip_prefix(name, "heads/", &name);
> return !strcmp(head, name);
> }
>
> @@ -799,8 +795,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
> has_head++;
> }
> if (!has_head) {
> - int offset = starts_with(head, "refs/heads/") ? 11 : 0;
> - append_one_rev(head + offset);
> + const char *name = head;
> + skip_prefix(name, "refs/heads/", &name);
> + append_one_rev(name);
> }
> }
^ permalink raw reply
* Re: Back quote typo in error messages (?)
From: Fabrizio Cucci @ 2017-02-15 21:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170215212157.qgscyglgzrd5cplf@sigill.intra.peff.net>
On 15 February 2017 at 21:21, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 15, 2017 at 09:06:46PM +0000, Fabrizio Cucci wrote:
>> Shouldn't the wrong flag be surrounded by two single quotes instead of
>> a back quote and a single quote?
>
> Some people use the matched backtick/single-quote to emulate the
> non-symmetric start/end quotes used in traditional typography (and in
> fact, ``foo'' in languages like asciidoc are typically rendered using
> smart-quotes).
I definitely didn't know about the use of them in traditional typography!
But I couldn't find any example of non-symmetric quotes in AsciiDoc...
> So I think what you are seeing is not wrong in the sense of being
> unintended by the author of the message.
I had the opposite impression from the quick search in the GitHub
repo, this is why I wrote here looking for some confirmation.
> I don't know how much we care about standardizing that punctuation.
I mentioned it was very minor but, still, in my opinion a project like
Git deserves consistent punctuation! :)
^ permalink raw reply
* Re: Confusing git messages when disk is full.
From: Jeff King @ 2017-02-15 21:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jáchym Barvínek, git
In-Reply-To: <xmqqk28r2kk4.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 15, 2017 at 01:47:23PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Sun, Feb 12, 2017 at 05:37:30PM +0100, Jáchym Barvínek wrote:
> > If FETCH_HEAD failed to write because of a full disk (or any other
> > reason), then the right thing is for "git fetch" to write an error to
> > stderr, and git-pull should not continue the operation at all.
> >
> > If we're not doing that, then that is certainly a bug.
>
> One suspect would be store_updated_refs(). We do catch failure from
> fopen("a") of FETCH_HEAD (it is truncated earlier in the code when
> the --append option is not given), but all the writes go through
> stdio without error checking.
>
> I wonder if this lazy patch is sufficient. I want to avoid having
> to sprinkle
>
> if (fputs("\\n", fp))
> error(...);
>
> all over the code.
Heh, I was just tracking down the exact same spot.
I think that yes, the lazy check-error-flag-at-the-end approach is fine
for stdio.
I tried to reproduce the original problem on a full loopback filesystem,
but got:
fatal: update_ref failed for ref 'ORIG_HEAD': could not write to '.git/ORIG_HEAD'
I suspect you'd need the _exact_ right amount of free space to get all
of the predecessor steps done, and then run out of space right when
trying to flush the FETCH_HEAD contents.
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b5ad09d046..72347f0054 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -868,7 +868,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
> abort:
> strbuf_release(¬e);
> free(url);
> - fclose(fp);
> + if (ferror(fp))
> + rc = -1;
> + if (fclose(fp))
> + rc = -1;
> return rc;
Yeah, I think this works. Normally you'd want to flush before checking
ferror(), but since you detect errors from fclose, too, it should be
fine.
We probably should write something stderr, though. Maybe:
if (ferror(fp) || fclose(fp))
rc = error_errno("unable to write to %s", filename);
-Peff
^ permalink raw reply
* Re: [PATCH] show-branch: fix crash with long ref name
From: Jeff King @ 2017-02-15 21:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, git, Maxim Kuvyrkov, Pranit Bauva
In-Reply-To: <xmqqfujf2kfk.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 15, 2017 at 01:50:07PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I see the patches are marked for 'next' in the latest What's Cooking.
> > If it is not too late in today's integration cycle, here is a re-roll of
> > patch 3 that squashes in Pranit's suggestion (if it is too late, then
> > Pranit, you may want to re-send it as a squash on top).
>
> Thanks.
>
> I think that matches what I queued last night, except for the
> Helped-by: line. Will replace.
Oh, indeed. I should have actually checked what you queued. Thanks.
-Peff
^ permalink raw reply
* Re: Back quote typo in error messages (?)
From: Jeff King @ 2017-02-15 21:56 UTC (permalink / raw)
To: Fabrizio Cucci; +Cc: git
In-Reply-To: <CAOxYW4xqk4j6Uu86jq2Vi9Bpgihxfr2Tw-DQLc+7YTZiPmDtiA@mail.gmail.com>
On Wed, Feb 15, 2017 at 09:51:30PM +0000, Fabrizio Cucci wrote:
> > Some people use the matched backtick/single-quote to emulate the
> > non-symmetric start/end quotes used in traditional typography (and in
> > fact, ``foo'' in languages like asciidoc are typically rendered using
> > smart-quotes).
>
> I definitely didn't know about the use of them in traditional typography!
> But I couldn't find any example of non-symmetric quotes in AsciiDoc...
Grep for "``" in Git's documentation directory, and you will see many
examples (asciidoc only accepts the double-quote form, not singles).
You can also try:
echo "this is \`\`quoted'' text" >foo.txt
asciidoc foo.txt
and then open "foo.html" in your browser.
> > I don't know how much we care about standardizing that punctuation.
>
> I mentioned it was very minor but, still, in my opinion a project like
> Git deserves consistent punctuation! :)
I think patches would be welcome, but as Junio said, it probably should
wait for the next cycle.
-Peff
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Philip Oakley @ 2017-02-15 22:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, Johannes Schindelin, git-for-windows, git
In-Reply-To: <xmqqshng5osz.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> There are also a few ideas at the SO answers:
>> http://stackoverflow.com/a/5652323/717355
>
> I vaguely recall that I saw somebody said the same "mark tips of
> topics as good" on the list and answered with why it does not quite
> work, though.
>
I think you may mean
https://public-inbox.org/git/7v8vyam5la.fsf@alter.siamese.dyndns.org/
I think we are thinking of opposite abstractions.
For regular bisect, the assumption (to a first order) is that there is a
single point of infection of a single persistent bug with a well defined
test, and that the goal is to find the point of first infection, as all
other incidents of the bug are in successor commits, which are all infected.
The fail-fix-break again sequence you mentioned in that thread is to my mind
a red herring as it contradicts the normal bisection assumptions (but see
below).
In the next..pu case the abstraction is in the other direction, we have
potentially multiple points of infection (from feature branches), and a
broad test (the whole test suite). In this case I believe we would like to
investigate initially the --first-parent line with a classic bisect for the
first point of failure (obviously including feature branch merges). This
would identify which feature merge, or regular commit, created the first
breakage.
Once the first point of failure has been identified, for the next..pu case,
each of the post-fail second parents of merge commits _could_ then also be
checked (which is a linear search, not a bisection), to identify any
additional feature branches that need attention. This second stage search
would probably be an option, but if the merging sequence onto pu is
generally from good to bad, then the search is likely to be short. At least
for a CI system this 2nd stage could provide useful feedback to the authors
of their mistakes...
I haven't looked back at the actual patches in that thread, so they may not
have followed my expectation of the --multi-bug (TM) search algorithm.
--
Philip
^ 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