* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Johannes Schindelin @ 2017-02-15 14:22 UTC (permalink / raw)
To: Philip Oakley; +Cc: Christian Couder, Junio C Hamano, git-for-windows, git
In-Reply-To: <E2C1B7A8FBF94C8CB1C9C5754D882800@PhilipOakley>
Hi Philip,
On Tue, 14 Feb 2017, Philip Oakley wrote:
> From: "Christian Couder" <christian.couder@gmail.com>
> > On Tue, Feb 14, 2017 at 10:08 PM, Junio C Hamano <gitster@pobox.com>
> > wrote:
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > >
> > > > On Mon, 13 Feb 2017, Junio C Hamano wrote:
> > > >
> > > > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > > > >
> > > > > > That is why I taught the Git for Windows CI job that tests the
> > > > > > four upstream Git integration branches to *also* bisect test
> > > > > > breakages and then upload comments to the identified commit on
> > > > > > GitHub
> > > > >
> > > > > Good. I do not think it is useful to try 'pu' as an aggregate
> > > > > and expect it to always build and work [*1*], but your "bisect
> > > > > and pinpoint" approach makes it useful to identify individual
> > > > > topic that brings in a breakage.
> > > >
> > > > Sadly the many different merge bases[*1*] between `next` and `pu`
> > > > (which are the obvious good/bad points for bisecting
> > > > automatically) bring my build agents to its knees. I may have to
> > > > disable the bisecting feature as a consequence.
> >
> > Yeah, this is a bug in the bisect algorithm. Fixing it is in the GSoC
> > 2017 Ideas.
>
> There are also a few ideas at the SO answers:
> http://stackoverflow.com/a/5652323/717355
Thanks for that link!
However, my main aim was not to get distracted into yet another corner of
Git that needs to be fixed (I am on enough of those projects already).
I was merely surprised (and not in a good way) that a plenty ordinary
bisect between `next` and `pu` all of a sudden tested a *one year old*
commit whether it was good or not.
And I doubt that the strategy to mark all second parents of all merge
commits in pu..next as "good" would work well, as the merge bases *still*
would have to be tested.
I guess what I have to resort to is this: if I know that `next` tests
fine, and that `pu` fails, I shall mark all merge bases as "good". I am
sure this has its own set of pitfalls, undoubtedly costing me more time on
that front...
But at least my cursory analysis of this idea seems to make sense: I use
`next` essentially as a catch-all to verify that the breakage has entered
`pu`, but not yet `next`. This reasoning makes sense, given that we know
the waterfall topology of pu/next/master/maint: patches enter from left to
right, i.e. anything that entered `pu` may later enter `next`, but not
vice versa.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: SZEDER Gábor @ 2017-02-15 14:26 UTC (permalink / raw)
To: Cornelius Weig; +Cc: Git mailing list, Richard Wagner, j6t
In-Reply-To: <20170214212404.31469-2-cornelius.weig@tngtech.com>
On Tue, Feb 14, 2017 at 10:24 PM, <cornelius.weig@tngtech.com> wrote:
> + *)
> + __git_complete_tree_file "$ref" "$cur"
> + ;;
There is one more caveat here.
Both our __git_complete_index_file() and Bash's builtin filename
completion lists matching paths like this:
$ git rm contrib/co<TAB>
coccinelle/ contacts/
completion/ convert-grafts-to-replace-refs.sh
i.e. the leading path components are not redundantly repeated.
Now, with this patch in this code path the list would look like this:
$ git checkout completion-refs-speedup contrib/co<TAB>
contrib/coccinelle/
contrib/completion/
contrib/contacts/
contrib/convert-grafts-to-replace-refs.sh
See the difference?
I once made a feeble attempt to make completion of the <ref>:<path>
notation (i.e. what you extracted into __git_complete_tree_file())
look like regular filename completion, but couldn't.
Gábor
^ permalink raw reply
* Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area
From: Jeff Hostetler @ 2017-02-15 14:27 UTC (permalink / raw)
To: Jeff King, Johannes Schindelin; +Cc: git, Junio C Hamano, Jeff Hostetler
In-Reply-To: <20170214220332.753i4tgclm62er4f@sigill.intra.peff.net>
On 2/14/2017 5:03 PM, Jeff King wrote:
> On Tue, Feb 14, 2017 at 12:31:46PM +0100, Johannes Schindelin wrote:
>
>> On Windows, calls to memihash() and maintaining the istate.name_hash and
>> istate.dir_hash HashMaps take significant time on very large
>> repositories. This series of changes reduces the overall time taken for
>> various operations by reducing the number calls to memihash(), moving
>> some of them into multi-threaded code, and etc.
>>
>> Note: one commenter in https://github.com/git-for-windows/git/pull/964
>> pointed out that memihash() only handles ASCII correctly. That is true.
>> And fixing this is outside the purview of this patch series.
> Out of curiosity, do you have numbers? Bonus points if the speedup can
> be shown via a t/perf script.
>
> We have a read-cache perf-test already, but I suspect you'd want
> something more like "git status" or "ls-files -o" that calls into
> read_directory().
I have some informal numbers in a spreadsheet. I was seeing
a 8-9% speed up on a status on my gigantic repo.
I'll try to put together a before/after perf-test to better demonstrate
this.
Jeff
^ permalink raw reply
* enhance git-add to avoid password being staged or committed?
From: ryenus @ 2017-02-15 14:36 UTC (permalink / raw)
To: Git mailing list
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.
The context of this is the following HN discussion about passwords on
GitHub: https://news.ycombinator.com/item?id=13650818
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2017, #04; Tue, 14)
From: René Scharfe @ 2017-02-15 16:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq60kc750b.fsf@gitster.mtv.corp.google.com>
Am 14.02.2017 um 23:59 schrieb Junio C Hamano:
> * rs/ls-files-partial-optim (2017-02-13) 2 commits
> - ls-files: move only kept cache entries in prune_cache()
> - ls-files: pass prefix length explicitly to prune_cache()
>
> "ls-files" run with pathspec has been micro-optimized to avoid one
> extra call to memmove().
Nit: The number of memmove(3) calls stays the same, but the number of
bytes that are moved is reduced.
René
^ permalink raw reply
* Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area
From: Jeff King @ 2017-02-15 16:44 UTC (permalink / raw)
To: Jeff Hostetler; +Cc: Johannes Schindelin, git, Junio C Hamano, Jeff Hostetler
In-Reply-To: <16b1259c-4cdc-8f4d-db47-d724386a3d2b@jeffhostetler.com>
On Wed, Feb 15, 2017 at 09:27:53AM -0500, Jeff Hostetler wrote:
> I have some informal numbers in a spreadsheet. I was seeing
> a 8-9% speed up on a status on my gigantic repo.
>
> I'll try to put together a before/after perf-test to better
> demonstrate this.
Thanks. What I'm mostly curious about is how much each individual step
buys. Sometimes when doing a long optimization series, I find that some
of the optimizations make other ones somewhat redundant (e.g., if patch
2 causes us to call the optimized code from patch 3 less often).
-Peff
^ permalink raw reply
* 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
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