* [PATCH v2 02/21] config: add git_config_get_split_index()
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
This new function will be used in a following commit to know
if we want to use the split index feature or not.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
cache.h | 1 +
config.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/cache.h b/cache.h
index a50a61a197..c126fe475e 100644
--- a/cache.h
+++ b/cache.h
@@ -1821,6 +1821,7 @@ extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
extern int git_config_get_untracked_cache(void);
+extern int git_config_get_split_index(void);
/*
* This is a hack for test programs like test-dump-untracked-cache to
diff --git a/config.c b/config.c
index 2eaf8ad77a..c1343bbb3e 100644
--- a/config.c
+++ b/config.c
@@ -1709,6 +1709,16 @@ int git_config_get_untracked_cache(void)
return -1; /* default value */
}
+int git_config_get_split_index(void)
+{
+ int val = -1;
+
+ if (!git_config_get_maybe_bool("core.splitindex", &val))
+ return val;
+
+ return -1; /* default value */
+}
+
NORETURN
void git_die_config_linenr(const char *key, const char *filename, int linenr)
{
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 01/21] config: mark an error message up for translation
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 83fdecb1bc..2eaf8ad77a 100644
--- a/config.c
+++ b/config.c
@@ -1701,8 +1701,8 @@ int git_config_get_untracked_cache(void)
if (!strcasecmp(v, "keep"))
return -1;
- error("unknown core.untrackedCache value '%s'; "
- "using 'keep' default value", v);
+ error(_("unknown core.untrackedCache value '%s'; "
+ "using 'keep' default value"), v);
return -1;
}
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 00/21] Add configuration options for split-index
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
Goal
~~~~
We want to make it possible to use the split-index feature
automatically by just setting a new "core.splitIndex" configuration
variable to true.
This can be valuable as split-index can help significantly speed up
`git rebase` especially along with the work to libify `git apply`
that has been merged to master
(see https://github.com/git/git/commit/81358dc238372793b1590efa149cc1581d1fbd98)
and is now in v2.11.
Design
~~~~~~
The design is similar as the previous work that introduced
"core.untrackedCache".
The new "core.splitIndex" configuration option can be either true,
false or undefined which is the default.
When it is true, the split index is created, if it does not already
exists, when the index is read. When it is false, the split index is
removed if it exists, when the index is read. Otherwise it is left as
is.
Along with this new configuration variable, the two following options
are also introduced:
- splitIndex.maxPercentChange
This is to avoid having too many changes accumulating in the split
index while in split index mode. The git-update-index
documentation says:
If split-index mode is already enabled and `--split-index` is
given again, all changes in $GIT_DIR/index are pushed back to
the shared index file.
but it is probably better to not expect the user to think about it
and to have a mechanism that pushes back all changes to the shared
index file automatically when some threshold is reached.
The default threshold is when the number of entries in the split
index file reaches 20% of the number of entries in the shared
index file. The new "splitIndex.maxPercentChange" config option
lets people tweak this value.
- splitIndex.sharedIndexExpire
To make sure that old sharedindex files are eventually removed
when a new one has been created, we "touch" the shared index file
every time a split index file using the shared index file is
either created or read from. Then we can delete shared indexes
with an mtime older than one week (by default), when we create a
new shared index file. The new "splitIndex.sharedIndexExpire"
config option lets people tweak this grace period.
This idea was suggested by Duy in:
https://public-inbox.org/git/CACsJy8BqMFASHf5kJgUh+bd7XG98CafNydE964VJyPXz-emEvA@mail.gmail.com/
and after some experiments, I agree that it is much simpler than
what I thought could be done during our discussion.
Junio also thinks that we have to do "time-based GC" in:
https://public-inbox.org/git/xmqqeg33ccjj.fsf@gitster.mtv.corp.google.com/
Highlevel view of the patches in the series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Except for patch 1/21, there are 3 big steps, one for each new
configuration variable introduced.
The main difference between this patch series and the v1 patch series
sent last October is that Step 3 has a few new commits to also update
the mtime of the shared index file when a split index based on the
shared index file is read from.
- Patch 1/21 marks a message for translation. It is a new patch
and it can be applied separately. (The patch 1/19 in v1, which
was a typo fix, has been merged separately into master already.)
Step 1 is:
- Patches 2/21 to 5/21 introduce the functions that are reading
the "core.splitIndex" configuration variable and tweaking the
split index depending on its value.
- Patch 6/21 adds a few tests for the new feature.
- Patches 7/21 and 8/21 add some documentation for the new
feature.
The only change since v1 in this step is that some warning
messages in 5/21 have been marked for translation as suggested
by Duy.
Step 2 is:
- Patches 9/21 and 10/21 introduce the functions that are reading
the "splitIndex.maxPercentChange" configuration variable and
regenerating a new shared index file depending on its value.
- Patch 11/21 adds a few tests for the new feature.
- Patch 12/21 add some documentation for the new feature.
The changes since v1 in this step are:
- an error message has been marked for translation in 9/21,
- camelCase is used in the same error message as suggested by
Duy in 9/21,
- "return error(...)" is now used as suggested by Junio in
9/21,
- too_many_not_shared_entries() is now "static" as suggested
by Ramsay in 10/21,
- changes made in write_locked_index() have been reorganized
as suggested by Duy in 10/21,
Step 3 is:
- Patches 13/21 to 16/21 introduce the functions that are reading
the "splitIndex.sharedIndexExpire" configuration variable and
expiring old shared index files depending on its value.
- Patch 17/21 adds a few tests for the new feature.
- Patches 18/21 and 19/21 are new patches. They update the mtime
of the shared index file when a split index based on the shared
index file is read from. 18/21 is a refactoring to make the
actual change in 19/21 easier.
- Patches 20/21 and 21/21 add some documentation for the new
feature.
The changes since v1 in this step are:
- a warning was removed from 14/21 as suggested by Junio,
- code to touch the shared index file as been refactored into
a function in 14/21,
- a function has been renamed git_config_get_expiry() in 15/21
as suggested by Junio,
- error messages have been marked for translation in 16/21 as
suggested by Duy,
- "return error_errno(...)" is now used as suggested by Duy in
16/21,
- patches 18/21 and 19/21 are new following Duy's suggestion,
- patches 20/21 and 21/21 are updated to take changes made in
19/21 into account.
Links
~~~~~
This patch series is also available here:
https://github.com/chriscool/git/commits/config-split-index
The previous versions were:
RFC: https://github.com/chriscool/git/commits/config-split-index7
v1: https://github.com/chriscool/git/commits/config-split-index72
On the mailing list the related patch series and discussions were:
RFC: https://public-inbox.org/git/20160711172254.13439-1-chriscool@tuxfamily.org/
v1: https://public-inbox.org/git/20161023092648.12086-1-chriscool@tuxfamily.org/
Christian Couder (21):
config: mark an error message up for translation
config: add git_config_get_split_index()
split-index: add {add,remove}_split_index() functions
read-cache: add and then use tweak_split_index()
update-index: warn in case of split-index incoherency
t1700: add tests for core.splitIndex
Documentation/config: add information for core.splitIndex
Documentation/git-update-index: talk about core.splitIndex config var
config: add git_config_get_max_percent_split_change()
read-cache: regenerate shared index if necessary
t1700: add tests for splitIndex.maxPercentChange
Documentation/config: add splitIndex.maxPercentChange
sha1_file: make check_and_freshen_file() non static
read-cache: touch shared index files when used
config: add git_config_get_expiry() from gc.c
read-cache: unlink old sharedindex files
t1700: test shared index file expiration
read-cache: refactor read_index_from()
read-cache: use freshen_shared_index() in read_index_from()
Documentation/config: add splitIndex.sharedIndexExpire
Documentation/git-update-index: explain splitIndex.*
Documentation/config.txt | 28 +++++++
Documentation/git-update-index.txt | 43 +++++++++--
builtin/gc.c | 15 +---
builtin/update-index.c | 25 +++---
cache.h | 8 ++
config.c | 42 +++++++++-
read-cache.c | 142 ++++++++++++++++++++++++++++++++--
sha1_file.c | 2 +-
split-index.c | 22 ++++++
split-index.h | 2 +
t/t1700-split-index.sh | 154 +++++++++++++++++++++++++++++++++++++
11 files changed, 441 insertions(+), 42 deletions(-)
--
2.11.0.53.gb263787
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #04; Fri, 16)
From: Karthik Nayak @ 2016-12-17 14:36 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, Git List
In-Reply-To: <10d14d56-c7a0-ac31-ec57-a9ed163e1204@ramsayjones.plus.com>
On Sat, Dec 17, 2016 at 7:13 PM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
>
> On 17/12/16 08:38, Karthik Nayak wrote:
>> Hello,
>>
>>>
>>> * kn/ref-filter-branch-list (2016-12-08) 20 commits
>>> - branch: implement '--format' option
>>> - branch: use ref-filter printing APIs
>>> - branch, tag: use porcelain output
>>> - ref-filter: allow porcelain to translate messages in the output
>>> - ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
>>> - ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
>>> - ref-filter: rename the 'strip' option to 'lstrip'
>>> - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
>>> - ref-filter: introduce refname_atom_parser()
>>> - ref-filter: introduce refname_atom_parser_internal()
>>> - ref-filter: make "%(symref)" atom work with the ':short' modifier
>>> - ref-filter: add support for %(upstream:track,nobracket)
>>> - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
>>> - ref-filter: introduce format_ref_array_item()
>>> - ref-filter: move get_head_description() from branch.c
>>> - ref-filter: modify "%(objectname:short)" to take length
>>> - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
>>> - ref-filter: include reference to 'used_atom' within 'atom_value'
>>> - ref-filter: implement %(if), %(then), and %(else) atoms
>>> - for-each-ref: do not segv with %(HEAD) on an unborn branch
>>>
>>> The code to list branches in "git branch" has been consolidated
>>> with the more generic ref-filter API.
>>>
>>> What's the doneness of the topic? I recall discussing die vs empty
>>> and also saw a "squash this in when you reroll", but I lost track.
>>>
>>
>> I was waiting for more reviews, if any.
>> For now we need to come to a conclusion on the die vs empty discussion
>> (http://marc.info/?l=git&m=148112502029302&w=2) I'll start working on returning
>> empty rather than die.
>>
>> Also Jeff suggested some changes, which I've incorporated into my local branch.
>> (http://marc.info/?t=148112503600001&r=1&w=2). I'll reroll if no
>> further changes are
>> suggested soon :)
>
> Not forgetting to make 'quote_literal_for_format()' static. ;-)
>
Of Course, thanks for the reminder :)
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: test failure
From: Lars Schneider @ 2016-12-17 14:28 UTC (permalink / raw)
To: Ramsay Jones, Jeff King; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <dd8decbc-f856-4f68-6d77-7ea9d5f9d126@ramsayjones.plus.com>
> On 16 Dec 2016, at 21:32, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
> Hi Lars,
>
> For the last two days, I've noticed t0021.15 on the 'pu' branch has been failing intermittently (well it fails with: 'make test >ptest-out', but
> when run by hand, it fails only say 1-in-6, 1-in-18, etc.).
>
> [yes, it's a bit strange; this hasn't changed in a couple of weeks!]
>
> I don't have time to investigate further tonight and, since I had not
> heard anyone else complain, I thought I should let you know.
>
> See below for the output from a failing run. [Note: this is on Linux
> Mint 18, tonight's pu branch @7c7984401].
Thanks Ramsay!
I was able to reproduce the problem with this test:
test_expect_success 'ramsay-report' '
test_config_global filter.protocol.clean cat &&
git init &&
echo "*.r filter=protocol" >.gitattributes &&
echo "bla" >test.r &&
git add . &&
GIT_TRACE=1 git commit -m "test commit 2" > trace 2>&1 &&
grep "run_command" trace
'
It looks like as if Git occasionally forgets to run the clean filter.
I bisected the problem and I think the problem starts with "diff: do not
reuse worktree files that need "clean" conversion" (06dec439a3) which
definitively sounds related.
Back in June I reported that Git invokes the clean process 4 times if a
single file is added. Peff took a closer look and suggested the patch
mentioned above to remove one unnecessary invocation. I re-read his comments
and everything sounds still reasonable to me:
http://public-inbox.org/git/1469134747-26785-1-git-send-email-larsxschneider@gmail.com/#t
Does anyone have a clue what is going on?
I keep digging...
Thanks,
Lars
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #04; Fri, 16)
From: Ramsay Jones @ 2016-12-17 13:43 UTC (permalink / raw)
To: Karthik Nayak, Junio C Hamano; +Cc: Git List
In-Reply-To: <CAOLa=ZREUWqdH_2HNn_JQcf4RW9k1dAN5BtwPN2HnzuDoUdkWw@mail.gmail.com>
On 17/12/16 08:38, Karthik Nayak wrote:
> Hello,
>
>>
>> * kn/ref-filter-branch-list (2016-12-08) 20 commits
>> - branch: implement '--format' option
>> - branch: use ref-filter printing APIs
>> - branch, tag: use porcelain output
>> - ref-filter: allow porcelain to translate messages in the output
>> - ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
>> - ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
>> - ref-filter: rename the 'strip' option to 'lstrip'
>> - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
>> - ref-filter: introduce refname_atom_parser()
>> - ref-filter: introduce refname_atom_parser_internal()
>> - ref-filter: make "%(symref)" atom work with the ':short' modifier
>> - ref-filter: add support for %(upstream:track,nobracket)
>> - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
>> - ref-filter: introduce format_ref_array_item()
>> - ref-filter: move get_head_description() from branch.c
>> - ref-filter: modify "%(objectname:short)" to take length
>> - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
>> - ref-filter: include reference to 'used_atom' within 'atom_value'
>> - ref-filter: implement %(if), %(then), and %(else) atoms
>> - for-each-ref: do not segv with %(HEAD) on an unborn branch
>>
>> The code to list branches in "git branch" has been consolidated
>> with the more generic ref-filter API.
>>
>> What's the doneness of the topic? I recall discussing die vs empty
>> and also saw a "squash this in when you reroll", but I lost track.
>>
>
> I was waiting for more reviews, if any.
> For now we need to come to a conclusion on the die vs empty discussion
> (http://marc.info/?l=git&m=148112502029302&w=2) I'll start working on returning
> empty rather than die.
>
> Also Jeff suggested some changes, which I've incorporated into my local branch.
> (http://marc.info/?t=148112503600001&r=1&w=2). I'll reroll if no
> further changes are
> suggested soon :)
Not forgetting to make 'quote_literal_for_format()' static. ;-)
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH] mingw: consider that UNICODE_STRING::Length counts bytes
From: Max Kirillov @ 2016-12-17 13:20 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin, Karsten Blees; +Cc: Max Kirillov, git
UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer
itself being array of wchar_t. Because of that terminating zero is placed twice
as far. Fix it.
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx
Signed-off-by: Max Kirillov <max@max630.net>
---
Access outside of buffer was very unlikely (for that user needed to redirect
standard fd to a file with path longer than ~250 symbols), it still did not
seem to do any harm, and otherwise it did not break because only substring is
checked, but it was still incorrect.
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index 3be60ce..6b4f736 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -553,7 +553,7 @@ static void detect_msys_tty(int fd)
buffer, sizeof(buffer) - 2, &result)))
return;
name = nameinfo->Name.Buffer;
- name[nameinfo->Name.Length] = 0;
+ name[nameinfo->Name.Length / sizeof(*name)] = 0;
/* check if this could be a MSYS2 pty pipe ('msys-XXXX-ptyN-XX') */
if (!wcsstr(name, L"msys-") || !wcsstr(name, L"-pty"))
--
2.3.4.2801.g3d0809b
^ permalink raw reply related
* Re: [PATCH v2 1/5] doc: add documentation for OPT_STRING_LIST
From: Philip Oakley @ 2016-12-17 11:56 UTC (permalink / raw)
To: Jacob Keller, git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-2-jacob.e.keller@intel.com>
From: "Jacob Keller" <jacob.e.keller@intel.com>
> From: Jacob Keller <jacob.keller@gmail.com>
>
> Commit c8ba16391655 ("parse-options: add OPT_STRING_LIST helper",
> 2011-06-09) added the OPT_STRING_LIST as a way to accumulate a repeated
> list of strings. However, this was not documented in the
> api-parse-options documentation. Add documentation now so that future
> developers may learn of its existence.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> Documentation/technical/api-parse-options.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/technical/api-parse-options.txt
> b/Documentation/technical/api-parse-options.txt
> index 27bd701c0d68..92791740aa64 100644
> --- a/Documentation/technical/api-parse-options.txt
> +++ b/Documentation/technical/api-parse-options.txt
> @@ -168,6 +168,11 @@ There are some macros to easily define options:
> Introduce an option with string argument.
> The string argument is put into `str_var`.
>
> +`OPT_STRING_LIST(short long, &list, arg_str, description)`::
should there be an extra comma between 'short long' in a similar manner to
the OPT_INTEGER argument list below?
> + Introduce an option with a string argument. Repeated invocations
> + accumulate into a list of strings. Reset and clear the list with
> + `--no-option`.
> +
> `OPT_INTEGER(short, long, &int_var, description)`::
> Introduce an option with integer argument.
> The integer is put into `int_var`.
> --
> 2.11.0.rc2.152.g4d04e67
>
>
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #04; Fri, 16)
From: Karthik Nayak @ 2016-12-17 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqbmwbqw25.fsf@gitster.mtv.corp.google.com>
Hello,
>
> * kn/ref-filter-branch-list (2016-12-08) 20 commits
> - branch: implement '--format' option
> - branch: use ref-filter printing APIs
> - branch, tag: use porcelain output
> - ref-filter: allow porcelain to translate messages in the output
> - ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
> - ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
> - ref-filter: rename the 'strip' option to 'lstrip'
> - ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
> - ref-filter: introduce refname_atom_parser()
> - ref-filter: introduce refname_atom_parser_internal()
> - ref-filter: make "%(symref)" atom work with the ':short' modifier
> - ref-filter: add support for %(upstream:track,nobracket)
> - ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
> - ref-filter: introduce format_ref_array_item()
> - ref-filter: move get_head_description() from branch.c
> - ref-filter: modify "%(objectname:short)" to take length
> - ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
> - ref-filter: include reference to 'used_atom' within 'atom_value'
> - ref-filter: implement %(if), %(then), and %(else) atoms
> - for-each-ref: do not segv with %(HEAD) on an unborn branch
>
> The code to list branches in "git branch" has been consolidated
> with the more generic ref-filter API.
>
> What's the doneness of the topic? I recall discussing die vs empty
> and also saw a "squash this in when you reroll", but I lost track.
>
I was waiting for more reviews, if any.
For now we need to come to a conclusion on the die vs empty discussion
(http://marc.info/?l=git&m=148112502029302&w=2) I'll start working on returning
empty rather than die.
Also Jeff suggested some changes, which I've incorporated into my local branch.
(http://marc.info/?t=148112503600001&r=1&w=2). I'll reroll if no
further changes are
suggested soon :)
--
Regards,
Karthik Nayak
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jacob Keller @ 2016-12-17 8:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Norbert Kiesel
In-Reply-To: <xmqq7f6zqr3i.fsf@gitster.mtv.corp.google.com>
On Fri, Dec 16, 2016 at 5:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Although I do not think we should spend too much braincycles on this
> one (we should rather just removing the older one soonish), I think
> this patch is going in a wrong direction. I agree that "the last
> one wins" is a bit hard to see (until you check with "git config -l"
> perhaps) but it at least is predictable. With this patch, you need
> to KNOW that indent wins over compaction, perhaps by knowing the
> order they were developed, which demands a lot more from the users.
>
I don't think we have too many config options that interact in this
way, so I understand that "last writing of a particular configuration"
makes sense, but interactions between configs is something that would
have never occurred to me. I'll send a patch to drop the compaction
heuristic since I think we're all 100% in agreement that it is
superseded by the new configuration (as no case has been shown where
the new one is worse than compaction, and most show it to be better).
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 1/2] alternates: accept double-quoted paths
From: Duy Nguyen @ 2016-12-17 7:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, Klaus Ethgen, Git Mailing List
In-Reply-To: <xmqq1sxb66t5.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 14, 2016 at 1:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> At least attr has the same problem and is going the same direction
>> [1]. Cool. (I actually thought the patch was in and evidence that this
>> kind of backward compatibility breaking was ok, turns out the patch
>> has stayed around for years)
>>
>> [1] http://public-inbox.org/git/%3C20161110203428.30512-18-sbeller@google.com%3E/
>
> Seeing that again, I see this in the proposed log message:
>
> Full pattern must be quoted. So 'pat"t"ern attr' will give exactly
> 'pat"t"ern', not 'pattern'.
>
> I cannot tell what it is trying to say.
It's another (bad) way of saying that quoting must start at the
beginning (so the closing quote must be at the end of pattern, i.e.
the whole pattern is quoted).
> The log message may need to be cleaned up.
Yeah. Next time I'll write more or less "Look at Jeff's commit, this
is basically it" when Stephan re-rolls sb/attr
--
Duy
^ permalink raw reply
* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Duy Nguyen @ 2016-12-17 7:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Turner, Git Mailing List, Jeff King
In-Reply-To: <xmqqpokrr2cf.fsf@gitster.mtv.corp.google.com>
On Sat, Dec 17, 2016 at 4:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> David Turner <novalis@novalis.org> writes:
>
>> I'm a bit confused by the message "disabling bitmap writing, as some
>> objects are not being packed". I see it the my gc.log file on my git
>> server.
>
>> 1. Its presence in the gc.log file prevents future automatic garbage
>> collection. This seems bad. I understand the desire to avoid making
>> things worse if a past gc has run into issues. But this warning is
>> non-fatal; the only consequence is that many operations get slower. But
>> a lack of gc when there are too many packs causes that consequence too
>> (often a much worse slowdown than would be caused by the missing
>> bitmap).
>>
>> So I wonder if it would be better for auto gc to grep gc.log for fatal
>> errors (as opposed to warnings) and only skip running if any are found.
>> Alternately, we could simply put warnings into gc.log.warning and
>> reserve gc.log for fatal errors. I'm not sure which would be simpler.
>
> I am not sure if string matching is really a good idea, as I'd
> assume that these messages are eligible for i18n.
And we can't grep for fatal errors anyway. The problem that led to
329e6e8794 was this line
warning: There are too many unreachable loose objects; run 'git
prune' to remove them.
which is not fatal.
> 329e6e8794 ("gc: save log from daemonized gc --auto and print it
> next time", 2015-09-19) wanted to notice that auto-gc is not
> making progress and used the presense of error messages as a cue.
> In your case, I think the auto-gc _is_ making progress, reducing
> number of loose objects in the repository or consolidating many
> packfiles into one
Yeah the key point is making progress, and to reliably detect that we
need some way for all the commands that git-gc executes to tell it
about that, git-repack in this particular case but...
> and the message is only about the fact that
> packing is punting and not producing a bitmap as you asked, which
> is different from not making any progress. I do not think log vs
> warn is a good criteria to tell them apart, either.
>
> In any case, as the error message asks the user to do, the user
> eventually wants to correct the root cause before removing the
> gc.log; I am not sure report_last_gc_error() is the place to correct
> this in the first place.
>
>> 2. I don't understand what would cause that message. That is, what bad
>> thing am I doing that I should stop doing? I've briefly skimmed the
>> code and commit message, but the answer isn't leaping out at me.
>
> Enabling bitmap generation for incremental packing that does not
> cram everything into a single pack is triggering it, I would
> presume. Perhaps we should ignore -b option in most of the cases
> and enable it only for "repack -a -d -f" codepath? Or detect that
> we are being run from "gc --auto" and automatically disable -b?
... since we have to change down in git-repack for that, perhaps doing
this is better. We can pass --auto (or something) to repack to tell it
about this special caller, so it only prints something to stderr in
serious cases.
Or we detect cases where background gc'ing won't work well and always
do it in foreground (e.g. when bitmap generation is enabled).
> I have a feeling that an approach along that line is closer to the
> real solution than tweaking report_last_gc_error() and trying to
> deduce if we are making any progress.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCHv2] Makefile: add cppcheck target
From: Chris Packham @ 2016-12-17 7:31 UTC (permalink / raw)
To: Lars Schneider; +Cc: Jeff King, GIT, stefan.naewe, Elia Pinto
In-Reply-To: <6ABA4AA4-BD5C-4178-BB3B-91CA045EA2AD@gmail.com>
On Fri, Dec 16, 2016 at 9:28 PM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
> On 14 Dec 2016, at 12:24, Jeff King <peff@peff.net> wrote:
>
> On Wed, Dec 14, 2016 at 10:27:31PM +1300, Chris Packham wrote:
>
> Changes in v2:
>
> - only run over actual git source files.
>
> - omit any files in t/
>
>
> I actually wonder if FIND_SOURCE_FILES should be taking care of the "t/"
> thing. I think "make tags" finds tags in t4051/appended1.c, which is
> just silly.
>
> - introduce CPPCHECK_FLAGS which can be overridden in the make command
>
> line. This also uses a GNU make-ism to allow CPPCHECK_ADD to specify
>
> additional checks to be enabled.
>
>
> The GNU-ism is fine; we already require GNU make to build.
>
> The patch itself is OK to me, I guess. The interesting part will be
> whether people start actually _using_ cppcheck and squelching the false
> positives.
>
>
> @Chris: If this gets in then it would be great to run it as part of the
> Travis-CI build: https://travis-ci.org/git/git/branches
>
Yeah I was thinking about this.
Since as always with a new tool there are some doubts over it's
usefulness. I could easily hook it up to a branch in my own fork of
git and keep that branch rebased on top of pu. I'd need to keep an eye
on it myself and report errors on the list.
If that goes well, at some point someone will ask how I'm detecting
these errors. Then I can point them at this patchset and if enough
people want easy access to it then that may provide an incentive for
this to be merged into git.git.
^ permalink raw reply
* Re: [PATCH] pack-objects: don't warn about bitmaps on incremental pack
From: Jeff King @ 2016-12-17 4:04 UTC (permalink / raw)
To: David Turner; +Cc: git
In-Reply-To: <1481932775-12952-1-git-send-email-dturner@twosigma.com>
On Fri, Dec 16, 2016 at 06:59:35PM -0500, David Turner wrote:
> When running git pack-objects --incremental, we do not expect to be
> able to write a bitmap; it is very likely that objects in the new pack
> will have references to objects outside of the pack. So we don't need
> to warn the user about it.
> [...]
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 0fd52bd..96de213 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1083,7 +1083,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
> if (!want_object_in_pack(sha1, exclude, &found_pack, &found_offset)) {
> /* The pack is missing an object, so it will not have closure */
> if (write_bitmap_index) {
> - warning(_(no_closure_warning));
> + if (!incremental)
> + warning(_(no_closure_warning));
> write_bitmap_index = 0;
> }
> return 0;
I agree that the user doesn't need to be warned about it when running
"gc --auto", but I wonder if somebody invoking "pack-objects
--incremental --write-bitmap-index" ought to be.
In other words, your patch is detecting at a low level that we've been
given a nonsense combination of options, but should we perhaps stop
passing nonsense in the first place?
Either at the repack level, with something like:
diff --git a/builtin/repack.c b/builtin/repack.c
index 80dd06b4a2..6608a902b1 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -231,8 +231,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
argv_array_pushf(&cmd.args, "--no-reuse-delta");
if (no_reuse_object)
argv_array_pushf(&cmd.args, "--no-reuse-object");
- if (write_bitmaps)
- argv_array_push(&cmd.args, "--write-bitmap-index");
if (pack_everything & ALL_INTO_ONE) {
get_non_kept_pack_filenames(&existing_packs);
@@ -256,8 +254,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
} else {
argv_array_push(&cmd.args, "--unpacked");
argv_array_push(&cmd.args, "--incremental");
+ write_bitmap_index = 0;
}
+ if (write_bitmaps)
+ argv_array_push(&cmd.args, "--write-bitmap-index");
if (local)
argv_array_push(&cmd.args, "--local");
if (quiet)
Though that still means we do not warn on:
git repack --write-bitmap-index
which is nonsense (it is asking for an incremental repack with bitmaps).
So maybe do it at the gc level, like:
diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d0b4..d3c978c765 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -191,6 +191,11 @@ static void add_repack_all_option(void)
}
}
+static void add_repack_incremental_option(void)
+{
+ argv_array_push(&repack, "--no-write-bitmap-index");
+}
+
static int need_to_gc(void)
{
/*
@@ -208,7 +213,9 @@ static int need_to_gc(void)
*/
if (too_many_packs())
add_repack_all_option();
- else if (!too_many_loose_objects())
+ else if (too_many_loose_objects())
+ add_repack_incremental_option();
+ else
return 0;
if (run_hook_le(NULL, "pre-auto-gc", NULL))
-Peff
^ permalink raw reply related
* Re: [PATCH] git-p4: Fix multi-path changelist empty commits
From: George Vanburgh @ 2016-12-17 2:25 UTC (permalink / raw)
To: Luke Diamand; +Cc: Git Users
In-Reply-To: <CAE5ih7-+tahL4=OrW6F6UPKKRg1KFkw32e=pnTx6j2WTZ-BhOw@mail.gmail.com>
On 17 December 2016 01:47:55 CET, Luke Diamand <luke@diamand.org> wrote:
>On 15 December 2016 at 17:14, George Vanburgh <george@vanburgh.me>
>wrote:
>> From: George Vanburgh <gvanburgh@bloomberg.net>
>>
>> When importing from multiple perforce paths - we may attempt to
>import a changelist that contains files from two (or more) of these
>depot paths. Currently, this results in multiple git commits - one
>containing the changes, and the other(s) as empty commits. This
>behavior was introduced in commit 1f90a64 ("git-p4: reduce number of
>server queries for fetches", 2015-12-19).
>
>That's definitely a bug, thanks for spotting that! Even more so for
>adding a test case.
Not a problem - thanks to you guys for maintaining such an awesome tool!
>
>>
>> Reproduction Steps:
>>
>> 1. Have a git repo cloned from a perforce repo using multiple depot
>paths (e.g. //depot/foo and //depot/bar).
>> 2. Submit a single change to the perforce repo that makes changes in
>both //depot/foo and //depot/bar.
>> 3. Run "git p4 sync" to sync the change from #2.
>>
>> Change is synced as multiple commits, one for each depot path that
>was affected.
>>
>> Using a set, instead of a list inside p4ChangesForPaths() ensures
>that each changelist is unique to the returned list, and therefore only
>a single commit is generated for each changelist.
>
>The change looks good to me apart from one missing "&&" in the test
>case (see below).
Oops - I'll correct that and resubmit :)
>Possibly need to rewrap the comment line (I think there's a 72
>character limit) ?
Sure - I'll fix that in the resubmission
>
>Luke
>
>
>>
>> Reported-by: James Farwell <jfarwell@vmware.com>
>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
>> ---
>> git-p4.py | 4 ++--
>> t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
>> 2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index fd5ca52..6307bc8 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange,
>requestedBlockSize):
>> die("cannot use --changes-block-size with
>non-numeric revisions")
>> block_size = None
>>
>> - changes = []
>> + changes = set()
>>
>> # Retrieve changes a block at a time, to prevent running
>> # into a MaxResults/MaxScanRows error from the server.
>> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange,
>requestedBlockSize):
>>
>> # Insert changes in chronological order
>> for line in reversed(p4_read_pipe_lines(cmd)):
>> - changes.append(int(line.split(" ")[1]))
>> + changes.add(int(line.split(" ")[1]))
>>
>> if not block_size:
>> break
>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>> index 0730f18..4d72e0b 100755
>> --- a/t/t9800-git-p4-basic.sh
>> +++ b/t/t9800-git-p4-basic.sh
>> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all,
>conflicting files' '
>> )
>> '
>>
>> +test_expect_success 'clone two dirs, each edited by submit, single
>git commit' '
>> + (
>> + cd "$cli" &&
>> + echo sub1/f4 >sub1/f4 &&
>> + p4 add sub1/f4 &&
>> + echo sub2/f4 >sub2/f4 &&
>> + p4 add sub2/f4 &&
>> + p4 submit -d "sub1/f4 and sub2/f4"
>> + ) &&
>> + git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all
>&&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git"
>
>Missing &&
>
>> + git ls-files >lines &&
>> + test_line_count = 4 lines &&
>> + git log --oneline p4/master >lines &&
>> + test_line_count = 5 lines
>> + )
>> +'
>> +
>> revision_ranges="2000/01/01,#head \
>> 1,2080/01/01 \
>> 2000/01/01,2080/01/01 \
>> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric
>revision ranges' '
>> (
>> cd "$git" &&
>> git ls-files >lines &&
>> - test_line_count = 6 lines
>> + test_line_count = 8 lines
>> )
>> done
>> '
>>
>> --
>> https://github.com/git/git/pull/311
^ permalink raw reply
* Re: [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Junio C Hamano @ 2016-12-17 1:30 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Norbert Kiesel, Jacob Keller
In-Reply-To: <20161217005442.5866-1-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> The current configuration code for enabling experimental heuristics
> prefers the last-set heuristic in the configuration. However, it is not
> necessarily easy to see what order the configuration will be read. This
> means that it is possible for a user to have accidentally enabled both
> heuristics, and end up only enabling the older compaction heuristic.
>
> Modify the code so that we do not clear the other heuristic when we set
> each heuristic enabled. Then, during diff_setup() when we check the
> configuration, we will first check the newer indent heuristic. This
> ensures that we only enable the newer heuristic if both have been
> enabled.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> diff.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
Although I do not think we should spend too much braincycles on this
one (we should rather just removing the older one soonish), I think
this patch is going in a wrong direction. I agree that "the last
one wins" is a bit hard to see (until you check with "git config -l"
perhaps) but it at least is predictable. With this patch, you need
to KNOW that indent wins over compaction, perhaps by knowing the
order they were developed, which demands a lot more from the users.
We probably should just keep one and remove the other.
> diff --git a/diff.c b/diff.c
> index ec8728362dae..48a5b2797e3d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -223,16 +223,10 @@ void init_diff_ui_defaults(void)
>
> int git_diff_heuristic_config(const char *var, const char *value, void *cb)
> {
> + if (!strcmp(var, "diff.indentheuristic"))
> diff_indent_heuristic = git_config_bool(var, value);
> + if (!strcmp(var, "diff.compactionheuristic"))
> diff_compaction_heuristic = git_config_bool(var, value);
> return 0;
> }
^ permalink raw reply
* [PATCH v2 5/5] describe: teach describe negative pattern matches
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Teach git-describe the `--discard` option which will allow specifying
a glob pattern of tags to ignore. This can be combined with the
`--match` patterns to enable more flexibility in determining which tags
to consider.
For example, suppose you wish to find the first official release tag
that contains a certain commit. If we assume that official release tags
are of the form "v*" and pre-release candidates include "*rc*" in their
name, we can now find the first tag that introduces commit abcdef via:
git describe --contains --match="v*" --discard="*rc*"
Add documentation and tests for this change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-describe.txt | 8 ++++++++
builtin/describe.c | 21 +++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index 7ad41e2f6ade..a89bbde207b2 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -88,6 +88,14 @@ OPTIONS
patterns will be considered. Use `--no-match` to clear and reset the
list of patterns.
+--discard <pattern>::
+ Do not consider tags matching the given `glob(7)` pattern, excluding
+ the "refs/tags/" prefix. This can be used to narrow the tag space and
+ find only tags matching some meaningful criteria. If given multiple
+ times, a list of patterns will be accumulated and tags matching any
+ of the patterns will be discarded. Use `--no-discard` to clear and
+ reset the list of patterns.
+
--always::
Show uniquely abbreviated commit object as fallback.
diff --git a/builtin/describe.c b/builtin/describe.c
index 5cc9e9abe798..c09288ee6321 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -29,6 +29,7 @@ static int max_candidates = 10;
static struct hashmap names;
static int have_util;
static struct string_list patterns = STRING_LIST_INIT_NODUP;
+static struct string_list discard_patterns = STRING_LIST_INIT_NODUP;
static int always;
static const char *dirty;
@@ -130,6 +131,22 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
return 0;
/*
+ * If we're given discard patterns, first discard any tag which match
+ * any of the discard pattern.
+ */
+ if (discard_patterns.nr) {
+ struct string_list_item *item;
+
+ if (!is_tag)
+ return 0;
+
+ for_each_string_list_item(item, &discard_patterns) {
+ if (!wildmatch(item->string, path + 10, 0, NULL))
+ return 0;
+ }
+ }
+
+ /*
* If we're given patterns, accept only tags which match at least one
* pattern.
*/
@@ -421,6 +438,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
N_("consider <n> most recent tags (default: 10)")),
OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
N_("only consider tags matching <pattern>")),
+ OPT_STRING_LIST(0, "discard", &discard_patterns, N_("pattern"),
+ N_("do not consider tags matching <pattern>")),
OPT_BOOL(0, "always", &always,
N_("show abbreviated commit object as fallback")),
{OPTION_STRING, 0, "dirty", &dirty, N_("mark"),
@@ -458,6 +477,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
argv_array_push(&args, "--tags");
for_each_string_list_item(item, &patterns)
argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
+ for_each_string_list_item(item, &discard_patterns)
+ argv_array_pushf(&args, "--discard=refs/tags/%s", item->string);
}
if (argc)
argv_array_pushv(&args, argv);
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 4/5] describe: teach --match to accept multiple patterns
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Teach `--match` to be accepted multiple times, accumulating a list of
patterns to match into a string list. Each pattern is inclusive, such
that a tag need only match one of the provided patterns to be
considered for matching.
This extension is useful as it enables more flexibility in what tags
match, and may avoid the need to run the describe command multiple
times to get the same result.
Add tests and update the documentation for this change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-describe.txt | 5 ++++-
builtin/describe.c | 30 +++++++++++++++++++++++-------
t/t6120-describe.sh | 19 +++++++++++++++++++
3 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e4ac448ff565..7ad41e2f6ade 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -83,7 +83,10 @@ OPTIONS
--match <pattern>::
Only consider tags matching the given `glob(7)` pattern,
excluding the "refs/tags/" prefix. This can be used to avoid
- leaking private tags from the repository.
+ leaking private tags from the repository. If given multiple times, a
+ list of patterns will be accumulated, and tags matching any of the
+ patterns will be considered. Use `--no-match` to clear and reset the
+ list of patterns.
--always::
Show uniquely abbreviated commit object as fallback.
diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157efc..5cc9e9abe798 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -28,7 +28,7 @@ static int abbrev = -1; /* unspecified */
static int max_candidates = 10;
static struct hashmap names;
static int have_util;
-static const char *pattern;
+static struct string_list patterns = STRING_LIST_INIT_NODUP;
static int always;
static const char *dirty;
@@ -129,9 +129,24 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
if (!all && !is_tag)
return 0;
- /* Accept only tags that match the pattern, if given */
- if (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))
- return 0;
+ /*
+ * If we're given patterns, accept only tags which match at least one
+ * pattern.
+ */
+ if (patterns.nr) {
+ struct string_list_item *item;
+
+ if (!is_tag)
+ return 0;
+
+ for_each_string_list_item(item, &patterns) {
+ if (!wildmatch(item->string, path + 10, 0, NULL))
+ break;
+
+ /* If we get here, no pattern matched. */
+ return 0;
+ }
+ }
/* Is it annotated? */
if (!peel_ref(path, peeled.hash)) {
@@ -404,7 +419,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
N_("only output exact matches"), 0),
OPT_INTEGER(0, "candidates", &max_candidates,
N_("consider <n> most recent tags (default: 10)")),
- OPT_STRING(0, "match", &pattern, N_("pattern"),
+ OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
N_("only consider tags matching <pattern>")),
OPT_BOOL(0, "always", &always,
N_("show abbreviated commit object as fallback")),
@@ -430,6 +445,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
die(_("--long is incompatible with --abbrev=0"));
if (contains) {
+ struct string_list_item *item;
struct argv_array args;
argv_array_init(&args);
@@ -440,8 +456,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
argv_array_push(&args, "--always");
if (!all) {
argv_array_push(&args, "--tags");
- if (pattern)
- argv_array_pushf(&args, "--refs=refs/tags/%s", pattern);
+ for_each_string_list_item(item, &patterns)
+ argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
}
if (argc)
argv_array_pushv(&args, argv);
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 85f269411cb3..9e5db9b87a1f 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -182,6 +182,10 @@ check_describe "test2-lightweight-*" --tags --match="test2-*"
check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
+check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
+
+check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
+
test_expect_success 'name-rev with exact tags' '
echo A >expect &&
tag_object=$(git rev-parse refs/tags/A) &&
@@ -206,4 +210,19 @@ test_expect_success 'describe --contains with the exact tags' '
test_cmp expect actual
'
+test_expect_success 'describe --contains and --match' '
+ echo "A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ test_must_fail git describe --contains --match="B" $tagged_commit &&
+ git describe --contains --match="B" --match="A" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'describe --contains and --no-match' '
+ echo "A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ git describe --contains --match="B" --no-match $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 3/5] name-rev: add support to discard refs by pattern match
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Extend name-rev further to support matching refs by adding `--discard`
patterns. These patterns will limit the scope of refs by discarding any
ref that matches at least one discard pattern. Checking the discard refs
shall happen first, before checking the include --refs patterns. This
will allow more flexibility to matching certain kinds of references.
Add tests and update Documentation for this change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-name-rev.txt | 7 +++++++
builtin/name-rev.c | 14 +++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index 7433627db12d..9b46e5ea9aae 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -30,6 +30,13 @@ OPTIONS
given multiple times, use refs whose names match any of the given shell
patterns. Use `--no-refs` to clear any previous ref patterns given.
+--discard=<pattern>::
+ Do not use any ref whose name matches a given shell pattern. The
+ pattern can be one of branch name, tag name or fully qualified ref
+ name. If given multiple times, discard refs that match any of the given
+ shell patterns. Use `--no-discards` to clear the list of discard
+ patterns.
+
--all::
List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 000a2a700ed3..86479c17a7c9 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -109,6 +109,7 @@ struct name_ref_data {
int tags_only;
int name_only;
struct string_list ref_filters;
+ struct string_list discard_filters;
};
static struct tip_table {
@@ -150,6 +151,15 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
+ if (data->discard_filters.nr) {
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, &data->discard_filters) {
+ if (subpath_matches(path, item->string) >= 0)
+ return 0;
+ }
+ }
+
if (data->ref_filters.nr) {
struct string_list_item *item;
int matched = 0;
@@ -323,12 +333,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
{
struct object_array revs = OBJECT_ARRAY_INIT;
int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
- struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
+ struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
N_("only use refs matching <pattern>")),
+ OPT_STRING_LIST(0, "discard", &data.discard_filters, N_("pattern"),
+ N_("ignore refs matching <pattern>")),
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 1/5] doc: add documentation for OPT_STRING_LIST
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Commit c8ba16391655 ("parse-options: add OPT_STRING_LIST helper",
2011-06-09) added the OPT_STRING_LIST as a way to accumulate a repeated
list of strings. However, this was not documented in the
api-parse-options documentation. Add documentation now so that future
developers may learn of its existence.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/technical/api-parse-options.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 27bd701c0d68..92791740aa64 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -168,6 +168,11 @@ There are some macros to easily define options:
Introduce an option with string argument.
The string argument is put into `str_var`.
+`OPT_STRING_LIST(short long, &list, arg_str, description)`::
+ Introduce an option with a string argument. Repeated invocations
+ accumulate into a list of strings. Reset and clear the list with
+ `--no-option`.
+
`OPT_INTEGER(short, long, &int_var, description)`::
Introduce an option with integer argument.
The integer is put into `int_var`.
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 2/5] name-rev: extend --refs to accept multiple patterns
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161217012431.29548-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Teach git name-rev to take a string list of patterns from --refs instead
of only a single pattern. The list of patterns will be matched
inclusively, such that a ref only needs to match one pattern to be
included. If a ref will only be excluded if it does not match any of the
patterns.
Add tests and documentation for this change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-name-rev.txt | 4 +++-
builtin/name-rev.c | 41 +++++++++++++++++++++++++-----------
t/t6007-rev-list-cherry-pick-file.sh | 30 ++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ca28fb8e2a07..7433627db12d 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -26,7 +26,9 @@ OPTIONS
--refs=<pattern>::
Only use refs whose names match a given shell pattern. The pattern
- can be one of branch name, tag name or fully qualified ref name.
+ can be one of branch name, tag name or fully qualified ref name. If
+ given multiple times, use refs whose names match any of the given shell
+ patterns. Use `--no-refs` to clear any previous ref patterns given.
--all::
List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65e8..000a2a700ed3 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)
struct name_ref_data {
int tags_only;
int name_only;
- const char *ref_filter;
+ struct string_list ref_filters;
};
static struct tip_table {
@@ -150,16 +150,33 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
- if (data->ref_filter) {
- switch (subpath_matches(path, data->ref_filter)) {
- case -1: /* did not match */
- return 0;
- case 0: /* matched fully */
- break;
- default: /* matched subpath */
- can_abbreviate_output = 1;
- break;
+ if (data->ref_filters.nr) {
+ struct string_list_item *item;
+ int matched = 0;
+
+ /* See if any of the patterns match. */
+ for_each_string_list_item(item, &data->ref_filters) {
+ /*
+ * We want to check every pattern even if we already
+ * found a match, just in case one of the later
+ * patterns could abbreviate the output.
+ */
+ switch (subpath_matches(path, item->string)) {
+ case -1: /* did not match */
+ break;
+ case 0: /* matched fully */
+ matched = 1;
+ break;
+ default: /* matched subpath */
+ matched = 1;
+ can_abbreviate_output = 1;
+ break;
+ }
}
+
+ /* If none of the patterns matched, stop now */
+ if (!matched)
+ return 0;
}
add_to_tip_table(oid->hash, path, can_abbreviate_output);
@@ -306,11 +323,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
{
struct object_array revs = OBJECT_ARRAY_INIT;
int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
- struct name_ref_data data = { 0, 0, NULL };
+ struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
- OPT_STRING(0, "refs", &data.ref_filter, N_("pattern"),
+ OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
N_("only use refs matching <pattern>")),
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index 1408b608eb03..d072ec43b016 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
test_cmp actual.named expect
'
+test_expect_success 'name-rev multiple --refs combine inclusive' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+<tags/F
+$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)
+EOF
+
+test_expect_success 'name-rev --refs excludes non-matched patterns' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+$(git rev-list --left-right --cherry-pick F...E -- bar)
+EOF
+
+test_expect_success 'name-rev --no-refs clears the refs list' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
cat >expect <<EOF
+tags/F
=tags/D
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 0/5] extend describe and name-rev pattern matching
From: Jacob Keller @ 2016-12-17 1:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
This series adds support for extending describe and name-rev pattern
matching to allow (a) taking multiple patterns and (b) negative patterns
which discard instead of keep. These changes increase the flexibility of
the describe mechanism so that searching for specific tag formats can be
done more easily.
Changes since v1
* add new patches for negative pattern matching
* tweak the documentation slightly
-- interdiff since v1 --
diff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt
index c85f2811ce28..a89bbde207b2 100644
--- c/Documentation/git-describe.txt
+++ w/Documentation/git-describe.txt
@@ -83,11 +83,18 @@ OPTIONS
--match <pattern>::
Only consider tags matching the given `glob(7)` pattern,
excluding the "refs/tags/" prefix. This can be used to avoid
- leaking private tags from the repository, or to shrink the scope of
- searched tags to avoid -rc tags or similar. If given multiple a list
- of patterns will be accumulated, and tags matching any of the patterns
- will be considered. Use `--no-match` to clear and reset the list of
- patterns.
+ leaking private tags from the repository. If given multiple times, a
+ list of patterns will be accumulated, and tags matching any of the
+ patterns will be considered. Use `--no-match` to clear and reset the
+ list of patterns.
+
+--discard <pattern>::
+ Do not consider tags matching the given `glob(7)` pattern, excluding
+ the "refs/tags/" prefix. This can be used to narrow the tag space and
+ find only tags matching some meaningful criteria. If given multiple
+ times, a list of patterns will be accumulated and tags matching any
+ of the patterns will be discarded. Use `--no-discard` to clear and
+ reset the list of patterns.
--always::
Show uniquely abbreviated commit object as fallback.
diff --git c/Documentation/git-name-rev.txt w/Documentation/git-name-rev.txt
index 7433627db12d..9b46e5ea9aae 100644
--- c/Documentation/git-name-rev.txt
+++ w/Documentation/git-name-rev.txt
@@ -30,6 +30,13 @@ OPTIONS
given multiple times, use refs whose names match any of the given shell
patterns. Use `--no-refs` to clear any previous ref patterns given.
+--discard=<pattern>::
+ Do not use any ref whose name matches a given shell pattern. The
+ pattern can be one of branch name, tag name or fully qualified ref
+ name. If given multiple times, discard refs that match any of the given
+ shell patterns. Use `--no-discards` to clear the list of discard
+ patterns.
+
--all::
List all commits reachable from all refs
diff --git c/builtin/describe.c w/builtin/describe.c
index e3ceab65e273..c09288ee6321 100644
--- c/builtin/describe.c
+++ w/builtin/describe.c
@@ -29,6 +29,7 @@ static int max_candidates = 10;
static struct hashmap names;
static int have_util;
static struct string_list patterns = STRING_LIST_INIT_NODUP;
+static struct string_list discard_patterns = STRING_LIST_INIT_NODUP;
static int always;
static const char *dirty;
@@ -130,6 +131,22 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
return 0;
/*
+ * If we're given discard patterns, first discard any tag which match
+ * any of the discard pattern.
+ */
+ if (discard_patterns.nr) {
+ struct string_list_item *item;
+
+ if (!is_tag)
+ return 0;
+
+ for_each_string_list_item(item, &discard_patterns) {
+ if (!wildmatch(item->string, path + 10, 0, NULL))
+ return 0;
+ }
+ }
+
+ /*
* If we're given patterns, accept only tags which match at least one
* pattern.
*/
@@ -140,9 +157,8 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
return 0;
for_each_string_list_item(item, &patterns) {
- if (!wildmatch(item->string, path + 10, 0, NULL)) {
+ if (!wildmatch(item->string, path + 10, 0, NULL))
break;
- }
/* If we get here, no pattern matched. */
return 0;
@@ -422,6 +438,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
N_("consider <n> most recent tags (default: 10)")),
OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
N_("only consider tags matching <pattern>")),
+ OPT_STRING_LIST(0, "discard", &discard_patterns, N_("pattern"),
+ N_("do not consider tags matching <pattern>")),
OPT_BOOL(0, "always", &always,
N_("show abbreviated commit object as fallback")),
{OPTION_STRING, 0, "dirty", &dirty, N_("mark"),
@@ -459,6 +477,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
argv_array_push(&args, "--tags");
for_each_string_list_item(item, &patterns)
argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
+ for_each_string_list_item(item, &discard_patterns)
+ argv_array_pushf(&args, "--discard=refs/tags/%s", item->string);
}
if (argc)
argv_array_pushv(&args, argv);
diff --git c/builtin/name-rev.c w/builtin/name-rev.c
index 000a2a700ed3..86479c17a7c9 100644
--- c/builtin/name-rev.c
+++ w/builtin/name-rev.c
@@ -109,6 +109,7 @@ struct name_ref_data {
int tags_only;
int name_only;
struct string_list ref_filters;
+ struct string_list discard_filters;
};
static struct tip_table {
@@ -150,6 +151,15 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
+ if (data->discard_filters.nr) {
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, &data->discard_filters) {
+ if (subpath_matches(path, item->string) >= 0)
+ return 0;
+ }
+ }
+
if (data->ref_filters.nr) {
struct string_list_item *item;
int matched = 0;
@@ -323,12 +333,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
{
struct object_array revs = OBJECT_ARRAY_INIT;
int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
- struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
+ struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
N_("only use refs matching <pattern>")),
+ OPT_STRING_LIST(0, "discard", &data.discard_filters, N_("pattern"),
+ N_("ignore refs matching <pattern>")),
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
diff --git c/t/t6007-rev-list-cherry-pick-file.sh w/t/t6007-rev-list-cherry-pick-file.sh
index d072ec43b016..8a4c35f6ffee 100755
--- c/t/t6007-rev-list-cherry-pick-file.sh
+++ w/t/t6007-rev-list-cherry-pick-file.sh
@@ -118,6 +118,13 @@ test_expect_success 'name-rev --refs excludes non-matched patterns' '
test_cmp actual.named expect
'
+test_expect_success 'name-rev --discard excludes matched patterns' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/*" --discard="*E" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
cat >expect <<EOF
$(git rev-list --left-right --cherry-pick F...E -- bar)
EOF
diff --git c/t/t6120-describe.sh w/t/t6120-describe.sh
index 9e5db9b87a1f..4e4a9f2e5305 100755
--- c/t/t6120-describe.sh
+++ w/t/t6120-describe.sh
@@ -218,6 +218,14 @@ test_expect_success 'describe --contains and --match' '
test_cmp expect actual
'
+test_expect_success 'describe --discard' '
+ echo "c~1" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ test_must_fail git describe --contains --match="B" $tagged_commit &&
+ git describe --contains --match="?" --discard="A" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'describe --contains and --no-match' '
echo "A^0" >expect &&
tagged_commit=$(git rev-parse "refs/tags/A^0") &&
Jacob Keller (5):
doc: add documentation for OPT_STRING_LIST
name-rev: extend --refs to accept multiple patterns
name-rev: add support to discard refs by pattern match
describe: teach --match to accept multiple patterns
describe: teach describe negative pattern matches
Documentation/git-describe.txt | 13 ++++++-
Documentation/git-name-rev.txt | 11 +++++-
Documentation/technical/api-parse-options.txt | 5 +++
builtin/describe.c | 51 ++++++++++++++++++++++----
builtin/name-rev.c | 53 +++++++++++++++++++++------
t/t6007-rev-list-cherry-pick-file.sh | 30 +++++++++++++++
t/t6120-describe.sh | 19 ++++++++++
7 files changed, 161 insertions(+), 21 deletions(-)
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCHv2] git-p4: avoid crash adding symlinked directory
From: Luke Diamand @ 2016-12-17 1:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Duy Nguyen, viniciusalexandre, aoakley,
Lars Schneider, Luke Diamand
In-Reply-To: <20161217010040.1399-1-luke@diamand.org>
When submitting to P4, if git-p4 came across a symlinked
directory, then during the generation of the submit diff, it would
try to open it as a normal file and fail.
Spot symlinks (of any type) and output a description of the symlink
instead.
Add a test case.
Signed-off-by: Luke Diamand <luke@diamand.org>
---
git-p4.py | 26 ++++++++++++++++++++------
t/t9830-git-p4-symlink-dir.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 6 deletions(-)
create mode 100755 t/t9830-git-p4-symlink-dir.sh
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..16d0b8a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -25,6 +25,7 @@ import stat
import zipfile
import zlib
import ctypes
+import errno
try:
from subprocess import CalledProcessError
@@ -1538,7 +1539,7 @@ class P4Submit(Command, P4UserMap):
if response == 'n':
return False
- def get_diff_description(self, editedFiles, filesToAdd):
+ def get_diff_description(self, editedFiles, filesToAdd, symlinks):
# diff
if os.environ.has_key("P4DIFF"):
del(os.environ["P4DIFF"])
@@ -1553,10 +1554,17 @@ class P4Submit(Command, P4UserMap):
newdiff += "==== new file ====\n"
newdiff += "--- /dev/null\n"
newdiff += "+++ %s\n" % newFile
- f = open(newFile, "r")
- for line in f.readlines():
- newdiff += "+" + line
- f.close()
+
+ is_link = os.path.islink(newFile)
+ expect_link = newFile in symlinks
+
+ if is_link and expect_link:
+ newdiff += "+%s\n" % os.readlink(newFile)
+ else:
+ f = open(newFile, "r")
+ for line in f.readlines():
+ newdiff += "+" + line
+ f.close()
return (diff + newdiff).replace('\r\n', '\n')
@@ -1574,6 +1582,7 @@ class P4Submit(Command, P4UserMap):
filesToDelete = set()
editedFiles = set()
pureRenameCopy = set()
+ symlinks = set()
filesToChangeExecBit = {}
for line in diff:
@@ -1590,6 +1599,11 @@ class P4Submit(Command, P4UserMap):
filesToChangeExecBit[path] = diff['dst_mode']
if path in filesToDelete:
filesToDelete.remove(path)
+
+ dst_mode = int(diff['dst_mode'], 8)
+ if dst_mode == 0120000:
+ symlinks.add(path)
+
elif modifier == "D":
filesToDelete.add(path)
if path in filesToAdd:
@@ -1727,7 +1741,7 @@ class P4Submit(Command, P4UserMap):
separatorLine = "######## everything below this line is just the diff #######\n"
if not self.prepare_p4_only:
submitTemplate += separatorLine
- submitTemplate += self.get_diff_description(editedFiles, filesToAdd)
+ submitTemplate += self.get_diff_description(editedFiles, filesToAdd, symlinks)
(handle, fileName) = tempfile.mkstemp()
tmpFile = os.fdopen(handle, "w+b")
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
new file mode 100755
index 0000000..3dc528b
--- /dev/null
+++ b/t/t9830-git-p4-symlink-dir.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git p4 symlinked directories'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'symlinked directory' '
+ (
+ cd "$cli" &&
+ : >first_file.t &&
+ p4 add first_file.t &&
+ p4 submit -d "first change"
+ ) &&
+ git p4 clone --dest "$git" //depot &&
+ (
+ cd "$git" &&
+ mkdir -p some/sub/directory &&
+ mkdir -p other/subdir2 &&
+ : > other/subdir2/file.t &&
+ (cd some/sub/directory && ln -s ../../../other/subdir2 .) &&
+ git add some other &&
+ git commit -m "symlinks" &&
+ git config git-p4.skipSubmitEdit true &&
+ git p4 submit -v
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ test -L some/sub/directory/subdir2
+ test_path_is_file some/sub/directory/subdir2/file.t
+ )
+
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
2.8.2.703.g78b384c.dirty
^ permalink raw reply related
* [PATCHv2] git-p4: handle symlinked directories
From: Luke Diamand @ 2016-12-17 1:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Duy Nguyen, viniciusalexandre, aoakley,
Lars Schneider, Luke Diamand
This is an updated version of my earlier git-p4 symlink fix.
This one now treats addition of all symlinks in the same way,
rather than displaying the target file if linking to a file,
or just the link target name if it's a directory.
That makes the git-p4 summary behave more like normal git
commands (which also treat symlinks uniformaly).
This is a very slight change in behaviour, but I don't think
it can break anything since it is only when generating
the summary that goes after the P4 change template.
Luke Diamand (1):
git-p4: avoid crash adding symlinked directory
git-p4.py | 26 ++++++++++++++++++++------
t/t9830-git-p4-symlink-dir.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 6 deletions(-)
create mode 100755 t/t9830-git-p4-symlink-dir.sh
--
2.8.2.703.g78b384c.dirty
^ permalink raw reply
* [PATCH] diff: prefer indent heuristic over compaction heuristic
From: Jacob Keller @ 2016-12-17 0:54 UTC (permalink / raw)
To: git; +Cc: Norbert Kiesel, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
The current configuration code for enabling experimental heuristics
prefers the last-set heuristic in the configuration. However, it is not
necessarily easy to see what order the configuration will be read. This
means that it is possible for a user to have accidentally enabled both
heuristics, and end up only enabling the older compaction heuristic.
Modify the code so that we do not clear the other heuristic when we set
each heuristic enabled. Then, during diff_setup() when we check the
configuration, we will first check the newer indent heuristic. This
ensures that we only enable the newer heuristic if both have been
enabled.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
diff.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/diff.c b/diff.c
index ec8728362dae..48a5b2797e3d 100644
--- a/diff.c
+++ b/diff.c
@@ -223,16 +223,10 @@ void init_diff_ui_defaults(void)
int git_diff_heuristic_config(const char *var, const char *value, void *cb)
{
- if (!strcmp(var, "diff.indentheuristic")) {
+ if (!strcmp(var, "diff.indentheuristic"))
diff_indent_heuristic = git_config_bool(var, value);
- if (diff_indent_heuristic)
- diff_compaction_heuristic = 0;
- }
- if (!strcmp(var, "diff.compactionheuristic")) {
+ if (!strcmp(var, "diff.compactionheuristic"))
diff_compaction_heuristic = git_config_bool(var, value);
- if (diff_compaction_heuristic)
- diff_indent_heuristic = 0;
- }
return 0;
}
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox