* Re: [PATCH 1/2] diff: document behavior of relative diff.orderFile
From: Junio C Hamano @ 2017-01-10 20:19 UTC (permalink / raw)
To: Richard Hansen; +Cc: Jeff King, git
In-Reply-To: <e100d30a-5ee8-8485-5012-f9b1c6961ffa@google.com>
Richard Hansen <hansenr@google.com> writes:
> On 2017-01-10 01:58, Jeff King wrote:
> ...
>> What happens in a bare repository?
>>
>> I'm guessing it's relative to the top-level of the repository,
>
> I just tried it out and it's relative to $PWD.
That is understandable. When the user says
$ cmd -O $file
with any option -O that takes a filename, it is most natural if we
used $PWD/$file when $file is not absolute path.
^ permalink raw reply
* Re: [PATCH 2/2] diff: document the pattern format for diff.orderFile
From: Junio C Hamano @ 2017-01-10 20:14 UTC (permalink / raw)
To: Richard Hansen; +Cc: git
In-Reply-To: <20170110004031.57985-3-hansenr@google.com>
Richard Hansen <hansenr@google.com> writes:
> Document the format of the patterns used for the diff.orderFile
> setting and diff's '-O' option by referring the reader to the
> gitignore[5] page.
>
> Signed-off-by: Richard Hansen <hansenr@google.com>
> ---
> Documentation/diff-config.txt | 3 ++-
> Documentation/diff-options.txt | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index 875212045..a35ecdd6b 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -100,7 +100,8 @@ diff.noprefix::
>
> diff.orderFile::
> File indicating how to order files within a diff, using
> - one shell glob pattern per line.
> + one glob pattern per line.
> + See linkgit:gitignore[5] for the pattern format.
I do not think it is wise to suggest referring to gitignore, as the
logic of matching is quite different, other than the fact that they
both use wildmatch() internally. Also, unlike gitignore, orderfile
does not allow any negative matching i.e. "!<pattern>".
> If `diff.orderFile` is a relative pathname, it is treated as
> relative to the top of the work tree.
> Can be overridden by the '-O' option to linkgit:git-diff[1].
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e6215c372..dc6b1af71 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -467,7 +467,8 @@ endif::git-format-patch[]
>
> -O<orderfile>::
> Output the patch in the order specified in the
> - <orderfile>, which has one shell glob pattern per line.
> + <orderfile>, which has one glob pattern per line.
> + See linkgit:gitignore[5] for the pattern format.
> This overrides the `diff.orderFile` configuration variable
> (see linkgit:git-config[1]). To cancel `diff.orderFile`,
> use `-O/dev/null`.
^ permalink raw reply
* [PATCH] index: improve constness for reading blob data
From: Brandon Williams @ 2017-01-10 20:06 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
Improve constness of the index_state parameter to the
'read_blob_data_from_index' function.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
cache.h | 2 +-
read-cache.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index a50a61a19..363953c1a 100644
--- a/cache.h
+++ b/cache.h
@@ -599,7 +599,7 @@ extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char
extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
extern int index_name_is_other(const struct index_state *, const char *, int);
-extern void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *);
+extern void *read_blob_data_from_index(const struct index_state *, const char *, unsigned long *);
/* do stat comparison even if CE_VALID is true */
#define CE_MATCH_IGNORE_VALID 01
diff --git a/read-cache.c b/read-cache.c
index f92a912dc..6ee044442 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2292,7 +2292,8 @@ int index_name_is_other(const struct index_state *istate, const char *name,
return 1;
}
-void *read_blob_data_from_index(struct index_state *istate, const char *path, unsigned long *size)
+void *read_blob_data_from_index(const struct index_state *istate,
+ const char *path, unsigned long *size)
{
int pos, len;
unsigned long sz;
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* Re: [PATCHv2 4/5] unpack-trees: factor file removal out of check_updates
From: Junio C Hamano @ 2017-01-10 20:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: peff, l.s.r, git
In-Reply-To: <20170109194621.17013-5-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This makes check_updates shorter and easier to understand.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
I agree that 3/5 made it easier to understand by ejecting a block
that is not essential to the functionality of the function out of
it, making the remainder of the fuction about "removing gone files
and then write out the modified files".
The ejecting of the first half of these two operations, both are
what this function is about, done by this step feels backwards. If
anything, the "only do the actual working tree manipulation when not
doing a dry-run and told to update" logic that must be in both are
spread in two helper functions after step 5/5, and with the added
boilerplate for these two helpers, the end result becomes _longer_
to understand what is really going on when check_updates() is
called.
Is the original after step 3/5 too long and hard to understand?
> unpack-trees.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index b564024472..ac59510251 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -218,6 +218,26 @@ static void unlink_entry(const struct cache_entry *ce)
> schedule_dir_for_removal(ce->name, ce_namelen(ce));
> }
>
> +static unsigned remove_workingtree_files(struct unpack_trees_options *o,
> + struct progress *progress)
> +{
> + int i;
> + unsigned cnt = 0;
> + struct index_state *index = &o->result;
> +
> + for (i = 0; i < index->cache_nr; i++) {
> + const struct cache_entry *ce = index->cache[i];
> +
> + if (ce->ce_flags & CE_WT_REMOVE) {
> + display_progress(progress, ++cnt);
> + if (o->update && !o->dry_run)
> + unlink_entry(ce);
> + }
> + }
> +
> + return cnt;
> +}
> +
> static struct progress *get_progress(struct unpack_trees_options *o)
> {
> unsigned cnt = 0, total = 0;
> @@ -254,15 +274,8 @@ static int check_updates(struct unpack_trees_options *o)
>
> if (o->update)
> git_attr_set_direction(GIT_ATTR_CHECKOUT, index);
> - for (i = 0; i < index->cache_nr; i++) {
> - const struct cache_entry *ce = index->cache[i];
>
> - if (ce->ce_flags & CE_WT_REMOVE) {
> - display_progress(progress, ++cnt);
> - if (o->update && !o->dry_run)
> - unlink_entry(ce);
> - }
> - }
> + cnt = remove_workingtree_files(o, progress);
> remove_marked_cache_entries(index);
> remove_scheduled_dirs();
^ permalink raw reply
* Re: [PATCHv2 2/5] unpack-trees: remove unneeded continue
From: Junio C Hamano @ 2017-01-10 19:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: peff, l.s.r, git
In-Reply-To: <20170109194621.17013-3-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> The continue is the last statement in the loop, so not needed.
> This situation arose in 700e66d66 (2010-07-30, unpack-trees: let
> read-tree -u remove index entries outside sparse area) when statements
> after the continue were removed.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
Thanks for digging the history to find the culprit.
This is an unrelated tangent, but I do not think of a way other than
"log -L" to find it; conceptually, "blame" ought to be usable for
it, but it is not useful for finding deleted lines.
> unpack-trees.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 8e6768f283..d443decb23 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -253,7 +253,6 @@ static int check_updates(struct unpack_trees_options *o)
> display_progress(progress, ++cnt);
> if (o->update && !o->dry_run)
> unlink_entry(ce);
> - continue;
> }
> }
> remove_marked_cache_entries(index);
^ permalink raw reply
* Re: [PATCH v4 14/14] mergetool: fix running in subdir when rerere enabled
From: Junio C Hamano @ 2017-01-10 19:25 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Richard Hansen, git, davvid, sbeller, simon
In-Reply-To: <3a09e318-f14b-5f14-a992-3bd045f0a4c6@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
>> + prefix=$(git rev-parse --show-prefix) || exit 1
>> + cd_to_toplevel
>> +
>> + if test -n "$orderfile"
>> + then
>> + orderfile=$(git rev-parse --prefix "$prefix" -- "$orderfile") || exit 1
>> + orderfile=$(printf %s\\n "$orderfile" | sed -e 1d)
>
> Is the purpose of this complication only to detect errors of the git
> invocation? IMHO, we could dispense with that, but others might
> disagree. I am arguing because this adds yet another process; but it
> is only paid when -O is used, so...
I do not terribly mind an added process, but this change makes it
harder to read and also forces the readers to wonder if the quoting
around printf is correct.
>> @@ -461,14 +470,17 @@ main () {
>> then
>> print_noop_and_exit
>> fi
>> + elif test $# -ge 0
>> + then
>> + files_quoted=$(git rev-parse --sq --prefix "$prefix" -- "$@") || exit 1
>> + eval "set -- $files_quoted"
>
> BTW, the --sq and eval business is not required here. At this point,
> $IFS = $'\n', so
>
> set -- $(git rev-parse --sq --prefix "$prefix" -- "$@")
>
> will do. (Except that it would not detect errors.)
I thought you are suggesting not to use --sq but it is still there.
I think the original is written in such a way that any letter in
each of "$@" is preserved, even an LF in the filename.
Such a pathname probably won't correctly be given from the "rerere
remaining" side (i.e. when you are lazy and run mergetool without
pathname), so it may appear not to matter, but being able to give an
explicit pathname from the command line is a workaround for that, so
in that sense, it has value to prepare this side of the codepath to
be able to cope with such a pathname.
Unrelated, but I notice that in this:
eval "set -- $(git rev-parse --sq --prefix "$prefix" -- "$@")"
shift
"set" will get one "--" from its direct command line argument,
followed by "--" that comes out of rev-parse, and then the first
pathname (i.e. "$prefix/$1", if "$1" is relative). Then the first
"--" is discarded and the second "--" that came out of rev-parse
becomes "$1", and the first pathname becomes "$2", etc. We use
"shift" to get rid of "--" and move everything down by one.
It is my fault but it is a roundabout way to say:
eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
I would think, and we should probably want to write this like so.
[PATCH v4 02/14] would probably want to be updated as well.
I can locally update them if everybody agrees it is a good idea.
^ permalink raw reply
* git-mergetool to be used for rebases as well?
From: Stefan Beller @ 2017-01-10 18:54 UTC (permalink / raw)
To: git; +Cc: Stefan Beller
An internal user report running on origin/next:
$ git pull
From .
* branch ... -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: ...
Applying: ...
Using index info to reconstruct a base tree...
CONFLICT (content): ....
error: Failed to merge in the changes.
Patch failed at 0002...
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
$ git status
rebase in progress; onto ...
You are currently rebasing branch '...' on '...'.
Changes to be committed:
modified: ...
Unmerged paths:
both modified: ...
$ git mergetool
No files need merging
$ git diff <file name>
diff --cc <file name>
index ...
--- a/file
+++ b/file
@@@ ...
content
++<<<<<<< HEAD
+ content
++=======
+ content
++>>>>>>> other commit
content
The mergetool used to work apparently, but stopped for rebases.
I noticed in neither t7610-mergetool.sh nor any rebase test the combination of
rebase and mergetool is tested.
Stefan
^ permalink raw reply
* Re: [RFC PATCH 0/5] Localise error headers
From: Stefan Beller @ 2017-01-10 18:28 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, git@vger.kernel.org
In-Reply-To: <20170110090418.4egk4oflblshmjon@sigill.intra.peff.net>
On Tue, Jan 10, 2017 at 1:04 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Jan 09, 2017 at 01:43:15PM +0100, Michael J Gruber wrote:
>
>> > I can't say I'm excited about having matching "_" variants for each
>> > function. Are we sure that they are necessary? I.e., would it be
>> > acceptable to just translate them always?
>>
>> We would still need to mark the strings, e.g.
>>
>> die(N_("oopsie"));
>>
>> and would not be able to opt out of translating in the code (only in the
>> po file, by not providing a translation).
>
> I meant more along the lines of: would it be OK to just always translate
> the prefix, even if the message itself is not translated? I.e.,
>
> diff --git a/usage.c b/usage.c
> index 82ff13163..8e5400f57 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -32,7 +32,7 @@ static NORETURN void usage_builtin(const char *err, va_list params)
>
> static NORETURN void die_builtin(const char *err, va_list params)
> {
> - vreportf("fatal: ", err, params);
> + vreportf(_("fatal: "), err, params);
> exit(128);
> }
>
>> In any case, the question is whether we want to tell the user
>>
>> A: B
>>
>> where A is in English and B is localised, or rather localise both A and
>> B (for A in "error", "fatal", "warning"...).
>>
>> For localising A and B, we'd need this series or something similar. For
>> keeping the mix, we don't need to do anything ;)
>
> What I wrote above would keep the mix, but switch it in the other
> direction.
>
> And then presumably that mix would gradually move to 100% consistency as
> more messages are translated. But the implicit question is: are there
> die() messages that should never be translated? I'm not sure.
I would assume any plumbing command is not localizing?
Because in plumbing land, (easily scriptable) you may find
a grep on the output/stderr for a certain condition?
To find a good example, "git grep die" giving me some food of though:
die_errno(..) should always take a string marked up for translation,
because the errno string is translated?
(-> we'd have to fix up any occurrence of git grep "die_errno(\"")
apply.c: die(_("internal error"));
That is funny, too. I think we should substitute that with
die("BUG: untranslated, but what went wrong instead")
>
> -Peff
^ permalink raw reply
* Re: [PATCHv8] pathspec: give better message for submodule related pathspec error
From: Junio C Hamano @ 2017-01-10 18:13 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, peff
In-Reply-To: <20170109231650.9043-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> This comes as a single patch again, replacing sb/pathspec-errors.
> It goes directly on top of bw/pathspec-cleanup.
>
> v8:
> cleaned white spaces in commit message
> removed TEST_CREATE_SUBMODULE=yes
> : > instead of touch.
>
> v7:
> do not rely on "test_commit -C" being there, nor the infrastructure
> to request a "good" submodule upstream. Just create a submodule outselves
> to test in.
>
> Thanks,
> Stefan
Thanks, queued.
^ permalink raw reply
* Re: [PATCH 1/2] diff: document behavior of relative diff.orderFile
From: Richard Hansen @ 2017-01-10 17:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170110065816.pu325sxajbyuqpj6@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]
On 2017-01-10 01:58, Jeff King wrote:
> On Mon, Jan 09, 2017 at 07:40:30PM -0500, Richard Hansen wrote:
>
>> Document that a relative pathname for diff.orderFile is interpreted as
>> relative to the top-level work directory.
>>
>> Signed-off-by: Richard Hansen <hansenr@google.com>
>> ---
>> Documentation/diff-config.txt | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
>> index 58f4bd6af..875212045 100644
>> --- a/Documentation/diff-config.txt
>> +++ b/Documentation/diff-config.txt
>> @@ -101,6 +101,8 @@ diff.noprefix::
>> diff.orderFile::
>> File indicating how to order files within a diff, using
>> one shell glob pattern per line.
>> + If `diff.orderFile` is a relative pathname, it is treated as
>> + relative to the top of the work tree.
>> Can be overridden by the '-O' option to linkgit:git-diff[1].
>
> What happens in a bare repository?
I didn't know which is why this patch is silent on that topic. :)
>
> I'm guessing it's relative to the top-level of the repository,
I just tried it out and it's relative to $PWD.
> but we should probably spell it out.
Do we want it to be relative to $PWD? I think relative to $GIT_DIR is
more useful. If we want it to be relative to $GIT_DIR, then I think we
should stay silent regarding bare repositories so that the behavior
remains unspecified until we update the logic.
>
> The other case is --no-index when we are not in a repository at all, but
> that should just be relative to the current directory,
It is.
> which isn't really worth mentioning.
Agreed.
I'll post a re-roll if people prefer the current behavior over relative
to $GIT_DIR.
Thanks for reviewing,
Richard
>
> -Peff
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4845 bytes --]
^ permalink raw reply
* Re: [PATCH v4 14/14] mergetool: fix running in subdir when rerere enabled
From: Richard Hansen @ 2017-01-10 17:09 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, davvid, sbeller, simon, gitster
In-Reply-To: <3a09e318-f14b-5f14-a992-3bd045f0a4c6@kdbg.org>
[-- Attachment #1: Type: text/plain, Size: 3164 bytes --]
On 2017-01-10 01:17, Johannes Sixt wrote:
> Am 10.01.2017 um 00:29 schrieb Richard Hansen:
>> The pathnames output by the 'git rerere remaining' command are
>> relative to the top-level directory but the 'git diff --name-only'
>> command expects its pathname arguments to be relative to the current
>> working directory. Run cd_to_toplevel before running 'git diff
>> --name-only' and adjust any relative pathnames so that 'git mergetool'
>> does not fail when run from a subdirectory with rerere enabled.
>>
>> This fixes a regression introduced in
>> 57937f70a09c12ef484c290865dac4066d207c9c (v2.11.0).
>>
>> Based-on-patch-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Richard Hansen <hansenr@google.com>
>> ---
>> git-mergetool.sh | 16 ++++++++++++++--
>> t/t7610-mergetool.sh | 7 ++++++-
>> 2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-mergetool.sh b/git-mergetool.sh
>> index b506896dc..cba6bbd05 100755
>> --- a/git-mergetool.sh
>> +++ b/git-mergetool.sh
>> @@ -454,6 +454,15 @@ main () {
>> merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
>> merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
>>
>> + prefix=$(git rev-parse --show-prefix) || exit 1
>> + cd_to_toplevel
>> +
>> + if test -n "$orderfile"
>> + then
>> + orderfile=$(git rev-parse --prefix "$prefix" -- "$orderfile") || exit 1
>> + orderfile=$(printf %s\\n "$orderfile" | sed -e 1d)
>
> Is the purpose of this complication only to detect errors of the git
> invocation?
Yes. I've been burned so many times by the shell not stopping when
there's an error that I always like to do things one step at a time with
error checking, even if it is less efficient.
> IMHO, we could dispense with that, but others might
> disagree. I am arguing because this adds yet another process; but it is
> only paid when -O is used, so...
>
>> + fi
>> +
>> if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR"
>> then
>> set -- $(git rerere remaining)
>> @@ -461,14 +470,17 @@ main () {
>> then
>> print_noop_and_exit
>> fi
>> + elif test $# -ge 0
>> + then
>> + files_quoted=$(git rev-parse --sq --prefix "$prefix" -- "$@") || exit 1
>> + eval "set -- $files_quoted"
>
> BTW, the --sq and eval business is not required here. At this point,
> $IFS = $'\n',
Whoa, really? That's surprising and feels wrong.
(BTW, I just noticed that guess_merge_tool sets IFS to a space without
resetting it afterward. That function is always run in a subshell so
there's no problem at the moment, but it's still a bit risky.)
> so
>
> set -- $(git rev-parse --sq --prefix "$prefix" -- "$@")
>
> will do. (Except that it would not detect errors.)
I think I would prefer to leave it as-is in case IFS is ever changed
back to the default.
>
>> + shift
>> fi
>
> As I don't see anything wrong with what you have written, these comments
> alone do not warrant another re-roll.
I'll reroll if I hear other votes in favor of changing. Thanks for
reviewing!
-Richard
>
> -- Hannes
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4845 bytes --]
^ permalink raw reply
* Re: [PATCH v5 0/4] Per-worktree config file support
From: Stefan Beller @ 2017-01-10 17:01 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git@vger.kernel.org, Junio C Hamano, Michael J Gruber,
Jens Lehmann, Lars Schneider, Michael Haggerty, Max Kirillov
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
On Tue, Jan 10, 2017 at 3:25 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Let's get this rolling again. To refresh your memory because it's half
> a year since v4 [1], this is about letting each worktree in multi
> worktree setup has their own config settings. The most prominent ones
> are core.worktree, used by submodules, and core.sparseCheckout.
Thanks for getting this rolling again.
>
> This time I'm not touching submodules at all. I'm leaving it in the
> good hands of "submodule people". All I'm providing is mechanism. How
> you use it is up to you. So the series benefits sparse checkout users
> only.
As one of the "submodule people", I have no complaints here.
>
> Not much has changed from v4, except that the migration to new config
> layout is done automatically _update_ a config variable with "git
> config --worktree".
>
> I think this one is more or less ready. I have an RFC follow-up patch
> about core.bare, but that could be handled separately.
I have read through the series and think the design is sound for worktrees
(though I have little knowledge about them).
---
Now even further:
So to build on top of this series, I'd like to make submodules usable
with worktrees (i.e. shared object store, only clone/fetch once and
all worktrees
benefit from it), the big question is how to get the underlying data
model right.
Would a submodule go into the superprojects
.git/worktrees/<worktree-name>/modules/<submodule-name>/
or rather
.git/modules<submodule-name>/worktrees/<worktree-name>
Or both (one of them being a gitlink file pointing at the other?)
I have not made up my mind, as I haven't laid out all cases that are
relevant here.
Thanks,
Stefan
>
> [1] http://public-inbox.org/git/20160720172419.25473-1-pclouds@gmail.com/
>
> Nguyễn Thái Ngọc Duy (4):
> config: read per-worktree config files
> config: --worktree for manipulating per-worktree config file
> config: automatically migrate to new config layout when --worktree is used
> t2029: add tests for per-worktree config
>
> Documentation/config.txt | 11 ++++-
> Documentation/git-config.txt | 26 ++++++++---
> Documentation/git-worktree.txt | 37 +++++++++++++++
> Documentation/gitrepository-layout.txt | 8 ++++
> builtin/config.c | 16 ++++++-
> cache.h | 2 +
> config.c | 7 +++
> environment.c | 1 +
> setup.c | 5 ++-
> t/t2029-worktree-config.sh (new +x) | 82 ++++++++++++++++++++++++++++++++++
> worktree.c | 40 +++++++++++++++++
> worktree.h | 6 +++
> 12 files changed, 230 insertions(+), 11 deletions(-)
> create mode 100755 t/t2029-worktree-config.sh
>
> --
> 2.8.2.524.g6ff3d78
>
^ permalink raw reply
* Re: [PATCH v5 2/4] config: --worktree for manipulating per-worktree config file
From: Stefan Beller @ 2017-01-10 16:52 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git@vger.kernel.org, Junio C Hamano, Michael J Gruber,
Jens Lehmann, Lars Schneider, Michael Haggerty, Max Kirillov
In-Reply-To: <20170110112524.12870-3-pclouds@gmail.com>
> + if (repository_format_worktree_config)
> + given_config_source.file = git_pathdup("config.worktree");
> + else if (worktrees[0] && worktrees[1]) {
> + die("BUG: migration is not supported yet");
> + } else
> + given_config_source.file = git_pathdup("config");
nit: inconsistent use uf braces for single statements here.
I mean the braces are needed in the BUG case, as otherwise we'd
have a dangling else, but then you could put the brace in the else case as well.
This nit doesn't warrant a reroll on its own.
> + free_worktrees(worktrees);
> + } else if (given_config_source.file) {
> if (!is_absolute_path(given_config_source.file) && prefix)
> given_config_source.file =
> xstrdup(prefix_filename(prefix,
> --
> 2.8.2.524.g6ff3d78
>
^ permalink raw reply
* Re: Refreshing index timestamps without reading content
From: Quentin Casasnovas @ 2017-01-10 14:17 UTC (permalink / raw)
To: Quentin Casasnovas
Cc: Vegard Nossum, Junio C Hamano, Duy Nguyen, Git Mailing List
In-Reply-To: <20170109155537.GG7000@chrystal.oracle.com>
[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]
On Mon, Jan 09, 2017 at 04:55:37PM +0100, Quentin Casasnovas wrote:
> On Mon, Jan 09, 2017 at 07:01:36AM -0800, Junio C Hamano wrote:
> > Duy Nguyen <pclouds@gmail.com> writes:
> >
> > > On Thu, Jan 5, 2017 at 6:23 PM, Quentin Casasnovas
> > > <quentin.casasnovas@oracle.com> wrote:
> > >> Is there any way to tell git, after the git ls-tree command above, to
> > >> refresh its stat cache information and trust us that the file content has
> > >> not changed, as to avoid any useless file read (though it will obviously
> > >> will have to stat all of them, but that's not something we can really
> > >> avoid)
> > >
> > > I don't think there's any way to do that, unfortunately.
> >
> > Lose "unfortunately".
> >
> > >> If not, I am willing to implement a --assume-content-unchanged to the git
> > >> update-index if you guys don't see something fundamentally wrong with this
> > >> approach.
> > >
> > > If you do that, I think you should go with either of the following options
> > >
> > > - Extend git-update-index --index-info to take stat info as well (or
> > > maybe make a new option instead). Then you can feed stat info directly
> > > to git without a use-case-specific "assume-content-unchanged".
> > >
> > > - Add "git update-index --touch" that does what "touch" does. In this
> > > case, it blindly updates stat info to latest. But like touch, we can
> > > also specify mtime from command line if we need to. It's a bit less
> > > generic than the above option, but easier to use.
> >
> > Even if we assume that it is a good idea to let people muck with the
> > index like this, either of the above would be a usable addition,
> > because the cached stat information does not consist solely of
> > mtime.
> >
> > "git update-index --index-info" was invented for the case where a
> > user or a script _knows_ the object ID of the blob that _would_
> > result if a contents of a file on the filesystem were run through
> > hash-object. So from the interface's point of view, it may make
> > sense to teach it to take an extra/optional argument that is the
> > path to the file and take the stat info out of the named file when
> > the extra/optional argument was given.
> >
> > But that assumes that it is a good idea to do this in the first
> > place. It was deliberate design decision that setting the cached
> > stat info for the entry was protected behind actual content
> > comparison, and removing that protection will open the index to
> > abuse.
> >
>
> Hi Junio,
>
> Thanks for your feedback, appreciated :)
>
> I do understand how it would be possible for someone to shoot themselves in
> the feet with such option, but it solves real life use cases and improved
> build times very signficantly here.
>
> Another use case we have is setting up very lightweight linux work trees,
> by reflinking from a base work-tree. This allows for a completely
> different work-tree taking up almost no size at first, whereas using a
> shared clone or the recent worktree subcommand would "waste" ~500MB*:
>
> # linux-2.6 is a shared clone of a bare clone residing locally
> ~ $ cp --reflink -a linux-2.6 linux-2.6-reflinked
>
> # At this point, the mtime inside linux-2.6-reflinked are matching the
> # mtime of the source linux-2.6 (since we used the '-a' option of 'cp)
> ~ $ diff -u <(stat linux-2.6/README) <(stat linux-2.6-reflinked/README)
> --- /proc/self/fd/11 2017-01-09 16:34:04.523438942 +0100
> +++ /proc/self/fd/12 2017-01-09 16:34:04.523438942 +0100
> @@ -1,8 +1,8 @@
> - File: 'linux-2.6/README'
> + File: 'linux-2.6-reflinked/README'
> Size: 18372 Blocks: 40 IO Block: 4096 regular file
> -Device: fd00h/64768d Inode: 268467090 Links: 1
> +Device: fd00h/64768d Inode: 805970606 Links: 1
> Access: (0644/-rw-r--r--) Uid: ( 1000/ quentin) Gid: ( 1000/ quentin)
> Access: 2017-01-09 12:04:15.317758718 +0100
> Modify: 2017-01-09 12:04:12.566758772 +0100
> -Change: 2017-01-09 12:04:12.566758772 +0100
> +Change: 2017-01-09 16:29:48.305444003 +0100
> Birth:
>
> # Now let's check how long it takes to refresh the index from the source
> # and destination..
> ~/linux-2.6 $ time git update-index --refresh
> git update-index --refresh 0.04s user 0.08s system 204% cpu 0.058 total
> ~~~~~~~~~~~
> ~/linux-2.6-reflinked $ time git update-index --refresh
> git update-index --refresh 2.40s user 1.43s system 38% cpu 10.003 total
> ~~~~~~~~~~~~
>
After discussing this with my friend Vegard, he found the core.checkStat
config which, if set to 'minimal', ignores the inode number which is enough
for the above use case to work just fine - so please excuse my ignorance!
For the initial problem I had when changing the mtime of all the files in
the tree, I should be able to change the mtime of the object files instead,
hence I don't really need the patch I sent earlier.
Sorry for the wasted time! :)
Q
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [musl] Re: Test failures when Git is built with libpcre and grep is built without it
From: Szabolcs Nagy @ 2017-01-10 11:40 UTC (permalink / raw)
To: musl; +Cc: Jeff King, Andreas Schwab, git, A. Wilcox
In-Reply-To: <5874B942.7070402@adelielinux.org>
* A. Wilcox <awilfox@adelielinux.org> [2017-01-10 04:36:50 -0600]:
> On 09/01/17 15:33, Jeff King wrote:
> > The problem is that we are expecting the regex "\x{2b}" to complain
> > in regcomp() (as an ERE), but it doesn't. And that probably _is_
> > related to musl, which is providing the libc regex (I know this
> > looks like a pcre test, but it's checking that "-P -E" overrides
> > the pcre option with "-E").
> >
> > I'm not sure if musl is wrong for failing to complain about a
> > bogus regex. Generally making something that would break into
> > something that works is an OK way to extend the standard. So our
> > test is at fault for assuming that the regex will fail. I guess
\x is undefined in posix and musl is based on tre which
supports \x{hexdigits} in ere.
(i think some bsd platforms use tre as libc regex so
i'm surprised musl is the first to run into this.)
> > we'd need to find some more exotic syntax that pcre supports, but
> > that ERE doesn't. Maybe "(?:)" or something.
i think you would have to use something that's invalid
in posix ere, ? after empty expression is undefined,
not an error so "(?:)" is a valid ere extension.
since most syntax is either defined or undefined in ere
instead of being invalid, distinguishing pcre using
syntax is not easy.
there are semantic differences in subexpression matching:
leftmost match has higher priority in pcre, longest match
has higher priority in ere.
$ echo ab | grep -o -E '(a|ab)'
ab
$ echo ab | grep -o -P '(a|ab)'
a
unfortunately grep -o is not portable.
^ permalink raw reply
* Re: [PATCH v5 0/4] Per-worktree config file support
From: Duy Nguyen @ 2017-01-10 11:41 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
Stefan Beller, Michael Haggerty, Max Kirillov,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
On Tue, Jan 10, 2017 at 6:25 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Not much has changed from v4, except that the migration to new config
> layout is done automatically _update_ a config variable with "git
> config --worktree".
I accidentally two words that may make it hard to understand this
sentence. It should be "... is done automatically when you update.."
--
Duy
^ permalink raw reply
* [PATCH/RFC 5/4] Redefine core.bare in multiple working tree setting
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:33 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
When core.bare was added, time was simpler, we only had one worktree
associated to one repository. The situation gets a bit complicated when
multiple worktrees are added. If core.bare is set in the per-repo config
file, should all worktrees see this variable?
Since core.bare affects worktree-related commands (e.g. you are not
supposed to run "git status" when core.bare is true because no worktree
is supposed to link to the repository), when multi worktree is added,
core.bare is evaluated true by the main worktree only. Other worktrees
simply do not see core.bare even if it's there.
With per-worktree configuration in place, core.bare is moved to main
worktree's private config file. But it does not really make sense
because this is about _repository_. Instead we could leave core.bare in
the per-repo config and change/extend its definition from:
If true this repository is assumed to be 'bare' and has no working
directory associated with it.
to
If true this repository is assumed to be 'bare' and has no _main_
working directory associated with it.
In other words, linked worktrees are not covered by core.bare. This
definition is the same as before when it comes to single worktree setup.
A plus of this definition is, it allows a setup where we only have
linked worktrees (e.g. core.bare set to true, and the main repo is
tucked somewhere safe), which makes all worktrees equal again because
"the special one" is gone.
This patch is incomplete. I need to go through all is_bare_repository()
calls and adjust their behavior. But I wanted to run the idea through
the community first..
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 2 +-
Documentation/git-worktree.txt | 7 +++----
t/t2029-worktree-config.sh | 4 ++--
worktree.c | 6 ------
4 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c508386..ff146be 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -484,7 +484,7 @@ core.preferSymlinkRefs::
expect HEAD to be a symbolic link.
core.bare::
- If true this repository is assumed to be 'bare' and has no
+ If true this repository is assumed to be 'bare' and has no main
working directory associated with it. If this is the case a
number of commands that require a working directory will be
disabled, such as linkgit:git-add[1] or linkgit:git-merge[1].
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index f5aad0a..a331d0a 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -161,7 +161,7 @@ them to the `config.worktree` of the main working directory. You may
also take this opportunity to move other configuration that you do not
want to share to all working directories:
- - `core.worktree` and `core.bare` should never be shared
+ - `core.worktree` should never be shared
- `core.sparseCheckout` is recommended per working directory, unless
you are sure you always use sparse checkout for all working
@@ -169,9 +169,8 @@ want to share to all working directories:
When `git config --worktree` is used to set a configuration variable
in multiple working directory setup, `extensions.worktreeConfig` will
-be automatically set. The two variables `core.worktree` and
-`core.bare` if present will be moved to `config.worktree` of the main
-working tree.
+be automatically set. The variable `core.worktree` if present will be
+moved to `config.worktree` of the main working tree.
DETAILS
-------
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
index 4ebdf13..dc84c94 100755
--- a/t/t2029-worktree-config.sh
+++ b/t/t2029-worktree-config.sh
@@ -70,13 +70,13 @@ test_expect_success 'config.worktree no longer read without extension' '
cmp_config -C wt2 shared this.is
'
-test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+test_expect_success 'config --worktree migrate core.worktree' '
git config core.bare true &&
git config --worktree foo.bar true &&
cmp_config true extensions.worktreeConfig &&
cmp_config true foo.bar &&
cmp_config true core.bare &&
- ! git -C wt1 config core.bare
+ cmp_config -C wt1 true core.bare
'
test_done
diff --git a/worktree.c b/worktree.c
index d8c9d85..c07cc50 100644
--- a/worktree.c
+++ b/worktree.c
@@ -395,12 +395,6 @@ void migrate_worktree_config(void)
read_repository_format(&format, main_path.buf);
assert(format.worktree_config == 0);
- if (format.is_bare >= 0) {
- git_config_set_in_file(worktree_path.buf,
- "core.bare", "true");
- git_config_set_in_file(main_path.buf,
- "core.bare", NULL);
- }
if (format.work_tree) {
git_config_set_in_file(worktree_path.buf,
"core.worktree",
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 4/4] t2029: add tests for per-worktree config
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t2029-worktree-config.sh (new +x) | 82 +++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100755 t/t2029-worktree-config.sh
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000..4ebdf13
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+cmp_config() {
+ if [ "$1" = "-C" ]; then
+ shift &&
+ GD="-C $1" &&
+ shift
+ else
+ GD=
+ fi &&
+ echo "$1" >expected &&
+ shift &&
+ git $GD config "$@" >actual &&
+ test_cmp expected actual
+}
+
+test_expect_success 'setup' '
+ test_commit start &&
+ git config --worktree per.worktree is-ok &&
+ git worktree add wt1 &&
+ git worktree add wt2 &&
+ git config --worktree per.worktree is-ok &&
+ cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+ git config this.is shared &&
+ cmp_config shared this.is &&
+ cmp_config -C wt1 shared this.is &&
+ cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+ git -C wt1 config that.is also-shared &&
+ cmp_config also-shared that.is &&
+ cmp_config -C wt1 also-shared that.is &&
+ cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+ git config --worktree this.is for-main &&
+ cmp_config for-main this.is &&
+ cmp_config -C wt1 shared this.is &&
+ cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+ git -C wt1 config --worktree this.is for-wt1 &&
+ cmp_config for-main this.is &&
+ cmp_config -C wt1 for-wt1 this.is &&
+ cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+ git config core.bare true &&
+ cmp_config true core.bare &&
+ cmp_config -C wt1 true core.bare &&
+ cmp_config -C wt2 true core.bare &&
+ git config --unset core.bare
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+ git config --unset extensions.worktreeConfig &&
+ cmp_config shared this.is &&
+ cmp_config -C wt1 shared this.is &&
+ cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+ git config core.bare true &&
+ git config --worktree foo.bar true &&
+ cmp_config true extensions.worktreeConfig &&
+ cmp_config true foo.bar &&
+ cmp_config true core.bare &&
+ ! git -C wt1 config core.bare
+'
+
+test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 3/4] config: automatically migrate to new config layout when --worktree is used
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
It's not fun to ask the user to set extensions.worktreeConfig manually.
It's error-prone too. So we do it automatically whenever anybody sets a
per-worktree config with "git config" (support for builtin commands is
coming later).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-worktree.txt | 6 ++++++
builtin/config.c | 3 ++-
worktree.c | 40 ++++++++++++++++++++++++++++++++++++++++
worktree.h | 6 ++++++
4 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 329a673..f5aad0a 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -167,6 +167,12 @@ want to share to all working directories:
you are sure you always use sparse checkout for all working
directories.
+When `git config --worktree` is used to set a configuration variable
+in multiple working directory setup, `extensions.worktreeConfig` will
+be automatically set. The two variables `core.worktree` and
+`core.bare` if present will be moved to `config.worktree` of the main
+working tree.
+
DETAILS
-------
Each linked working tree has a private sub-directory in the repository's
diff --git a/builtin/config.c b/builtin/config.c
index 7d390af..9dafefd 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -533,7 +533,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
if (repository_format_worktree_config)
given_config_source.file = git_pathdup("config.worktree");
else if (worktrees[0] && worktrees[1]) {
- die("BUG: migration is not supported yet");
+ migrate_worktree_config();
+ given_config_source.file = git_pathdup("config.worktree");
} else
given_config_source.file = git_pathdup("config");
free_worktrees(worktrees);
diff --git a/worktree.c b/worktree.c
index eb61212..d8c9d85 100644
--- a/worktree.c
+++ b/worktree.c
@@ -380,3 +380,43 @@ const struct worktree *find_shared_symref(const char *symref,
return existing;
}
+
+void migrate_worktree_config(void)
+{
+ struct strbuf worktree_path = STRBUF_INIT;
+ struct strbuf main_path = STRBUF_INIT;
+ struct repository_format format;
+
+ assert(repository_format_worktree_config == 0);
+
+ strbuf_git_common_path(&worktree_path, "config.worktree");
+ strbuf_git_path(&main_path, "config");
+
+ read_repository_format(&format, main_path.buf);
+ assert(format.worktree_config == 0);
+
+ if (format.is_bare >= 0) {
+ git_config_set_in_file(worktree_path.buf,
+ "core.bare", "true");
+ git_config_set_in_file(main_path.buf,
+ "core.bare", NULL);
+ }
+ if (format.work_tree) {
+ git_config_set_in_file(worktree_path.buf,
+ "core.worktree",
+ format.work_tree);
+ git_config_set_in_file(main_path.buf,
+ "core.worktree", NULL);
+ }
+
+ git_config_set_in_file(main_path.buf,
+ "extensions.worktreeConfig", "true");
+ if (format.version == 0)
+ git_config_set_in_file(main_path.buf,
+ "core.repositoryFormatVersion", "1");
+
+ repository_format_worktree_config = 1;
+
+ strbuf_release(&main_path);
+ strbuf_release(&worktree_path);
+}
diff --git a/worktree.h b/worktree.h
index d59ce1f..cf82676 100644
--- a/worktree.h
+++ b/worktree.h
@@ -76,4 +76,10 @@ extern const char *worktree_git_path(const struct worktree *wt,
const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+/*
+ * Called to add extensions.worktreeConfig to $GIT_DIR/config and move
+ * main worktree specific config variables to $GIT_DIR/config.worktree.
+ */
+extern void migrate_worktree_config(void);
+
#endif
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 2/4] config: --worktree for manipulating per-worktree config file
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
As noted in the previous commit, "git config" without options will read
both per-worktree and per-repo by default. --worktree is needed to read
just per-worktree config. Writing goes to per-repo by default though,
unless --worktree is given.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-config.txt | 22 +++++++++++++++-------
builtin/config.c | 15 ++++++++++++++-
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 806873c..ead33a8 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -47,13 +47,15 @@ checks or transformations are performed on the value.
When reading, the values are read from the system, global and
repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
When writing, the new value is written to the repository local
configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
This command will fail with non-zero status upon error. Some exit
codes are:
@@ -133,6 +135,11 @@ from all available files.
+
See also <<FILES>>.
+--worktree::
+ Similar to `--local` except that `.git/config.worktree` is
+ read from or written to if `extensions.worktreeConfig` is
+ present. If not it's the same as `--local`.
+
-f config-file::
--file config-file::
Use the given config file instead of the one specified by GIT_CONFIG.
@@ -275,9 +282,10 @@ configuration file. Note that this also affects options like `--replace-all`
and `--unset`. *'git config' will only ever change one file at a time*.
You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
ENVIRONMENT
diff --git a/builtin/config.c b/builtin/config.c
index 05843a0..7d390af 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -4,6 +4,7 @@
#include "parse-options.h"
#include "urlmatch.h"
#include "quote.h"
+#include "worktree.h"
static const char *const builtin_config_usage[] = {
N_("git config [<options>]"),
@@ -23,6 +24,7 @@ static char key_delim = ' ';
static char term = '\n';
static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
static struct git_config_source given_config_source;
static int actions, types;
static int end_null;
@@ -56,6 +58,7 @@ static struct option builtin_config_options[] = {
OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+ OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
OPT_GROUP(N_("Action")),
@@ -490,6 +493,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
PARSE_OPT_STOP_AT_NON_OPTION);
if (use_global_config + use_system_config + use_local_config +
+ use_worktree_config +
!!given_config_source.file + !!given_config_source.blob > 1) {
error("only one config file at a time.");
usage_with_options(builtin_config_usage, builtin_config_options);
@@ -524,7 +528,16 @@ int cmd_config(int argc, const char **argv, const char *prefix)
given_config_source.file = git_etc_gitconfig();
else if (use_local_config)
given_config_source.file = git_pathdup("config");
- else if (given_config_source.file) {
+ else if (use_worktree_config) {
+ struct worktree **worktrees = get_worktrees(0);
+ if (repository_format_worktree_config)
+ given_config_source.file = git_pathdup("config.worktree");
+ else if (worktrees[0] && worktrees[1]) {
+ die("BUG: migration is not supported yet");
+ } else
+ given_config_source.file = git_pathdup("config");
+ free_worktrees(worktrees);
+ } else if (given_config_source.file) {
if (!is_absolute_path(given_config_source.file) && prefix)
given_config_source.file =
xstrdup(prefix_filename(prefix,
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 1/4] config: read per-worktree config files
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>
A new repo extension is added, worktreeConfig. When it is present:
- Repository config reading by default includes $GIT_DIR/config _and_
$GIT_DIR/config.worktree. "config" file remains shared in multiple
worktree setup.
- The special treatment for core.bare and core.worktree, to stay
effective only in main worktree, is gone. These config files are
supposed to be in config.worktree.
This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).
This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).
"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes (not implemented in this patch) still go to
"config", not "config.worktree". A new option --worktree is added for
that (*).
Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".
This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.
(*) "git config --worktree" points back to "config" file when this
extension is not present so that it works in any setup.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 11 +++++++++--
Documentation/git-config.txt | 4 ++++
Documentation/git-worktree.txt | 31 +++++++++++++++++++++++++++++++
Documentation/gitrepository-layout.txt | 8 ++++++++
cache.h | 2 ++
config.c | 7 +++++++
environment.c | 1 +
setup.c | 5 ++++-
8 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 30cb946..c508386 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
------------------
The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) are each
+repository is used to store the configuration for that repository, and
`$HOME/.gitconfig` is used to store a per-user configuration as
fallback values for the `.git/config` file. The file `/etc/gitconfig`
can be used to store a system-wide default configuration.
@@ -264,6 +265,12 @@ advice.*::
show directions on how to proceed from the current state.
--
+extensions.worktreeConfig::
+ If set, by default "git config" reads from both "config" and
+ "config.worktree" file in that order. In multiple working
+ directory mode, "config" file is shared while
+ "config.worktree" is per-working directory.
+
core.fileMode::
Tells Git if the executable bit of files in the working tree
is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 83f86b9..806873c 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -253,6 +253,10 @@ $XDG_CONFIG_HOME/git/config::
$GIT_DIR/config::
Repository specific configuration file.
+$GIT_DIR/config.worktree::
+ This is optional and is only searched when
+ `extensions.worktreeConfig` is present in $GIT_DIR/config.
+
If no further options are given, all reading options will read all of these
files that are available. If the global or the system-wide configuration
file are not available they will be ignored. If the repository configuration
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19..329a673 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -136,6 +136,37 @@ working trees, it can be used to identify worktrees. For example if
you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
then "ghi" or "def/ghi" is enough to point to the former working tree.
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+directories. If the config variables `core.bare` or `core.worktree`
+are already present in the config file, they will be applied to the
+main working directory only.
+
+In order to have configuration specific to working directories, you
+can turn on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Git before
+version 2.13 will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and
+`core.worktree` is gone. If you have them before, you need to move
+them to the `config.worktree` of the main working directory. You may
+also take this opportunity to move other configuration that you do not
+want to share to all working directories:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working directory, unless
+ you are sure you always use sparse checkout for all working
+ directories.
+
DETAILS
-------
Each linked working tree has a private sub-directory in the repository's
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index a5f99cb..fee2557 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
used instead.
+config.worktree::
+ Working directory specific configuration file for the main
+ working directory in multiple working directory setup (see
+ linkgit:git-worktree[1]).
+
branches::
A slightly deprecated way to store shorthands to be used
to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -276,6 +281,9 @@ worktrees/<id>/link::
file. It is used to detect if the linked repository is
manually removed.
+worktrees/<id>/config.worktree::
+ Working directory specific configuration file.
+
SEE ALSO
--------
linkgit:git-init[1],
diff --git a/cache.h b/cache.h
index a50a61a..c621a35 100644
--- a/cache.h
+++ b/cache.h
@@ -777,10 +777,12 @@ extern int grafts_replace_parents;
#define GIT_REPO_VERSION 0
#define GIT_REPO_VERSION_READ 1
extern int repository_format_precious_objects;
+extern int repository_format_worktree_config;
struct repository_format {
int version;
int precious_objects;
+ int worktree_config;
int is_bare;
char *work_tree;
struct string_list unknown_extensions;
diff --git a/config.c b/config.c
index 83fdecb..a461e68 100644
--- a/config.c
+++ b/config.c
@@ -1307,6 +1307,13 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
if (repo_config && !access_or_die(repo_config, R_OK, 0))
ret += git_config_from_file(fn, repo_config, data);
+ if (repository_format_worktree_config) {
+ char *path = git_pathdup("config.worktree");
+ if (!access_or_die(path, R_OK, 0))
+ ret += git_config_from_file(fn, path, data);
+ free(path);
+ }
+
current_parsing_scope = CONFIG_SCOPE_CMDLINE;
if (git_config_from_parameters(fn, data) < 0)
die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 0935ec6..df1be15 100644
--- a/environment.c
+++ b/environment.c
@@ -26,6 +26,7 @@ int warn_ambiguous_refs = 1;
int warn_on_object_refname_ambiguity = 1;
int ref_paranoia = -1;
int repository_format_precious_objects;
+int repository_format_worktree_config;
const char *git_commit_encoding;
const char *git_log_output_encoding;
const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index fe572b8..69efe45 100644
--- a/setup.c
+++ b/setup.c
@@ -389,6 +389,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
;
else if (!strcmp(ext, "preciousobjects"))
data->precious_objects = git_config_bool(var, value);
+ else if (!strcmp(ext, "worktreeconfig"))
+ data->worktree_config = git_config_bool(var, value);
else
string_list_append(&data->unknown_extensions, ext);
} else if (strcmp(var, "core.bare") == 0) {
@@ -432,8 +434,9 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
}
repository_format_precious_objects = candidate.precious_objects;
+ repository_format_worktree_config = candidate.worktree_config;
string_list_clear(&candidate.unknown_extensions, 0);
- if (!has_common) {
+ if (!has_common || repository_format_worktree_config) {
if (candidate.is_bare != -1) {
is_bare_repository_cfg = candidate.is_bare;
if (is_bare_repository_cfg == 1)
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH v5 0/4] Per-worktree config file support
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
mhagger, max, Nguyễn Thái Ngọc Duy
Let's get this rolling again. To refresh your memory because it's half
a year since v4 [1], this is about letting each worktree in multi
worktree setup has their own config settings. The most prominent ones
are core.worktree, used by submodules, and core.sparseCheckout.
This time I'm not touching submodules at all. I'm leaving it in the
good hands of "submodule people". All I'm providing is mechanism. How
you use it is up to you. So the series benefits sparse checkout users
only.
Not much has changed from v4, except that the migration to new config
layout is done automatically _update_ a config variable with "git
config --worktree".
I think this one is more or less ready. I have an RFC follow-up patch
about core.bare, but that could be handled separately.
[1] http://public-inbox.org/git/20160720172419.25473-1-pclouds@gmail.com/
Nguyễn Thái Ngọc Duy (4):
config: read per-worktree config files
config: --worktree for manipulating per-worktree config file
config: automatically migrate to new config layout when --worktree is used
t2029: add tests for per-worktree config
Documentation/config.txt | 11 ++++-
Documentation/git-config.txt | 26 ++++++++---
Documentation/git-worktree.txt | 37 +++++++++++++++
Documentation/gitrepository-layout.txt | 8 ++++
builtin/config.c | 16 ++++++-
cache.h | 2 +
config.c | 7 +++
environment.c | 1 +
setup.c | 5 ++-
t/t2029-worktree-config.sh (new +x) | 82 ++++++++++++++++++++++++++++++++++
worktree.c | 40 +++++++++++++++++
worktree.h | 6 +++
12 files changed, 230 insertions(+), 11 deletions(-)
create mode 100755 t/t2029-worktree-config.sh
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* Re: git branch -D doesn't work with deleted worktree
From: Duy Nguyen @ 2017-01-10 10:47 UTC (permalink / raw)
To: Jacob Keller; +Cc: Stefan Beller, Roland Illig, git@vger.kernel.org
In-Reply-To: <CA+P7+xqOrRwJsnskfGQpLYyQFjce=4z24zuhrEBd2P4gBLh0Qw@mail.gmail.com>
On Tue, Jan 10, 2017 at 12:08 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Mon, Jan 9, 2017 at 2:07 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Mon, Jan 9, 2017 at 10:44 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> Why not just update the documentation to be "when you are done with a
>>> work tree you can delete it and then run git worktree prune"?
>>
>> The document does say that (a bit verbose though):
>>
>> When you are done with a linked working tree you can simply delete it.
>> The working tree's administrative files in the repository (see
>> "DETAILS" below) will eventually be removed automatically (see
>> `gc.worktreePruneExpire` in linkgit:git-config[1]), or you can run
>> `git worktree prune` in the main or any linked working tree to
>> clean up any stale administrative files.
>> --
>> Duy
>
> Right, so maybe we can make it more clear since it's not quite as
> simple as "deleting the work tree" because of stuff like stale
> branches..
Patches are welcome. I wrote that paragraph and you could clearly see
its "quality" (from user perspective).
> or would it be worth re-scanning worktrees when we do
> branch deletion? (obviously ignoring the ones that are marked as on
> removable media)
Probably. We run "git worktree prune" as part of "git gc" and that
command is automatically called in a couple places. Adding another
"git gc" here probably does not hurt. However it could trigger a full
repack and make "git branch" hang for a few seconds...
And no I don't want to single out "git worktree prune" to be called in
git-branch. We should either go through git-gc or none. We could check
if a worktree is deleted and suggest the user to run "git gc" though,
but it's getting too complicated when "git worktree remove" will soon
be the recommended way of deleting a worktree. So.. I don't know..
--
Duy
^ permalink raw reply
* Re: Test failures when Git is built with libpcre and grep is built without it
From: A. Wilcox @ 2017-01-10 10:36 UTC (permalink / raw)
To: Jeff King, Andreas Schwab; +Cc: git, musl
In-Reply-To: <20170109213303.4rupe5cqwejfp6af@sigill.intra.peff.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
(Hi, musl developers! Read on for some information regarding what
began as a test failure during packaging of Git 2.7.3 that appears to
be related to musl's regexp library.)
On 09/01/17 05:27, Jeff King wrote:
> Are you trying with a version of git older than v2.7.x?
We are using 2.7.3. I will admit the version is a bit old, but it is
working on all other arches (and it took an inordinate amount of time
to spin up PowerPC - hooray for toolchain issues).
On 09/01/17 15:33, Jeff King wrote:
> On Mon, Jan 09, 2017 at 02:05:44PM +0100, Andreas Schwab wrote:
>> You need to quote the regexp argument, see the line starting
>> with "test_must_fail" above.
>
> Oh, duh. I checked that the line in the test was quoted
Guilty of this as well. Sorry about that. With the proper
invocation, I receive a success:
elaine trash directory.t7810-grep # git grep -G -F -P -E
"a\x{2b}b\x{2a}c" ab
error: cannot run less: No such file or directory
ab:a+b*c
elaine trash directory.t7810-grep # echo $?
0
Haven't managed to build less(1) for PowerPC yet, but it does seem to
work when quoted. Yikes!
> The problem is that we are expecting the regex "\x{2b}" to complain
> in regcomp() (as an ERE), but it doesn't. And that probably _is_
> related to musl, which is providing the libc regex (I know this
> looks like a pcre test, but it's checking that "-P -E" overrides
> the pcre option with "-E").
>
> I'm not sure if musl is wrong for failing to complain about a
> bogus regex. Generally making something that would break into
> something that works is an OK way to extend the standard. So our
> test is at fault for assuming that the regex will fail. I guess
> we'd need to find some more exotic syntax that pcre supports, but
> that ERE doesn't. Maybe "(?:)" or something.
>
> -Peff
>
I am cc:ing in the musl development list to see what their thoughts are.
Thanks for the help so far; I really appreciate it. Hopefully we can
resolve this in a way that makes musl and Git's test suite both more
correct, if necessary.
Happy new year!
- --arw
- --
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJYdLk+AAoJEMspy1GSK50Uz9kP/2JUSzSSXkKLy3788aAjgBOq
aTPBqKXLCvnTeBpsymXoGueaJzgDnYP6Xi9tUb/j4JAXqaJKGHXTgz8ixsFQJ4SJ
p7NN1JZXsIVGKWQHcxvEXEIRmBK7T9BQ2Hq6qUuk3n4PM6lbD1Ur3G1rUqIM/z76
5cSkvwgvGuOVYXLwSTTprb6EbbZc6WTgcDECFIl4z7eJ3GihChg+EgH4cTDt3xLD
9VHx++hWoEZXued0g0myENjBMeCiFOpYw1bzxn7d2s8masx/If2TkK0CLdexy2ti
hFV/F7g8etgZSdmrmiaJIudTsY6p46/sm/VmGm+8yPIeR8/8EOBlyhKhz4EidNtA
2dpUUJW/RD4uz9yVNjmWhIHaL10weuNLQLd/Tt5O/0dG422vhJxOwwJL9Vhxw70y
1PprD11+ZUJeWsz91C/Bl9CqHqljDP00QX8w/dbHCDsjeKgfUdksy2iKlHBG0sx3
CbL0EcHeE9BLmqmJi9ED7w78cTmNryemK29WFHUGSwzR99Rv9QlMn+4GCF1s0O3N
UKPCueLpPcNvRK+1WHkAwUC0iV97Un3WI6kyExs9khygIHRd1y/yRhbpxibbz337
FJfOTcT7e/YqML8Nb8EdwECRhdGX5u1tKqaOfBwLC62SKGTPi5qXpjaSPPJcJsrZ
phxFt6a45PPECV3ZsEO1
=PMkh
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
From: Jeff King @ 2017-01-10 9:35 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git discussion list
In-Reply-To: <ce6f98a4-1fb7-aa4b-2efb-78d8f49397a7@alum.mit.edu>
On Sun, Jan 08, 2017 at 10:52:25AM +0100, Michael Haggerty wrote:
> > I see the symmetry and simplicity in allowing the user to specify a full
> > range. But it also seems like it's easy to make a mistake that's going
> > to want to test a lot of commits. I wonder if it should complain when
> > there's no lower bound to the commit range. Or alternatively, if there's
> > a single positive reference, treat it as a lower bound, with HEAD as the
> > upper bound (which is vaguely rebase-like).
>
> I see how this might be unexpected, and it's definitely inconvenient at
> some times (like when you want to test a single commit). I thought it
> would be nice to allow arbitrary `rev-list` expressions (albeit
> currently only a single word), but I think that you are right that other
> semantics would be more convenient.
>
> I'm thinking of maybe
>
> * If an argument matches `*..*`, pass it to `rev-list` (like now).
>
> * Otherwise, treat each argument as a single commit/tree (i.e., pass it
> to `rev-parse`).
>
> * If no argument is specified, test `@{u}..` (assuming that an
> upstream is configured). Though actually, this won't be as
> convenient as it sounds, because (a) `git test` is often run
> in a separate worktree, and (2) on errors, it currently leaves the
> repository with a detached `HEAD`.
>
> * Support a `--stdin` option, to read a list of commits/trees to test
> from standard input. By this mechanism, users could use arbitrary
> `rev-list` commands to choose what to test.
That seems quite reasonable to me. The rebase semantics of "a single
argument is my upstream" is convenient, but I think it also confuses
people. Yours seem very simple to explain.
It doesn't allow complex stuff like "git test ^foo ^bar HEAD", but I
don't think "git rev-list ^foo ^bar HEAD | git test --stdin" is really
so bad for complicated cases like that (if anybody would even ever want
such a thing).
> Aside: It would be nice if `git notes` had a subcommand to initialize a
> note reference with an empty tree. (I know how to do it longhand, but
> it's awkward and it should be possible to do it via the `notes` interface.)
>
> I think ideally `git notes add` would look for pre-existing notes, and:
>
> * If none are found, create an empty notes reference.
>
> * If pre-existing notes are found and there was no existing test with
> that name, probably just leave the old notes in place.
>
> * If pre-existing notes are found and there was already a test with
> that name but a different command, perhaps insist that the user
> decide explicitly whether to forget the old results or continue using
> them. This might help users avoid the mistake of re-using old results
> even if they change the manner of testing.
I'm not quite sure what you mean here. By "test" and "command", do you
mean the test name that is used in the notes ref, and the command that
it is defined as?
In the notes-cache.c subsystem, the commit message stores a validity
token which must match in order to use the cache. You could do something
similar here (store the executed command in the commit message, and
invalidate the cache the user has changed the command). The notes-cache
stuff isn't available outside of the C code, though. You could either
expose it, or just do something similar longhand.
Thinking about it, though, I did notice that the tree sha1 is not the
only input to the cache. Things like config.mak (not to mention the
system itself) contribute to the test results. So no system will ever be
perfect, and it seems like just making an easy way to force a retest (or
just invalidate the whole cache) would be sufficient.
But maybe I totally missed what you were trying to say here.
> > It would be even easier if I could just repeat my range and only re-test
> > the "bad" commits. It was then that I decided to actually read the rest
> > of "git test help range" and see that you already wrote such an option,
> > cleverly hidden under the name "--retest".
>
> I think you were being ironic, but if not, would this have been easier
> to find under another name?
Sorry, the knob on my sarcasm module must have accidentally been knocked
down from "so thick it's obvious even by email".
Yes, I did just mean that I was being blind. You not only had added the
option already, but gave it the exact name I was about to propose it
under. :)
> Yeah, this is awkward, not only because many people don't know what to
> make of detached HEAD, but also because it makes it awkward in general
> to use `git test` in your main working directory. I didn't model this
> behavior on `git rebase --interactive`'s `edit` command, because I
> rarely use that. But I can see how they would fit together pretty well
> for people who like that workflow.
Yeah, after sleeping on it, I think it's best if "git test" remains
separate from that. It's primary function is to run the test, possibly
serving up a cached answer. So it would be perfectly reasonable to do:
git rebase -x 'git test range HEAD'
to accomplish the interactive testing (though perhaps just "git test"
would be a nice synonym for that).
And then "jump to a thing that I know is broken" becomes a separate
action, whether you are using "git test" or not. I wonder if we could
have:
git rebase -e HEAD~2
to do an interactive rebase, with "edit" for HEAD~2. I feel like
somebody even proposed that at one point, but I don't think it got
merged.
And then "git test fix" basically becomes:
git rebase -e "$(git test first-broken)"
though I think you'd still want a shorthand for that.
> > I think it should be possible to script the next steps, though.
> > Something like like "git test fix foo", which would:
> >
> > - expand the range of foo@{u}..foo to get the list of commits
> >
> > - see which ones were marked as broken
> >
> > - kick off an interactive rebase, but override GIT_EDITOR to mark any
> > broken ones as "edit" instead of "pick"
> >
> > That lets you separate the act of testing from the act of fixing. You
> > can let the tester run continuously in the background, and only stop to
> > fix when you're at an appropriate point in your work.
>
> I think you would usually only want to mark only the *first* broken
> commit as "edit", because often errors cascade to descendant commits.
Yeah, I had a similar thought. OTOH, if you didn't say "--keep-going",
you'd only have one breakage either way. I doubt it matters that much in
practice, and either behavior would probably be fine until somebody
comes up with a good use for the multi-commit case.
-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