* Re: [PATCH 1/5] doc: add documentation for OPT_STRING_LIST
From: Jacob Keller @ 2017-01-13 0:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <alpine.DEB.2.20.1701121044120.3469@virtualbox>
On Thu, Jan 12, 2017 at 1:47 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Jake,
>
> On Wed, 11 Jan 2017, Jacob Keller wrote:
>
>> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
>> index 27bd701c0d68..15e876e4c804 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`.
>
> One suggestions: as the list parameter is not type-safe (apart from
> checking that it can be cast to a `void *`), it would be good to mention
> in the documentation that `list` must be of type `struct string_list`.
>
Ok.
> I was about to suggest that `--no-option` may be misleading, as the
> command-line option is not really called `--option` in almost all cases,
> but I see that the rest of that document uses that convention to refer to
> the negated option already...
Also, I am merely documenting what already existed, since the original
commit failed to add documentation. I don't know if we could make a
better implementation, but it certainly would be a behavior change for
the current users.
Thanks,
Jake
>
> Ciao,
> Dscho
^ permalink raw reply
* Re: [PATCH 2/5] name-rev: extend --refs to accept multiple patterns
From: Jacob Keller @ 2017-01-13 0:56 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <alpine.DEB.2.20.1701121049530.3469@virtualbox>
On Thu, Jan 12, 2017 at 1:56 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Jake,
>
> On Wed, 11 Jan 2017, Jacob Keller wrote:
>
>> 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 &&
>
> Our current coding style seems to skip the space between `>` and `actual`
> (this applies to all redirections added in this patch).
>
Right that's easy to fix.
>> + 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
>
> In the current revision of t6007, we seem to list the expected output
> explicitly, i.e. *not* generating it dynamically.
>
> If you *do* insist to generate the `expect` file dynamically, a better way
> would be to include that generation in the `test_expect_success` code so
> that errors in the call can be caught, too:
>
The main reason I don't like static expecations is that often other
tests inserted before or after my test now have to know what to put
here. Specifically, the problem is that we expect the output not to be
named, but actually to be sha1 hex output. This seems really brittle
for a test.
> test_expect_success 'name-rev --refs excludes non-matched patterns' '
> echo "<tags/F" >expect &&
> git rev-list --left-right --right-only --cherry-pick F...E -- \
> bar >>expect &&
> [...]
>
> However, if I was asked for my preference, I would suggest to specify the
> `expect` contents explicitly, to document the expectation as of time of
> writing. The reason: I debugged my share of test breakages and these
> dynamically-generated `expect` files are the worst. When things break, you
> have to dig *real* deep to figure out what is going wrong, as sometimes
> the *generation of the `expect` file* regresses.
>
Do you have a better suggestion for how to check the expect vs actual
output ignoring the raw hex data that would be there otherwise? What I
want to avoid is a brittle test that depends precisely on actions of
prior tests in generating commits and tags. Specifically in this case,
we're going to end up with the sha1 hex ID of the commit in the
expected value.. hard-coding that feels wrong.
Thanks,
Jake
> Ciao,
> Dscho
^ permalink raw reply
* Re: [PATCH 3/5] name-rev: add support to discard refs by pattern match
From: Jacob Keller @ 2017-01-13 0:56 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <alpine.DEB.2.20.1701121056470.3469@virtualbox>
On Thu, Jan 12, 2017 at 1:57 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Jake,
>
> On Wed, 11 Jan 2017, Jacob Keller wrote:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Extend name-rev further to support matching refs by adding `--discard`
>> patterns.
>
> Same comment applies as for 5/5: `--exclude-refs` may be a better name
> than `--discard`.
>
I agree, will change.
Thanks,
Jake
> Ciao,
> Dscho
^ permalink raw reply
* Re: [PATCH 5/5] describe: teach describe negative pattern matches
From: Jacob Keller @ 2017-01-13 0:59 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <5f723a0d-623f-bf97-00de-29d430484fed@kdbg.org>
On Thu, Jan 12, 2017 at 5:45 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 12.01.2017 um 01:17 schrieb Jacob Keller:
>>
>> 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*"
>
>
> I have a few dozen topic branches, many of them are work in progress and
> named wip/something. To see the completed branches, I routinely say
>
> gitk --exclude=wip/* --branches
>
> these days.
>
> It would be great if you could provide the same user interface here. The
> example in the commit message would then look like this:
>
> git describe --contains --exclude="*rc*" --match="v*"
>
> (I'm not saying that you should add --branches, but that you should prefer
> --exclude over --discard. Also, the order of --exclude and --match would be
> important.)
I think that --exclude makes sense, but the current implementation
does not differentiate ordering, since both are merely accumulated
into string_lists and then matched together. I'm not sure how order
would impact things here? In the current implementation, if something
is excluded and matched, it will be excluded. That is, exclusion
patterns take precedence over match patterns. I think this makes the
most sense semantically.
Thanks,
Jake
>
> -- Hannes
>
^ permalink raw reply
* Re: Bug report: Documentation error in git-bisect man description
From: Christian Couder @ 2017-01-13 1:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Manuel Ullmann, git
In-Reply-To: <xmqq8tqfj15z.fsf@gitster.mtv.corp.google.com>
On Fri, Jan 13, 2017 at 12:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Manuel Ullmann <ullman.alias@posteo.de> writes:
>>
>> Hmmm, I tend to agree, modulo a minor fix.
>>
>> If the description were in a context inside a paragraph like this:
>>
>> When you want to tell 'git bisect' that a <rev> belongs to
>> the newer half of the history, you say
>>
>> git bisect (bad|new) [<rev>]
>>
>> On the other hand, when you want to tell 'git bisect' that a
>> <rev> belongs to the older half of the history, you can say
>>
>> git bisect (good|old) [<rev>]
>>
>> then the pairing we see in the current text makes quite a lot of
>> sense.
>
> Actually, the above is _exactly_ what was intended. I misread the
> current documentation when I made the comment, and I think that the
> current one _IS_ correct. The latter half of the above is not about
> a single rev. You can paint multiple commits with the "older half"
> color, i.e.
>
> On the other hand, when you want to tell 'git bisect' that
> one or more <rev>s belong to the older half of the history,
> you can say
>
> git bisect (good|old) [<rev>...]
>
> In contrast, you can mark only one <rev> as newer (or "already
> bad"). So pairing (bad|good) and (new|old) like you suggested
> breaks the correctness of the command line description.
Yeah, I agree.
> If (bad|new) and (good|old) bothers you because they may mislead the
> readers to think bad is an opposite of new (and good is an opposite
> of old), the only solution I can think of to that problem is to
> expand these two lines into four and list them like this:
>
> git bisect bad [<rev>]
> git bisect good [<rev>...]
> git bisect new [<rev>]
> git bisect old [<rev>...]
Maybe it would be more complete and a bit clearer if it was:
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
^ permalink raw reply
* Re: Bug report: Documentation error in git-bisect man description
From: Manuel Ullmann @ 2017-01-13 1:39 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, git
In-Reply-To: <CAP8UFD3JDWVVTWsSQcnh+dOHoZDcipVUFwkfQYNOAyV4431C2w@mail.gmail.com>
> On Fri, Jan 13, 2017 at 12:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Manuel Ullmann <ullman.alias@posteo.de> writes:
>>>
>>> Hmmm, I tend to agree, modulo a minor fix.
>>>
>>> If the description were in a context inside a paragraph like this:
>>>
>>> When you want to tell 'git bisect' that a <rev> belongs to
>>> the newer half of the history, you say
>>>
>>> git bisect (bad|new) [<rev>]
>>>
>>> On the other hand, when you want to tell 'git bisect' that a
>>> <rev> belongs to the older half of the history, you can say
>>>
>>> git bisect (good|old) [<rev>]
>>>
>>> then the pairing we see in the current text makes quite a lot of
>>> sense.
>>
>> Actually, the above is _exactly_ what was intended. I misread the
>> current documentation when I made the comment, and I think that the
>> current one _IS_ correct. The latter half of the above is not about
>> a single rev. You can paint multiple commits with the "older half"
>> color, i.e.
>>
>> On the other hand, when you want to tell 'git bisect' that
>> one or more <rev>s belong to the older half of the history,
>> you can say
>>
>> git bisect (good|old) [<rev>...]
>>
>> In contrast, you can mark only one <rev> as newer (or "already
>> bad"). So pairing (bad|good) and (new|old) like you suggested
>> breaks the correctness of the command line description.
>
> Yeah, I agree.
>
>> If (bad|new) and (good|old) bothers you because they may mislead the
>> readers to think bad is an opposite of new (and good is an opposite
>> of old), the only solution I can think of to that problem is to
>> expand these two lines into four and list them like this:
>>
>> git bisect bad [<rev>]
>> git bisect good [<rev>...]
>> git bisect new [<rev>]
>> git bisect old [<rev>...]
>
> Maybe it would be more complete and a bit clearer if it was:
>
> git bisect (bad|new|<term-new>) [<rev>]
> git bisect (good|old|<term-old>) [<rev>...]
That would clarify the intention quite a bit (at least for me).
Best regards,
Manuel
^ permalink raw reply
* [RFC-PATCH] lib-submodule-update.sh: define tests for recursing into submodules
From: Stefan Beller @ 2017-01-13 1:51 UTC (permalink / raw)
Cc: git, jacob.keller, bmwill, Jens.Lehmann, Stefan Beller
Currently lib-submodule-update.sh provides 2 functions
test_submodule_switch and test_submodule_forced_switch that are used by a
variety of tests to ensure that submodules behave as expected. The current
expected behavior is that submodules are not touched at all.
(See 42639d2317a for the exact setup)
In the future we want to teach all these commands to properly recurse
into submodules. To do that, we'll add two testing functions to
submodule-update-lib.sh test_submodule_switch_recursing and
test_submodule_forced_switch_recursing.
These two functions behave in analogy to the already existing functions
just with a different expectation on submodule behavior. The submodule
in the working tree is expected to be updated to the recorded submodule
version. The behavior is analogous to e.g. the behavior of files in a
nested directory in the working tree, where a change to the working tree
handles any arising directory/file conflicts just fine.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
So I have been looking into implementing "checkout --recurse-submodules",
for quite some time, but the correct approach is to not just make git-checkout
support submodules, but all the working tree operations at the same time.
Currently all the working tree operations have a test that submodules are
not touched even when the superproject is messing with the submodule,
so all submodule operations are tested via the submodule library, e.g.:
test_submodule_switch "git pull"
would see what happens for git-pull. Below I am proposing a test
that can be used for any operation that is aware of submodules, e.g.
eventually we'll have checks like:
test_submodule_switch_recursing "git checkout --recurse-submodules"
# as well as
test_submodule_switch_recursing "git pull --recurse-submodules"
This RFC email is asking if the behavior is sound and expected for submodule
operations in the working tree.
Thanks,
Stefan
t/lib-submodule-update.sh | 465 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 463 insertions(+), 2 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 79cdd34a54..bf33c30409 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,6 +4,7 @@
# - New submodule (no_submodule => add_sub1)
# - Removed submodule (add_sub1 => remove_sub1)
# - Updated submodule (add_sub1 => modify_sub1)
+# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
# - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
# - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
# - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -20,8 +21,8 @@
# / replace_sub1_with_directory
# /----O
# / ^
-# / modify_sub1
-# O------O-------O
+# / modify_sub1 modify_sub1_recursive
+# O------O-------O----------------O
# ^ ^\ ^
# | | \ remove_sub1
# | | -----O-----O
@@ -67,6 +68,14 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
+ git checkout -b "modify_sub1_recursively" "modify_sub1" &&
+ git -C sub1 checkout -b "add_nested_sub" &&
+ git -C sub1 submodule add --branch no_submodule ./. sub2 &&
+ git -C sub1 commit -a -m "add a nested submodule" &&
+ git add sub1 &&
+ git commit -a -m "update submodule, that updates a nested submodule" &&
+ git -C sub1 submodule deinit -f --all &&
+
git checkout -b "replace_sub1_with_directory" "add_sub1" &&
git submodule update &&
(
@@ -133,6 +142,15 @@ test_git_directory_is_unchanged () {
)
}
+test_git_directory_exists() {
+ test -e ".git/modules/$1" &&
+ (
+ cd ".git/modules/$1" &&
+ # does core.worktree point at the right place?
+ test "$(git config core.worktree)" = "../../../$1"
+ )
+}
+
# Helper function to be executed at the start of every test below, it sets up
# the submodule repo if it doesn't exist and configures the most problematic
# settings for diff.ignoreSubmodules.
@@ -678,3 +696,446 @@ test_submodule_forced_switch () {
)
'
}
+
+# Test that submodule contents are correctly updated when switching
+# between commits that change a submodule.
+# Test that the following transitions are correctly handled:
+# (These tests are also above in the case where we expect no change
+# in the submodule)
+# - Updated submodule
+# - New submodule
+# - Removed submodule
+# - Directory containing tracked files replaced by submodule
+# - Submodule replaced by tracked files in directory
+# - Submodule replaced by tracked file with the same name
+# - tracked file replaced by submodule
+#
+# New test cases
+# - Removing a submodule with a git directory absorbs the submodules
+# git directory first into the superproject.
+
+test_submodule_switch_recursing () {
+ command="$1"
+ ######################### Appearing submodule #########################
+ # Switching to a commit letting a submodule appear checks it out ...
+ test_expect_success "$command: added submodule is checked out" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... ignoring an empty existing directory ...
+ test_expect_success "$command: added submodule is checked out in empty dir" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ mkdir sub1 &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... unless there is an untracked file in its place.
+ test_expect_success "$command: added submodule doesn't remove untracked file with same name" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ : >sub1 &&
+ test_must_fail $command add_sub1 &&
+ test_superproject_content origin/no_submodule &&
+ test_must_be_empty sub1
+ )
+ '
+
+ # ... but an ignored file is fine.
+ test_expect_success "$command: added submodule removes an untracked ignored file" '
+ test_when_finished "rm submodule_update/.git/info/exclude" &&
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ : >sub1 &&
+ echo sub1 > .git/info/exclude
+ $command add_sub1 &&
+ test_superproject_content origin/no_submodule &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+
+ # Replacing a tracked file with a submodule produces a checked out submodule
+ test_expect_success "$command: replace tracked file with submodule checks out submodule" '
+ prolog &&
+ reset_work_tree_to replace_sub1_with_file &&
+ (
+ cd submodule_update &&
+ git branch -t replace_file_with_sub1 origin/replace_file_with_sub1 &&
+ $command replace_file_with_sub1 &&
+ test_superproject_content origin/replace_file_with_sub1 &&
+ test_submodule_content sub1 origin/replace_file_with_sub1
+ )
+ '
+ # ... as does removing a directory with tracked files with a
+ # submodule.
+ test_expect_success "$command: replace directory with submodule" '
+ prolog &&
+ reset_work_tree_to replace_sub1_with_directory &&
+ (
+ cd submodule_update &&
+ git branch -t replace_directory_with_sub1 origin/replace_directory_with_sub1 &&
+ $command replace_directory_with_sub1 &&
+ test_superproject_content origin/replace_directory_with_sub1 &&
+ test_submodule_content sub1 origin/replace_directory_with_sub1
+ )
+ '
+
+ ######################## Disappearing submodule #######################
+ # Removing a submodule removes its work tree ...
+ test_expect_success "$command: removed submodule removes submodules working tree" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1
+ )
+ '
+ # ... absorbing a .git directory along the way.
+ test_expect_success "$command: removed submodule absorbs submodules .git directory" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1 &&
+ test_git_directory_exists sub1
+ )
+ '
+ # Replacing a submodule with files in a directory must succeeds
+ # when the submodule is clean
+ test_expect_success "$command: replace submodule with a directory" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_submodule_content sub1 origin/replace_sub1_with_directory
+ )
+ '
+ # ... absorbing a .git directory.
+ test_expect_success "$command: replace submodule containing a .git directory with a directory must absorb the git dir" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_git_directory_exists sub1
+ )
+ '
+
+ # Replacing it with a file ...
+ test_expect_success "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file &&
+ test -f sub1
+ )
+ '
+
+ # ... must check its local work tree for untracked files
+ test_expect_success "$command: replace submodule with a file must fail with untracked files" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ > sub1/untrackedfile &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file &&
+ test -f sub1
+ )
+ '
+
+ # ... and ignored files are ignroed
+ test_expect_success "$command: replace submodule with a file works ignores ignored files in submodule" '
+ test_when_finished "rm .git/modules/sub1/info/exclude" &&
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ > sub1/ignored &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file &&
+ test -f sub1
+ )
+ '
+
+ ########################## Modified submodule #########################
+ # Updating a submodule sha1 updates the submodule's work tree
+ test_expect_success "$command: modified submodule updates submodule work tree" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1 origin/modify_sub1 &&
+ $command modify_sub1 &&
+ test_superproject_content origin/modify_sub1 &&
+ test_submodule_content sub1 origin/modify_sub1
+ )
+ '
+
+ # Updating a submodule to an invalid sha1 doesn't update the
+ # superproject nor the submodule's work tree.
+ test_expect_success "$command: updating to a missing submodule commit fails" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t invalid_sub1 origin/invalid_sub1 &&
+ test_must_fail $command invalid_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+
+ test_expect_success "$command: modified submodule updates submodule recursively" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1_recursively origin/modify_sub1_recursively &&
+ $command modify_sub1_recursively &&
+ test_superproject_content origin/modify_sub1_recursively &&
+ test_submodule_content sub1 origin/modify_sub1_recursively
+ test_submodule_content sub1/sub2
+ )
+ '
+}
+
+# Test that submodule contents are updated when switching between commits
+# that change a submodule, but throwing away local changes in
+# the superproject as well as the submodule is allowed.
+test_submodule_forced_switch_recursing () {
+ command="$1"
+ ######################### Appearing submodule #########################
+ # Switching to a commit letting a submodule appear creates empty dir ...
+ test_expect_success "$command: added submodule is checked out" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... and doesn't care if it already exists ...
+ test_expect_success "$command: added submodule ignores empty directory" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ mkdir sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # ... not caring about an untracked file either
+ test_expect_success "$command: added submodule does remove untracked unignored file with same name when forced" '
+ prolog &&
+ reset_work_tree_to no_submodule &&
+ (
+ cd submodule_update &&
+ git branch -t add_sub1 origin/add_sub1 &&
+ >sub1 &&
+ $command add_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # Replacing a tracked file with a submodule checks out the submodule
+ test_expect_success "$command: replace tracked file with submodule populates the submodule" '
+ prolog &&
+ reset_work_tree_to replace_sub1_with_file &&
+ (
+ cd submodule_update &&
+ git branch -t replace_file_with_sub1 origin/replace_file_with_sub1 &&
+ $command replace_file_with_sub1 &&
+ test_superproject_content origin/replace_file_with_sub1 &&
+ test_submodule_content sub1 origin/replace_file_with_sub1
+ )
+ '
+ # ... as does removing a directory with tracked files with a
+ # submodule.
+ test_expect_success "$command: replace directory with submodule" '
+ prolog &&
+ reset_work_tree_to replace_sub1_with_directory &&
+ (
+ cd submodule_update &&
+ git branch -t replace_directory_with_sub1 origin/replace_directory_with_sub1 &&
+ $command replace_directory_with_sub1 &&
+ test_superproject_content origin/replace_directory_with_sub1 &&
+ test_submodule_content sub1 origin/replace_directory_with_sub1
+ )
+ '
+
+ ######################## Disappearing submodule #######################
+ # Removing a submodule doesn't remove its work tree ...
+ test_expect_success "$command: removed submodule leaves submodule directory and its contents in place" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ ! test -e sub1
+ )
+ '
+ # ... especially when it contains a .git directory.
+ test_expect_success "$command: removed submodule leaves submodule containing a .git directory alone" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t remove_sub1 origin/remove_sub1 &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules/sub1 &&
+ $command remove_sub1 &&
+ test_superproject_content origin/remove_sub1 &&
+ test_git_directory_exists sub1 &&
+ ! test -e sub1
+ )
+ '
+ # Replacing a submodule with files in a directory ...
+ test_expect_failure "$command: replace submodule with a directory" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory
+ )
+ '
+ # ... absorbing a .git directory.
+ test_expect_failure "$command: replace submodule containing a .git directory with a directory must fail" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
+ replace_gitfile_with_git_dir sub1 &&
+ rm -rf .git/modules/sub1 &&
+ $command replace_sub1_with_directory &&
+ test_superproject_content origin/replace_sub1_with_directory &&
+ test_git_directory_exists sub1
+ )
+ '
+ # Replacing it with a file
+ test_expect_failure "$command: replace submodule with a file must fail" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file
+ )
+ '
+
+ # ... even if the submodule contains ignored files
+ test_expect_failure "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : > sub1/expect &&
+ $command replace_sub1_with_file &&
+ test_superproject_content origin/replace_sub1_with_file
+ )
+ '
+
+ # ... but stops for untracked files that would be lost
+ test_expect_failure "$command: replace submodule with a file" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
+ : > sub1/untracked_file &&
+ test_must_fail $command replace_sub1_with_file &&
+ test_superproject_content origin/add_sub1 &&
+ test -f sub1/untracked_file
+ )
+ '
+
+ ########################## Modified submodule #########################
+ # Updating a submodule sha1 updates the submodule's work tree
+ test_expect_success "$command: modified submodule updates submodule work tree" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t modify_sub1 origin/modify_sub1 &&
+ $command modify_sub1 &&
+ test_superproject_content origin/modify_sub1 &&
+ test_submodule_content sub1 origin/modify_sub1
+ )
+ '
+ # Updating a submodule to an invalid sha1 doesn't update the
+ # submodule's work tree, subsequent update will fail
+ test_expect_success "$command: modified submodule does not update submodule work tree to invalid commit" '
+ prolog &&
+ reset_work_tree_to add_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t invalid_sub1 origin/invalid_sub1 &&
+ test_must_fail $command invalid_sub1 &&
+ test_superproject_content origin/add_sub1 &&
+ test_submodule_content sub1 origin/add_sub1
+ )
+ '
+ # Updating a submodule from an invalid sha1 updates
+ test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
+ prolog &&
+ reset_work_tree_to invalid_sub1 &&
+ (
+ cd submodule_update &&
+ git branch -t valid_sub1 origin/valid_sub1 &&
+ $command valid_sub1 &&
+ test_superproject_content origin/valid_sub1 &&
+ test_submodule_content sub1 origin/valid_sub1
+ )
+ '
+}
--
2.11.0.299.g77584d2295
^ permalink raw reply related
* Re: [PATCH 2/2] Use 'env' to find perl instead of fixed path
From: Eric Wong @ 2017-01-13 2:48 UTC (permalink / raw)
To: Pat Pannuto; +Cc: Junio C Hamano, Johannes Schindelin, Johannes Sixt, git
In-Reply-To: <CAAnLKaGvz4Wzs36gMSdoYCg+tzx6KFCe59FNnk5zNQ-L58ww1g@mail.gmail.com>
Pat Pannuto <pat.pannuto@gmail.com> wrote:
> You may still want the 1/2 patch in this series, just to make things
> internally consistent with "-w" vs "use warnings;" inside git's perl
> scripts.
No, that is a step back. "-w" affects the entire process, so it
spots more potential problems. The "warnings" pragma is scoped
to the enclosing block, so it won't span across files.
Existing instances of "use warnings" should remain, but I would
rather support adding "-w" to scripts which do not have it (and
fixing newly-found warnings along the way).
Thanks.
^ permalink raw reply
* Re: [PATCH 5/5] describe: teach describe negative pattern matches
From: Johannes Sixt @ 2017-01-13 6:43 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CA+P7+xrmAmCPOzuaKcm+WxceXnowkM4gKz05tSpdC=CDwpCEug@mail.gmail.com>
Am 13.01.2017 um 01:59 schrieb Jacob Keller:
> I think that --exclude makes sense, but the current implementation
> does not differentiate ordering, since both are merely accumulated
> into string_lists and then matched together. I'm not sure how order
> would impact things here? In the current implementation, if something
> is excluded and matched, it will be excluded. That is, exclusion
> patterns take precedence over match patterns. I think this makes the
> most sense semantically.
When you write
git log --exclude=wip/* --branches --remotes
--exclude applies only to --branches, not to --remotes.
When you write
git log --branches --exclude=origin/* --remotes
--exclude=origin/* applies only to --remotes, but not to --branches.
-- Hannes
^ permalink raw reply
* Re: [PATCH 5/5] describe: teach describe negative pattern matches
From: Jacob Keller @ 2017-01-13 6:57 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <5c8401ef-9609-f235-9228-be980a13edf1@kdbg.org>
On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 13.01.2017 um 01:59 schrieb Jacob Keller:
>>
>> I think that --exclude makes sense, but the current implementation
>> does not differentiate ordering, since both are merely accumulated
>> into string_lists and then matched together. I'm not sure how order
>> would impact things here? In the current implementation, if something
>> is excluded and matched, it will be excluded. That is, exclusion
>> patterns take precedence over match patterns. I think this makes the
>> most sense semantically.
>
>
> When you write
>
> git log --exclude=wip/* --branches --remotes
>
> --exclude applies only to --branches, not to --remotes.
>
> When you write
>
> git log --branches --exclude=origin/* --remotes
>
> --exclude=origin/* applies only to --remotes, but not to --branches.
>
> -- Hannes
>
Well for describe I don't think the order matters.
Thanks,
Jake
^ permalink raw reply
* Re: "git fsck" not detecting garbage at the end of blob object files...
From: John Szakmeister @ 2017-01-13 9:15 UTC (permalink / raw)
To: Jeff King; +Cc: Dennis Kaarsemaker, git
In-Reply-To: <20170108052619.4ucjamsqad4g5add@sigill.intra.peff.net>
On Sun, Jan 8, 2017 at 12:26 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 07, 2017 at 10:47:03PM +0100, Dennis Kaarsemaker wrote:
>> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
>> > I was perusing StackOverflow this morning and ran across this
>> > question: http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
>> >
>> > It was a simple question about why "checking objects" was not
>> > appearing, but in it was another issue. The user purposefully
>> > corrupted a blob object file to see if `git fsck` would catch it by
>> > tacking extra data on at the end. `git fsck` happily said everything
>> > was okay, but when I played with things locally I found out that `git
>> > gc` does not like that extra garbage. I'm not sure what the trade-off
>> > needs to be here, but my expectation is that if `git fsck` says
>> > everything is okay, then all operations using that object (file)
>> > should work too.
>> >
>> > Is that unreasonable? What would be the impact of fixing this issue?
>>
>> If you do this with a commit object or tree object, fsck does complain.
>> I think it's sensible to do so for blob objects as well.
>
> The existing extra-garbage check is in unpack_sha1_rest(), which is
> called as part of read_sha1_file(). And that's what we hit for commits
> and trees. However, we check the sha1 of blobs using the streaming
> interface (in case they're large). I think you'd want to put a similar
> check into read_istream_loose(). But note if you are grepping for it, it
> is hidden behind a macro; look for read_method_decl(loose).
That's for the pointer.
> I'm actually not sure if this should be downgrade to a warning. It's
> true that it's a form of corruption, but it doesn't actually prohibit us
> from getting the data we need to complete the operation. Arguably fsck
> should be more picky, but it is just relying on the same parse_object()
> code path that the rest of git uses.
>
> I doubt anybody cares too much either way, though. It's not like this is
> a common thing.
I kind of wonder about that myself too, and I'm not sure what to
think about it. On the one hand, I'd like to know about
*anything* that has changed in an adverse way--it could indicate
a failure somewhere else that needs to be handled. On the other
hand, scaring the user isn't all that advantageous. I guess I'm
in the former camp.
As to whether this is common, yeah, it's probably not. However,
I was surprised by the number of results that turned up when I
search for "garbage at end of loose object".
> I did notice another interesting case when looking at this. Fsck ends up
> in fsck_loose(), which has the sha1 and path of the loose object. It
> passes the sha1 to fsck_sha1(), and ignores the path entirely!
>
> So if you have a duplicate copy of the object in a pack, we'd actually
> find and check the duplicate. This can happen, e.g., if you had a loose
> object and fetched a thin-pack which made a copy of the loose object to
> complete the pack).
>
> Probably fsck_loose() should be more picky about making sure we are
> reading the data from the loose version we found.
Interesting find! Thanks for the information Peff!
-John
^ permalink raw reply
* Re: "git fsck" not detecting garbage at the end of blob object files...
From: John Szakmeister @ 2017-01-13 9:16 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git
In-Reply-To: <1483825623.31837.9.camel@kaarsemaker.net>
On Sat, Jan 7, 2017 at 4:47 PM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
>> I was perusing StackOverflow this morning and ran across this
>> question: http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
>>
>> It was a simple question about why "checking objects" was not
>> appearing, but in it was another issue. The user purposefully
>> corrupted a blob object file to see if `git fsck` would catch it by
>> tacking extra data on at the end. `git fsck` happily said everything
>> was okay, but when I played with things locally I found out that `git
>> gc` does not like that extra garbage. I'm not sure what the trade-off
>> needs to be here, but my expectation is that if `git fsck` says
>> everything is okay, then all operations using that object (file)
>> should work too.
>>
>> Is that unreasonable? What would be the impact of fixing this issue?
>
> If you do this with a commit object or tree object, fsck does complain.
> I think it's sensible to do so for blob objects as well.
Also very good information. Thanks Dennis!
-John
^ permalink raw reply
* [PATCH v2 0/2] diff --no-index: support symlinks and pipes
From: Dennis Kaarsemaker @ 2017-01-13 10:20 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker
git diff <(command1) <(command2) is less useful than it could be, all it outputs is:
diff --git a/dev/fd/63 b/dev/fd/62
index 9e6542b297..9f7b2c291b 120000
--- a/dev/fd/63
+++ b/dev/fd/62
@@ -1 +1 @@
-pipe:[464811685]
\ No newline at end of file
+pipe:[464811687]
\ No newline at end of file
Normal diff provides arguably better output: the diff of the output of the
commands. This series makes it possible for git diff --no-index to follow
symlinks and read from pipes, mimicking the behaviour of normal diff.
v1: http://public-inbox.org/git/20161111201958.2175-1-dennis@kaarsemaker.net/
Changes since the RFC/v1 patch:
- Following symlinks is now the default. I think an accurate summary of the
discussion on v1 is that this behaviour is useful enough to be the default,
but to add an escape hatch. That escape hatch is named --no-dereference, name
stolen from gnu diff.
- Added tests and documentation
Specifically not changed:
These changes affect only diff --no-index. Using --no-dereference is an error
without --no-index.
Dennis Kaarsemaker (2):
diff --no-index: follow symlinks
diff --no-index: support reading from pipes
Documentation/diff-options.txt | 7 +++++++
diff-no-index.c | 15 ++++++++++++---
diff.c | 23 +++++++++++++++++++----
diff.h | 2 +-
t/t4053-diff-no-index.sh | 40 ++++++++++++++++++++++++++++++++++++++++
t/test-lib.sh | 4 ++++
6 files changed, 83 insertions(+), 8 deletions(-)
--
2.11.0-234-gaf85957
^ permalink raw reply related
* [PATCH v2 2/2] diff --no-index: support reading from pipes
From: Dennis Kaarsemaker @ 2017-01-13 10:20 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker
In-Reply-To: <20170113102021.6054-1-dennis@kaarsemaker.net>
diff <(command1) <(command2) provides useful output, let's make it
possible for git to do the same.
Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
diff-no-index.c | 8 ++++++++
diff.c | 13 +++++++++++--
t/t4053-diff-no-index.sh | 10 ++++++++++
t/test-lib.sh | 4 ++++
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 826fe97ffc..2123da435b 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -83,6 +83,14 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)
name = "/dev/null";
s = alloc_filespec(name);
fill_filespec(s, null_sha1, 0, mode);
+ /*
+ * In --no-index mode, we support reading from pipes. canon_mode, called by
+ * fill_filespec, gets confused by this and thinks we now have subprojects.
+ * Detect this and tell the rest of the diff machinery to treat pipes as
+ * normal files.
+ */
+ if (S_ISGITLINK(s->mode))
+ s->mode = S_IFREG | ce_permissions(mode);
if (name == file_from_standard_input)
populate_from_stdin(s);
return s;
diff --git a/diff.c b/diff.c
index 2fc0226338..bb04eab331 100644
--- a/diff.c
+++ b/diff.c
@@ -2839,9 +2839,18 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
fd = open(s->path, O_RDONLY);
if (fd < 0)
goto err_empty;
- s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
+ if (!S_ISREG(st.st_mode)) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_read(&sb, fd, 0);
+ s->size = sb.len;
+ s->data = strbuf_detach(&sb, NULL);
+ s->should_free = 1;
+ }
+ else {
+ s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
+ s->should_munmap = 1;
+ }
close(fd);
- s->should_munmap = 1;
/*
* Convert from working tree format to canonical git format
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index c6046fef19..ba343566c0 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -157,4 +157,14 @@ test_expect_success SYMLINKS 'diff --no-index --no-dereference does not follow s
test_cmp expect actual
'
+test_expect_success PROCESS_SUBSTITUTION 'diff --no-index works on fifos' '
+ cat >expect <<\EOF
+ @@ -1 +1 @@
+ -1
+ +2
+ EOF
+ test_expect_code 1 git diff --no-index <(echo 1) <(echo 2) | tail -n +5 > actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 11562bde10..78f3d24651 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1128,3 +1128,7 @@ build_option () {
test_lazy_prereq LONG_IS_64BIT '
test 8 -le "$(build_option sizeof-long)"
'
+
+test_lazy_prereq PROCESS_SUBSTITUTION '
+ eval "foo=<(echo test)" 2>/dev/null
+'
--
2.11.0-234-gaf85957
^ permalink raw reply related
* [PATCH v2 1/2] diff --no-index: follow symlinks
From: Dennis Kaarsemaker @ 2017-01-13 10:20 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker
In-Reply-To: <20170113102021.6054-1-dennis@kaarsemaker.net>
Git's diff machinery does not follow symlinks, which makes sense as git
itself also does not, but stores the symlink destination.
In --no-index mode however, it is useful for diff to to follow symlinks,
matching the behaviour of ordinary diff. A new --no-dereference (name
copied from diff) option has been added to disable this behaviour.
Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
Documentation/diff-options.txt | 7 +++++++
diff-no-index.c | 7 ++++---
diff.c | 10 ++++++++--
diff.h | 2 +-
t/t4053-diff-no-index.sh | 30 ++++++++++++++++++++++++++++++
5 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 2d77a19626..48bcf3cc5e 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -216,6 +216,13 @@ any of those replacements occurred.
commit range. Defaults to `diff.submodule` or the 'short' format
if the config option is unset.
+ifdef::git-diff[]
+--no-dereference::
+ Normally, "git diff --no-index" will dereference symlinks and compare
+ the contents of the linked files, mimicking ordinary diff. This
+ option disables that behaviour.
+endif::git-diff[]
+
--color[=<when>]::
Show colored diff.
`--color` (i.e. without '=<when>') is the same as `--color=always`.
diff --git a/diff-no-index.c b/diff-no-index.c
index f420786039..826fe97ffc 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -40,7 +40,7 @@ static int read_directory_contents(const char *path, struct string_list *list)
*/
static const char file_from_standard_input[] = "-";
-static int get_mode(const char *path, int *mode)
+static int get_mode(const char *path, int *mode, int no_dereference)
{
struct stat st;
@@ -52,7 +52,7 @@ static int get_mode(const char *path, int *mode)
#endif
else if (path == file_from_standard_input)
*mode = create_ce_mode(0666);
- else if (lstat(path, &st))
+ else if (no_dereference ? lstat(path, &st) : stat(path, &st))
return error("Could not access '%s'", path);
else
*mode = st.st_mode;
@@ -93,7 +93,8 @@ static int queue_diff(struct diff_options *o,
{
int mode1 = 0, mode2 = 0;
- if (get_mode(name1, &mode1) || get_mode(name2, &mode2))
+ if (get_mode(name1, &mode1, DIFF_OPT_TST(o, NO_DEREFERENCE)) ||
+ get_mode(name2, &mode2, DIFF_OPT_TST(o, NO_DEREFERENCE)))
return -1;
if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {
diff --git a/diff.c b/diff.c
index be11e4ef2b..2fc0226338 100644
--- a/diff.c
+++ b/diff.c
@@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
s->size = xsize_t(st.st_size);
if (!s->size)
goto empty;
- if (S_ISLNK(st.st_mode)) {
+ if (S_ISLNK(s->mode)) {
struct strbuf sb = STRBUF_INIT;
if (strbuf_readlink(&sb, s->path, s->size))
@@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
s->should_free = 1;
return 0;
}
+ if (S_ISLNK(st.st_mode)) {
+ stat(s->path, &st);
+ s->size = xsize_t(st.st_size);
+ }
if (size_only)
return 0;
if ((flags & CHECK_BINARY) &&
@@ -3884,7 +3888,9 @@ int diff_opt_parse(struct diff_options *options,
else if (!strcmp(arg, "--no-follow")) {
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
- } else if (!strcmp(arg, "--color"))
+ } else if (!strcmp(arg, "--no-dereference"))
+ DIFF_OPT_SET(options, NO_DEREFERENCE);
+ else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (skip_prefix(arg, "--color=", &arg)) {
int value = git_config_colorbool(NULL, arg);
diff --git a/diff.h b/diff.h
index 25ae60d5ff..74883db1eb 100644
--- a/diff.h
+++ b/diff.h
@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)
#define DIFF_OPT_FOLLOW_RENAMES (1 << 7)
#define DIFF_OPT_RENAME_EMPTY (1 << 8)
-/* (1 << 9) unused */
+#define DIFF_OPT_NO_DEREFERENCE (1 << 9)
#define DIFF_OPT_HAS_CHANGES (1 << 10)
#define DIFF_OPT_QUICK (1 << 11)
#define DIFF_OPT_NO_INDEX (1 << 12)
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 453e6c35eb..c6046fef19 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -127,4 +127,34 @@ test_expect_success 'diff --no-index from repo subdir respects config (implicit)
test_cmp expect actual.head
'
+test_expect_success SYMLINKS 'diff --no-index follows symlinks' '
+ echo a >1 &&
+ echo b >2 &&
+ ln -s 1 3 &&
+ ln -s 2 4 &&
+ cat >expect <<\EOF
+ --- a/3
+ +++ b/4
+ @@ -1 +1 @@
+ -a
+ +b
+ EOF
+ test_expect_code 1 git diff --no-index 3 4 | tail -n +3 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success SYMLINKS 'diff --no-index --no-dereference does not follow symlinks' '
+ cat >expect <<\EOF
+ --- a/3
+ +++ b/4
+ @@ -1 +1 @@
+ -1
+ \ No newline at end of file
+ +2
+ \ No newline at end of file
+ EOF
+ test_expect_code 1 git diff --no-index --no-dereference 3 4 | tail -n +3 > actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0-234-gaf85957
^ permalink raw reply related
* Re: gitk: "lime" color incompatible with older Tk versions
From: David Aguilar @ 2017-01-13 11:20 UTC (permalink / raw)
To: Stefan Beller; +Cc: Andrew Janke, Paul Mackerras, git@vger.kernel.org
In-Reply-To: <CAGZ79kaO9T+Qc=M6s_ZdpAfLZCVQEYNF=zNxDWArDmsA7jjCWg@mail.gmail.com>
On Mon, May 02, 2016 at 09:20:43AM -0700, Stefan Beller wrote:
> + Paul Mackerras, who maintains gitk
>
> On Sun, May 1, 2016 at 10:03 AM, Andrew Janke <floss@apjanke.net> wrote:
> > Hi, git folks,
> >
> > I'm having trouble running gitk on Mac OS X 10.9.5. The gitk program uses
> > the color "lime", which is not present in older versions of Tk, apparently
> > including the Tk 8.5 which ships with 10.9.
Ping.. it would be nice to get this patch applied.
I can verify that gitk on Mac OS X 10.11 also has this problem.
gitk is usually pretty good about backwards-compatibility.
> > This compatibility problem was noted before back in 2012, in
> > http://www.mail-archive.com/git%40vger.kernel.org/msg14496.html.
> >
> > Would you consider switching from lime to a hex value color, for
> > compatibility with users of older versions of Tk? A patch to do so is below;
> > only the file gitk-git/gitk needs to be changed.
I can recreate and resend this patch if needed; it's simply:
:%s/lime/"#99FF00"/g
Would a re-roll of this patch be accepted, or is it not worth
bothering?
Google for "gitk lime" to get a taste for some of the fallout
caused by this problem.
The fact that multiple pages, with different OS's, have examples
of users stumbling over this change is a good hint that it's
worth fixing.
Thoughts?
--
David
^ permalink raw reply
* [PATCH] Documentation/bisect: improve on (bad|new) and (good|bad)
From: Christian Couder @ 2017-01-13 14:44 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Manuel Ullmann, Matthieu Moy, Christian Couder
The following part of the description:
git bisect (bad|new) [<rev>]
git bisect (good|old) [<rev>...]
may be a bit confusing, as a reader may wonder if instead it should be:
git bisect (bad|good) [<rev>]
git bisect (old|new) [<rev>...]
Of course the difference between "[<rev>]" and "[<rev>...]" should hint
that there is a good reason for the way it is.
But we can further clarify and complete the description by adding
"<term-new>" and "<term-old>" to the "bad|new" and "good|old"
alternatives.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-bisect.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 2bb9a577a2..bdd915a66b 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -18,8 +18,8 @@ on the subcommand:
git bisect start [--term-{old,good}=<term> --term-{new,bad}=<term>]
[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
- git bisect (bad|new) [<rev>]
- git bisect (good|old) [<rev>...]
+ git bisect (bad|new|<term-new>) [<rev>]
+ git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-good | --term-bad]
git bisect skip [(<rev>|<range>)...]
git bisect reset [<commit>]
--
2.11.0.313.g11b7cc88e6.dirty
^ permalink raw reply related
* Re: [PATCH] Remove dependency on deprecated Net::SMTP::SSL
From: Renato Botelho @ 2017-01-13 14:59 UTC (permalink / raw)
To: brian m. carlson; +Cc: Mike Fisher, git
In-Reply-To: <20161120215344.jaqt4owlhovig3hz@genre.crustytoothpaste.net>
> On 20 Nov 2016, at 19:53, brian m. carlson <sandals@crustytoothpaste.net> wrote:
>
> On Sun, Nov 20, 2016 at 04:18:16PM -0500, Mike Fisher wrote:
>> Refactor send_message() to remove dependency on deprecated
>> Net::SMTP::SSL:
>>
>> <http://search.cpan.org/~rjbs/Net-SMTP-SSL-1.04/lib/Net/SMTP/SSL.pm#DEPRECATED>
>
> As much as I hate to say this, I think this is going to cause
> compatibility problems. Net::SMTP is part of core Perl (as of v5.7.3),
> but the version you want to rely on (which you did not provide an
> explicit dependency on) is from October 2014.
>
> That basically means that no Perl on a Red Hat or CentOS system is going
> to provide that support, since RHEL 7 was released in June 2014.
> Providing an updated Git on those platforms would require replacing the
> system Perl or parts of it, which would be undesirable. This would
> affect Debian 7 as well.
>
> We currently support Perl 5.8 [0], so if you want to remove support for
> Net::SMTP::SSL, I'd recommend a solution that works with that version.
>
> [0] I personally believe we should drop support for Perl older than
> 5.10.1 (if not newer), but that's my opinion and it isn't shared by
> other list regulars.
> --
> brian m. carlson / brian with sandals: Houston, Texas, US
> +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: https://keybase.io/bk2204
Net::SMTP::SSL is marked as DEPRECATED on FreeBSD ports tree and will be removed in 2017-03-31. When it happens users will not be able to run git-send-email anymore. I’m considering to add Mike’s patch to FreeBSD ports tree as an alternative but it would be good to have a official solution for this problem.
FreeBSD bug report can be found at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214335
--
Renato Botelho
^ permalink raw reply
* Re: [PATCH 2/2] Use 'env' to find perl instead of fixed path
From: Johannes Schindelin @ 2017-01-13 15:21 UTC (permalink / raw)
To: Pat Pannuto; +Cc: Junio C Hamano, Johannes Sixt, git
In-Reply-To: <CAAnLKaGvz4Wzs36gMSdoYCg+tzx6KFCe59FNnk5zNQ-L58ww1g@mail.gmail.com>
Hi Pat,
On Thu, 12 Jan 2017, Pat Pannuto wrote:
> You may still want the 1/2 patch in this series, just to make things
> internally consistent with "-w" vs "use warnings;" inside git's perl
> scripts.
Yes, I like that patch.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 0/3] updates to gitk & git-gui doc now gitview has gone
From: Johannes Schindelin @ 2017-01-13 15:23 UTC (permalink / raw)
To: Philip Oakley; +Cc: GitList, Junio C Hamano
In-Reply-To: <20170112213240.7972-1-philipoakley@iee.org>
Hi Philip,
On Thu, 12 Jan 2017, Philip Oakley wrote:
> gitview was removed recently.
>
> > Sent: Tuesday, January 10, 2017 11:48 PM
> > Subject: What's cooking in git.git (Jan 2017, #01; Tue, 10)
>
> > * sb/remove-gitview (2017-01-07) 1 commit
> > (merged to 'next' on 2017-01-10 at dcb3abd146)
> > + contrib: remove gitview
>
> > Will merge to 'master'.
>
>
> Lets remove the reference in the gitk man page, and do some other
> fixups while in the area.
These changes all look sensible to me.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] Use 'env' to find perl instead of fixed path
From: Johannes Schindelin @ 2017-01-13 15:27 UTC (permalink / raw)
To: Eric Wong; +Cc: Pat Pannuto, Junio C Hamano, Johannes Sixt, git
In-Reply-To: <20170113024842.GA20572@starla>
Hi Eric,
On Fri, 13 Jan 2017, Eric Wong wrote:
> Pat Pannuto <pat.pannuto@gmail.com> wrote:
> > You may still want the 1/2 patch in this series, just to make things
> > internally consistent with "-w" vs "use warnings;" inside git's perl
> > scripts.
>
> No, that is a step back. "-w" affects the entire process, so it
> spots more potential problems.
I guess I do not understand, still, what the difference is between using
-w and adding `use warnings` *very early* in the script... Could you give
an example where it makes a difference?
Ciao,
Dscho
^ permalink raw reply
* [ANNOUNCE] Git for Windows 2.11.0(2)
From: Johannes Schindelin @ 2017-01-13 15:43 UTC (permalink / raw)
To: git-for-windows, git; +Cc: Johannes Schindelin
MIME-Version: 1.0
Fcc: Sent
Dear Git users,
It is my pleasure to announce that Git for Windows 2.11.0(2) is available from:
https://git-for-windows.github.io/
Changes since Git for Windows v2.11.0 (December 1st 2016)
New Features
• Reading a large index has been speeded up using pthreads.
• The checkout operation was speeded up for the common cases.
• The status operation was made faster in large worktrees with many
changes.
• The diff operation saw performance improvements when working on a
huge number of renamed files.
• PuTTY's plink.exe can now be used in GIT_SSH_COMMAND without
jumping through hoops, too.
• The MSYS2 runtime was synchronized with Cygwin 2.6.1.
Bug Fixes
• Non-ASCII characters are now shown properly again in Git Bash.
• Implicit NTLM authentication works again when accessing a remote
repository via HTTP/HTTPS without having to specify empty user name
and password.
• Our poll() emulation now uses 64-bit tick counts to avoid the (very
rare) wraparound issue where it could miscalculate time differences
every 49 days.
• The --no-lock-index option of git status is now also respected also
in submodules.
• The regression of v2.11.0 where Git could no longer push to shared
folders via UNC paths is fixed.
• A bug in the MSYS2 runtime where it performed POSIX->Windows
argument conversion incorrectly was fixed.
• The MSYS2 runtime was prepared to access the FAST_CWD internal data
structure in upcoming Windows versions.
• Fixed a bug in the experimental builtin difftool where it would not
handle copied/renamed files properly.
Filename | SHA-256
-------- | -------
Git-2.11.0.2-64-bit.exe | c7c6f8ba88a6da491117e03df559abd0fece1352f40d8b2d9bffc9c6c12c5800
Git-2.11.0.2-32-bit.exe | f7862331c994072402f9d7f03a4a6e2caec8ce0e91581ffbc6114631e920d9c9
PortableGit-2.11.0.2-64-bit.7z.exe | d4de119186bd63535fb792d73437cd0e2eb640ad50572ef7e04013f96aa70270
PortableGit-2.11.0.2-32-bit.7z.exe | 1f871552d1736bf86b08f81a55a29ff5aba9943d3c77b16294bdffca8c066f09
MinGit-2.11.0.2-64-bit.zip | 77558f381d21175dc017e90bc9cab90c8850bff6348a7dd112780f4f1f256449
MinGit-2.11.0.2-32-bit.zip | 23167f05f1274e169d42677a99bbc3e03e879250c71d5dcce6b0fbf3164013b8
Git-2.11.0.2-64-bit.tar.bz2 | 28d6d35db9f706c0439a55183ff68d9bbef9b67d66b11ada3775b2492a1d67bb
Git-2.11.0.2-32-bit.tar.bz2 | 592bbfd08c91b5fc826116b0ee29d9949e8e95b3094b81f0d1acea045e98fb64
Ciao,
Johannes
^ permalink raw reply
* [RFC] Add support for downloading blobs on demand
From: Ben Peart @ 2017-01-13 15:52 UTC (permalink / raw)
To: git; +Cc: benpeart
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4344 bytes --]
Goal
~~~~
To be able to better handle repos with many files that any individual
developer doesn’t need it would be nice if clone/fetch only brought down
those files that were actually needed.
To enable that, we are proposing adding a flag to clone/fetch that will
instruct the server to limit the objects it sends to commits and trees
and to not send any blobs.
When git performs an operation that requires a blob that isn’t currently
available locally, it will download the missing blob and add it to the
local object store.
Design
~~~~~~
Clone and fetch will pass a “--lazy-clone” flag (open to a better name
here) similar to “--depth” that instructs the server to only return
commits and trees and to ignore blobs.
Later during git operations like checkout, when a blob cannot be found
after checking all the regular places (loose, pack, alternates, etc),
git will download the missing object and place it into the local object
store (currently as a loose object) then resume the operation.
To prevent git from accidentally downloading all missing blobs, some git
operations are updated to be aware of the potential for missing blobs.
The most obvious being check_connected which will return success as if
everything in the requested commits is available locally.
To minimize the impact on the server, the existing dumb HTTP protocol
endpoint “objects/<sha>” can be used to retrieve the individual missing
blobs when needed.
Performance considerations
~~~~~~~~~~~~~~~~~~~~~~~~~~
We found that downloading commits and trees on demand had a significant
negative performance impact. In addition, many git commands assume all
commits and trees are available locally so they quickly got pulled down
anyway. Even in very large repos the commits and trees are relatively
small so bringing them down with the initial commit and subsequent fetch
commands was reasonable.
After cloning, the developer can use sparse-checkout to limit the set of
files to the subset they need (typically only 1-10% in these large
repos). This allows the initial checkout to only download the set of
files actually needed to complete their task. At any point, the
sparse-checkout file can be updated to include additional files which
will be fetched transparently on demand.
Typical source files are relatively small so the overhead of connecting
and authenticating to the server for a single file at a time is
substantial. As a result, having a long running process that is started
with the first request and can cache connection information between
requests is a significant performance win.
Now some numbers
~~~~~~~~~~~~~~~~
One repo has 3+ million files at tip across 500K folders with 5-6K
active developers. They have done a lot of work to remove large files
from the repo so it is down to < 100GB.
Before changes: clone took hours to transfer the 87GB .pack + 119MB .idx
After changes: clone took 4 minutes to transfer 305MB .pack + 37MB .idx
After hydrating 35K files (the typical number any individual developer
needs to do their work), there was an additional 460 MB of loose files
downloaded.
Total savings: 86.24 GB * 6000 developers = 517 Terabytes saved!
We have another repo (3.1 M files, 618 GB at tip with no history with
3K+ active developers) where the savings are even greater.
Future Work
~~~~~~~~~~~
The current prototype calls a new hook proc in sha1_object_info_extended
and read_object, to download each missing blob. A better solution would
be to implement this via a long running process that is spawned on the
first download and listens for requests to download additional objects
until it terminates when the parent git operation exits (similar to the
recent long running smudge and clean filter work).
Need to do more investigation into possible code paths that can trigger
unnecessary blobs to be downloaded. For example, we have determined
that the rename detection logic in status can also trigger unnecessary
blobs to be downloaded making status slow.
Need to investigate an alternate batching scheme where we can make a
single request for a set of "related" blobs and receive single a
packfile (especially during checkout).
Need to investigate adding a new endpoint in the smart protocol that can
download both individual blobs as well as a batch of blobs.
^ permalink raw reply
* [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context
From: Vegard Nossum @ 2017-01-13 16:15 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Vegard Nossum, René Scharfe
In-Reply-To: <1484324112-17773-1-git-send-email-vegard.nossum@oracle.com>
When using -W to include the whole function in the diff context, you
are typically doing this to be able to review the change in its entirety
within the context of the function. It is therefore almost always
desirable to include any comments that immediately precede the function.
This also the fixes the case for C where the declaration is split across
multiple lines (where the first line of the declaration would not be
included in the output), e.g.:
void
dummy(void)
{
...
}
We can include these lines by simply scanning upwards from the place of
the detected function start until we hit the first non-blank line.
Cc: René Scharfe <l.s.r@web.de>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
xdiff/xemit.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 8c88dbd..3a3060d 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -200,6 +200,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
}
fs1 = get_func_line(xe, xecfg, NULL, i1, -1);
+ while (fs1 > 0 && !is_empty_rec(&xe->xdf1, fs1 - 1))
+ fs1--;
if (fs1 < 0)
fs1 = 0;
if (fs1 < s1) {
@@ -220,6 +222,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
long fe1 = get_func_line(xe, xecfg, NULL,
xche->i1 + xche->chg1,
xe->xdf1.nrec);
+ while (fe1 > 0 && !is_empty_rec(&xe->xdf1, fe1 - 1))
+ fe1--;
while (fe1 > 0 && is_empty_rec(&xe->xdf1, fe1 - 1))
fe1--;
if (fe1 < 0)
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] xdiff: -W: relax end-of-file function detection
From: Vegard Nossum @ 2017-01-13 16:15 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Vegard Nossum, René Scharfe
When adding a new function to the end of a file, it's enough to know
that 1) the addition is at the end of the file; and 2) there is a
function _somewhere_ in there.
If we had simply been changing the end of an existing function, then we
would also be deleting something from the old version.
This fixes the case where we add e.g.
// Begin of dummy
static int dummy(void)
{
}
to the end of the file.
Cc: René Scharfe <l.s.r@web.de>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
xdiff/xemit.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 7389ce4..8c88dbd 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -183,16 +183,14 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
/*
* We don't need additional context if
- * a whole function was added, possibly
- * starting with empty lines.
+ * a whole function was added.
*/
- while (i2 < xe->xdf2.nrec &&
- is_empty_rec(&xe->xdf2, i2))
+ while (i2 < xe->xdf2.nrec) {
+ if (match_func_rec(&xe->xdf2, xecfg, i2,
+ dummy, sizeof(dummy)) >= 0)
+ goto post_context_calculation;
i2++;
- if (i2 < xe->xdf2.nrec &&
- match_func_rec(&xe->xdf2, xecfg, i2,
- dummy, sizeof(dummy)) >= 0)
- goto post_context_calculation;
+ }
/*
* Otherwise get more context from the
--
2.7.4
^ 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