* Re: [PATCH 02/14] lib-submodule-update.sh: define tests for recursing into submodules
From: Brandon Williams @ 2017-02-15 16:51 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, jrnieder, sandals, gitster
In-Reply-To: <20170215003423.20245-3-sbeller@google.com>
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?
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 07/14] update submodules: introduce is_interesting_submodule
From: Brandon Williams @ 2017-02-15 17:04 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, jrnieder, sandals, gitster
In-Reply-To: <20170215003423.20245-8-sbeller@google.com>
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.
> +{
> + const struct submodule *sub;
> +
> + if (!S_ISGITLINK(ce->ce_mode))
> + return 0;
> +
> + if (!submodules_interesting_for_update())
> + return 0;
> +
> + sub = submodule_from_path(null_sha1, ce->name);
> + if (!sub)
> + return 0;
> +
> + return sub->update_strategy.type != SM_UPDATE_NONE;
> +}
> +
> static int has_remote(const char *refname, const struct object_id *oid,
> int flags, void *cb_data)
> {
> diff --git a/submodule.h b/submodule.h
> index c4e1ac828e..84b67a7c4a 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -59,6 +59,14 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
> const struct diff_options *opt);
> extern void set_config_fetch_recurse_submodules(int value);
> extern void set_config_update_recurse_submodules(int value);
> +
> +/*
> + * Traditionally Git ignored changes made for submodules.
> + * This function checks if we are interested in the given submodule
> + * for any kind of operation.
> + */
> +extern int submodules_interesting_for_update(void);
> +extern int is_interesting_submodule(const struct cache_entry *ce);
> extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
> extern int fetch_populated_submodules(const struct argv_array *options,
> const char *prefix, int command_line_option,
> --
> 2.12.0.rc0.16.gd1691994b4.dirty
>
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 03/14] make is_submodule_populated gently
From: Junio C Hamano @ 2017-02-15 18:10 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, sandals
In-Reply-To: <20170215003423.20245-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> We need the gentle version in a later patch. As we have just one caller,
> migrate the caller.
Ordinarily, we keep the original helper implemented as a thin
wrapper that passes NULL as retun_error_code, which causes it to
die() on error for existing callers. But because we only have one
caller (and topics in-flight do not add new ones), we do not bother
with that.
The reasoning makes sense, at least to me.
We may want to add a comment about the behaviour upon error for the
helper function? I see resolve_gitdir_gently() does not do so and
the readers have to follow the callflow down to read_gitfile_gently()
which does have the comment, so perhaps we are OK without any.
Looks good to me.
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> builtin/grep.c | 2 +-
> submodule.c | 4 ++--
> submodule.h | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef499..b17835aed6 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -616,7 +616,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
> {
> if (!is_submodule_initialized(path))
> return 0;
> - if (!is_submodule_populated(path)) {
> + if (!is_submodule_populated_gently(path, NULL)) {
> /*
> * If searching history, check for the presense of the
> * submodule's gitdir before skipping the submodule.
> diff --git a/submodule.c b/submodule.c
> index 3b98766a6b..9bbdd3ce7c 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -237,12 +237,12 @@ int is_submodule_initialized(const char *path)
> /*
> * Determine if a submodule has been populated at a given 'path'
> */
> -int is_submodule_populated(const char *path)
> +int is_submodule_populated_gently(const char *path, int *return_error_code)
> {
> int ret = 0;
> char *gitdir = xstrfmt("%s/.git", path);
>
> - if (resolve_gitdir(gitdir))
> + if (resolve_gitdir_gently(gitdir, return_error_code))
> ret = 1;
>
> free(gitdir);
> diff --git a/submodule.h b/submodule.h
> index 05ab674f06..689033e538 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -41,7 +41,7 @@ extern int submodule_config(const char *var, const char *value, void *cb);
> extern void gitmodules_config(void);
> extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
> extern int is_submodule_initialized(const char *path);
> -extern int is_submodule_populated(const char *path);
> +extern int is_submodule_populated_gently(const char *path, int *return_error_code);
> extern int parse_submodule_update_strategy(const char *value,
> struct submodule_update_strategy *dst);
> extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
^ permalink raw reply
* Re: [BUG] submodule config does not apply to upper case submodules?
From: Stefan Beller @ 2017-02-15 18:14 UTC (permalink / raw)
To: Lars Schneider; +Cc: git@vger.kernel.org
In-Reply-To: <20170215111704.78320-1-larsxschneider@gmail.com>
On Wed, Feb 15, 2017 at 3:17 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
> It looks like as if submodule configs ("submodule.*") for submodules
> with upper case names are ignored. The test cases shows that skipping
> a submodule during a recursive clone seems not to work.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> I observed the bug on Windows, macOS, and Linux and at least on
> v2.11.0 and v2.11.1:
> https://travis-ci.org/larsxschneider/git/builds/201828672
Thanks for the bug report.
>
> Right now I have no time to fix it but I might be able to look into it
> next week (if no one else tackles it before that).
I might look into it before next week.
> Notes:
> Base Commit: 3b9e3c2ced (v2.11.1)
> Diff on Web: https://github.com/larsxschneider/git/commit/a122feaf9f
> Checkout: git fetch https://github.com/larsxschneider/git submodule/uppercase-bug-v1 && git checkout a122feaf9f
I like these notes, though base commit is duplicate with below.
> +test_expect_success 'submodule config does not apply to upper case submodules' '
...
> + git submodule add ../UPPERSUB &&
> + git commit -m "add submodules"
> + ) &&
up to here we only do "setup"-sy stuff.
("setup being a trigger word that you cannot omit
the test for subsequent tests to work)
So maybe have
test_expect_success 'setup submodule with lower and uppercase' '
...
'
test_expect_success 'just the clone' '
...
'
test_expect_success ' check for lower case'
grep -e "Skipping submodule *lowersub*" err
'
test_expect_failure ' check for upper case'
grep ...
'
> + git -c submodule.lowersub.update=none clone --recursive super clone-success 2>&1 |
> + grep "Skipping submodule" &&
> + git -c submodule.UPPERSUB.update=none clone --recursive super clone-failure 2>&1 |
> + grep "Skipping submodule"
I'd rather give both options in one invocation and then grep from a file, e.g.
git -c submodule.lowersub.update=none -c submodule.UPPERSUB.update=none \
clone --recursive super super_clone 2>err 1>out &&
grep -e "Skipping submodule *lowersub*" err
> +'
>
> test_done
>
> base-commit: 3b9e3c2cede15057af3ff8076c45ad5f33829436
Heh, I see what you did here. :)
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 04/14] connect_work_tree_and_git_dir: safely create leading directories
From: Junio C Hamano @ 2017-02-15 18:22 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, jrnieder, sandals
In-Reply-To: <20170215003423.20245-5-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> In a later patch we'll use connect_work_tree_and_git_dir when the
> directory for the gitlink file doesn't exist yet. Safely create
> the directory first.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
Among the existing two callers, the "absorb" logic in submodule.c
has safe-create-leading-directory (SCLD) immediately before the call
to this function, which can now be lost with this change. The other
one in cmd_mv() has the target directory as the result of moving the
original directory, and I do not think there is any corresponding
call that can be lost from there after this change, but it is not an
error to call SCLD on a path that already exists, so all is OK.
Is it sensible to let the code continue with just an fprintf() (not
even warning() or error()) when SCLD fails?
> ---
> dir.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/dir.c b/dir.c
> index 4541f9e146..69ca3d1411 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2735,6 +2735,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>
> /* Update gitfile */
> strbuf_addf(&file_name, "%s/.git", work_tree);
> + if (safe_create_leading_directories_const(file_name.buf))
> + fprintf(stderr, "could not create directories for %s\n", file_name.buf);
> write_file(file_name.buf, "gitdir: %s",
> relative_path(git_dir, work_tree, &rel_path));
^ permalink raw reply
* Re: [PATCH 05/14] update submodules: add submodule config parsing
From: Junio C Hamano @ 2017-02-15 18:29 UTC (permalink / raw)
To: Stefan Beller
In-Reply-To: <20170215003423.20245-6-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Similar to b33a15b08 (push: add recurseSubmodules config option,
> 2015-11-17) and 027771fcb1 (submodule: allow erroneous values for the
> fetchRecurseSubmodules option, 2015-08-17), we add submodule-config code
> that is later used to parse whether we are interested in updating
> submodules.
>
> We need the `die_on_error` parameter to be able to call this parsing
> function for the config file as well, which if incorrect lets Git die.
>
> As we're just touching the header file, also mark all functions extern.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> submodule-config.c | 22 ++++++++++++++++++++++
> submodule-config.h | 17 +++++++++--------
> 2 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 93453909cf..93f01c4378 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -234,6 +234,28 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)
> return parse_fetch_recurse(opt, arg, 1);
> }
>
> +static int parse_update_recurse(const char *opt, const char *arg,
> + int die_on_error)
> +{
> + switch (git_config_maybe_bool(opt, arg)) {
> + case 1:
> + return RECURSE_SUBMODULES_ON;
> + case 0:
> + return RECURSE_SUBMODULES_OFF;
> + default:
> + if (!strcmp(arg, "checkout"))
> + return RECURSE_SUBMODULES_ON;
> + if (die_on_error)
> + die("bad %s argument: %s", opt, arg);
> + return RECURSE_SUBMODULES_ERROR;
> + }
> +}
Proliferation of similarly looking config parser made me briefly
wonder if they can somehow be shared, but I think it is correct to
view that update/fetch/push have conceptually different set of
allowed modes, whose names happen to overlap, so keeping them
separate like this patch does (and its predecessors did to take us
into the shape of the current code) makes sense, at least to me.
^ permalink raw reply
* Re: [PATCH v1] t7400: cleanup "submodule add clone shallow submodule" test
From: Stefan Beller @ 2017-02-15 18:29 UTC (permalink / raw)
To: Lars Schneider; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170215113325.14393-1-larsxschneider@gmail.com>
On Wed, Feb 15, 2017 at 3:33 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
> @Junio: I think this patch should be applied regardless of the bug.
Sounds good; thanks!
Stefan
^ permalink raw reply
* 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
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