* Re: [msysGit] [PATCH] mingw: work around stat-limitation
From: Johannes Schindelin @ 2012-02-23 22:37 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, gitster, j6t, msysgit
In-Reply-To: <CABPQNSZ+=no5PKkaa8_44FbOzh_W4A5knoxKgDQ2wTvOaeKdew@mail.gmail.com>
Hi kusma,
On Thu, 23 Feb 2012, Erik Faye-Lund wrote:
> On Thu, Feb 23, 2012 at 10:26 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 23 Feb 2012, Erik Faye-Lund wrote:
> >
> >> Our stat implementation for Windows always sets st_ino to 0. This
> >> means that checking if isatty(0) and comparing the reported inodes
> >> of stdout and stdin is not sufficient to detect that both are
> >> pointing to the same TTY.
> >>
> >> Luckily, there's only one console on Windows, so adding a check for
> >> isatty(1) should do the trick. For platforms where inodes are
> >> reported correctly, this should still be correct.
> >
> > Sorry to ask so stupidly, but why does isatty(1) work and isatty(0) does
> > not? Should they not access the very same console object?
> >
>
> The point is that they might not. The old test would draw the wrong
> conclusion if isatty(0) was true, but isatty(1) was not because it
> used st_ino to verify that stdin and stdout pointed to the same
> terminal. The reason it did that was to catch cases where stdin and
> stdout pointed to different terminals, AFAICT. Not checking isatty(1)
> was simply an optimization, which works when st_ino is filled out
> correctly.
>
> On Windows there is only one terminal, so it's sufficient to check if
> both are connected.
Ah, okay. Thanks for answering so patiently!
ACK.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-svn.perl: fix a false-positive in the "already exists" test
From: Steven Walter @ 2012-02-23 23:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: normalperson, git
In-Reply-To: <7vmx8bcv4u.fsf@alter.siamese.dyndns.org>
Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
On Wed, Feb 22, 2012 at 12:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Steven Walter <stevenrwalter@gmail.com> writes:
>
>>>> + test -e "$SVN_TREE"/bar/zzz/yyy ' || true
>>>
>>> Care to explain what this " || true" is doing here, please?
>>
>> Ahh, good catch. I think the answer is that it shouldn't be there.
>> It was originally there because of the "test_must_fail" line, I think
>> (at least the other tests that use test_must_fail also have "||
>> true").
>
> Ok, that may explain the copy&paste error.
>
> But I do not think test_must_fail followed by || true makes much sense,
> either. The purpose of "test_must_fail" is to make sure the tested git
> command exits with non-zero status in a controlled way (i.e. not crash)
> so if the tested command that is expected to exit with non-zero status
> exited with zero status, the test has detected an *error*. E.g. if you
> know that the index and the working tree are different at one point in the
> test sequence, you would say:
>
> ... other setup steps ... &&
> test_must_fail git diff --exit-code &&
> ... and other tests ...
>
> so that failure by "git diff --exit-code" to exit with non-zero status
> (i.e. it did not find any difference when it should have) breaks the &&
> cascade.
>
> I just took a quick look at t9100 but I think all " || true" can be safely
> removed. None of them is associated with test_must_fail in any way. For
> whatever reason, these test seem to do
>
> test_expect_success 'label of the test' '
> body of the test
> ' || true
>
> for no good reason.
>
>> Do you want to just fix that up, or a new version of the original patch,
>> or a fix on top of the original patches?
>
> Eric queued the patch and then had me pull it as part of his history
> already, so it is doubly too late to replace it.
>
> Can you apply this patch and re-test?
>
>
> t/t9100-git-svn-basic.sh | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
> index 4029f84..749b75e 100755
> --- a/t/t9100-git-svn-basic.sh
> +++ b/t/t9100-git-svn-basic.sh
> @@ -65,7 +65,8 @@ test_expect_success "$name" "
> git update-index --add dir/file/file &&
> git commit -m '$name' &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch" || true
> + ${remotes_git_svn}..mybranch
> +"
>
>
> name='detect node change from directory to file #1'
> @@ -79,7 +80,8 @@ test_expect_success "$name" '
> git update-index --add -- bar &&
> git commit -m "$name" &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch2' || true
> + ${remotes_git_svn}..mybranch2
> +'
>
>
> name='detect node change from file to directory #2'
> @@ -96,7 +98,8 @@ test_expect_success "$name" '
> ${remotes_git_svn}..mybranch3 &&
> svn_cmd up "$SVN_TREE" &&
> test -d "$SVN_TREE"/bar/zzz &&
> - test -e "$SVN_TREE"/bar/zzz/yyy ' || true
> + test -e "$SVN_TREE"/bar/zzz/yyy
> +'
>
> name='detect node change from directory to file #2'
> test_expect_success "$name" '
> @@ -109,7 +112,8 @@ test_expect_success "$name" '
> git update-index --add -- dir &&
> git commit -m "$name" &&
> test_must_fail git svn set-tree --find-copies-harder --rmdir \
> - ${remotes_git_svn}..mybranch4' || true
> + ${remotes_git_svn}..mybranch4
> +'
>
>
> name='remove executable bit from a file'
> @@ -162,7 +166,7 @@ test_expect_success "$name" '
>
> name='modify a symlink to become a file'
> test_expect_success "$name" '
> - echo git help > help || true &&
> + echo git help >help &&
> rm exec-2.sh &&
> cp help exec-2.sh &&
> git update-index exec-2.sh &&
--
-Steven Walter <stevenrwalter@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 3/4] t5704: match tests to modern style
From: Junio C Hamano @ 2012-02-23 23:36 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Sixt, Jeff King, Jannis Pohlmann, git
In-Reply-To: <c6579adc648119fbd1c54e2a1a9a0de86e0e8b57.1329988335.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Fix all of them. There's a catch to the last point: test_commit
> creates a tag. We still change it to test_commit, and explicitly
> delete the tags, so as to highlight that the test relies on not having
> them.
I do not have an objection to the use of these three test_commits, and I
do not have an objection to delete the extra three tags they create so
that the set of refs in the repository matches what the later test
expects, either.
But I found the explanation a bit iffy.
> test_expect_success 'setup' '
> + test_commit initial &&
> test_tick &&
> git tag -m tag tag &&
> + test_commit second &&
> + test_commit third &&
> + git tag -d initial &&
> + git tag -d second &&
> + git tag -d third
> '
>
> test_expect_success 'tags can be excluded by rev-list options' '
> git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
> git ls-remote bundle > output &&
> ! grep tag output
> '
If you do not delete 'third' tag, which matches the tip of the current
branch, the resulting bundle created with "create --all" would end up
containing that tag, and you will see it in the ls-remote output.
But the funny thing is that you can leave initial and second in the
repository and the resulting bundle still passes the test. 'initial',
'second' and 'tag' are excluded. Exclusion of 'tag' tag (sheesh, it makes
this conversation more confusing than necessary) is what this test checks,
and the date range given to the rev-list ensures that initial and second
are not included.
But the tip of the current branch and the lightweight 'third' tag both
point at the same commit object, and that, together with the fact that we
ask for '--all', is the reason why it is included in the result, making
the test fail.
So perhaps a better fix may be to do something like the attached on top,
and rewrite everything after "Fix all of them".
... Fix all of them.
Also rename the manually created tag 'tag' that points at a commit
that is older than the --since threshold a later test uses, to a
more descriptive 'ancienttag', and update the check that reads
from the resulting bundle with ls-remote to look for 'ancienttag'.
The purpose of this test is to make sure that the tag that
predates the date range is not in the resulting bundle, but
because these test_commit also will create tags, one of which
(namely, 'third') points at a commit that is newer than that
threshold, ls-remote will list it in its output. Looking for 'tag'
will match refs/tags/third, making the test incorrectly fail.
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index a51c8b0..3c436e7 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -6,18 +6,15 @@ test_description='some bundle related tests'
test_expect_success 'setup' '
test_commit initial &&
test_tick &&
- git tag -m tag tag &&
+ git tag -m tag antienttag &&
test_commit second &&
- test_commit third &&
- git tag -d initial &&
- git tag -d second &&
- git tag -d third
+ test_commit third
'
test_expect_success 'tags can be excluded by rev-list options' '
git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
git ls-remote bundle > output &&
- ! grep tag output
+ ! grep antienttag output
'
test_expect_success 'die if bundle file cannot be created' '
^ permalink raw reply related
* git gui: how to fetch a single branch
From: Matt Seitz (matseitz) @ 2012-02-23 23:58 UTC (permalink / raw)
To: git
How can I use "git gui" to fetch a single branch from a remote
repository?
If I select Remote->Fetch From, and then a remote repository, it appears
to always fetch all the branches from the remote repository with no
option to select which branch I want to fetch.
^ permalink raw reply
* [PATCH v3] contrib: added git-diffall
From: Tim Henigan @ 2012-02-24 0:49 UTC (permalink / raw)
To: git, gitster; +Cc: davvid, stefano.lattarini, tim.henigan
The 'git difftool' command allows the user to view diffs using an
external tool. It runs a separate instance of the tool for each
file in the diff. This makes it tedious to review changes spanning
multiple files.
The 'git-diffall' script instead prepares temporary directories
with the files to be compared and launches a single instance of
the external diff tool to view them (i.e. a directory diff).
The 'diff.tool' or 'merge.tool' configuration variable is used
to specify which external tool is used.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
This script has been hosted on GitHub [1] since April 2010. Enough people
have found it useful that I hope it will be considered for inclusion in
the standard git install, either in contrib or as a new core command.
Changes in v3:
- Fixed a bug that caused failures if file names included spaces
- Added unique suffix to tmp dir name (tmp/git-diffall-tmp.$$)
- Renamed "common_ancestor" to "merge_base"
- Cleaned up README to be more accurate
- Added useful error message if --extcmd is final option, but no
command was specified
- Removed spaces after redirection operators
v3 matches commit f36e4881e5 on GitHub [1].
Changes in v2:
- Changed to #!/bin/sh
- Eliminated use of 'which' statements
- Fixed trap function to actually run on abnormal exit
- Simplified path concatenation logic ($IFS)
- Corrected indentation errors
- Improved readability of while loop
- Cleaned up quoting of variables
v2 matches commit 5d4b90de3 on GitHub [1].
[1]: https://github.com/thenigan/git-diffall
contrib/diffall/README | 24 ++++
contrib/diffall/git-diffall | 261 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 285 insertions(+)
create mode 100644 contrib/diffall/README
create mode 100755 contrib/diffall/git-diffall
diff --git a/contrib/diffall/README b/contrib/diffall/README
new file mode 100644
index 0000000..111f3f6
--- /dev/null
+++ b/contrib/diffall/README
@@ -0,0 +1,24 @@
+The git-diffall script provides a directory based diff mechanism
+for git. The script relies on the diff.tool configuration option
+to determine what diff viewer is used.
+
+This script is compatible with most common forms used to specify a
+range of revisions to diff:
+
+ 1. git diffall: shows diff between working tree and staged changes
+ 2. git diffall --cached [<commit>]: shows diff between staged
+ changes and HEAD (or other named commit)
+ 3. git diffall <commit>: shows diff between working tree and named
+ commit
+ 4. git diffall <commit> <commit>: show diff between two named commit
+ 5. git diffall <commit>..<commit>: same as above
+ 6. git diffall <commit>...<commit>: show the changes on the branch
+ containing and up to the second , starting at a common ancestor
+ of both <commit>
+
+Note: all forms take an optional path limiter [--] [<path>*]
+
+This script is based on an example provided by Thomas Rast on the
+Git list [1]:
+
+[1] http://thread.gmane.org/gmane.comp.version-control.git/124807
diff --git a/contrib/diffall/git-diffall b/contrib/diffall/git-diffall
new file mode 100755
index 0000000..e00fe89
--- /dev/null
+++ b/contrib/diffall/git-diffall
@@ -0,0 +1,261 @@
+#!/bin/sh
+# Copyright 2010 - 2012, Tim Henigan <tim.henigan@gmail.com>
+#
+# Perform a directory diff between commits in the repository using
+# the external diff or merge tool specified in the user's config.
+
+USAGE='[--cached] [--copy-back] [-x|--extcmd=<command>] <commit>{0,2} -- <path>*
+
+ --cached Compare to the index rather than the working tree.
+
+ --copy-back Copy files back to the working tree when the diff
+ tool exits (in case they were modified by the
+ user). This option is only valid if the diff
+ compared with the working tree.
+
+ -x=<command>
+ --extcmd=<command> Specify a custom command for viewing diffs.
+ git-diffall ignores the configured defaults and
+ runs $command $LOCAL $REMOTE when this option is
+ specified. Additionally, $BASE is set in the
+ environment.
+'
+
+SUBDIRECTORY_OK=1
+. "$(git --exec-path)/git-sh-setup"
+
+TOOL_MODE=diff
+. "$(git --exec-path)/git-mergetool--lib"
+
+merge_tool="$(get_merge_tool)"
+if test -z "$merge_tool"
+then
+ echo "Error: Either the 'diff.tool' or 'merge.tool' option must be set."
+ usage
+fi
+
+start_dir=$(pwd)
+
+# needed to access tar utility
+cdup=$(git rev-parse --show-cdup) &&
+cd "$cdup" || {
+ echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
+ exit 1
+}
+
+# mktemp is not available on all platforms (missing from msysgit)
+# Use a hard-coded tmp dir if it is not available
+tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
+ tmp=/git-diffall-tmp.$$
+ mkdir "$tmp" || exit 1
+}
+
+trap 'rm -rf "$tmp" 2>/dev/null' EXIT
+
+left=
+right=
+paths=
+path_sep=
+compare_staged=
+merge_base=
+left_dir=
+right_dir=
+diff_tool=
+copy_back=
+
+while test $# != 0
+do
+ case "$1" in
+ -h|--h|--he|--hel|--help)
+ usage
+ ;;
+ --cached)
+ compare_staged=1
+ ;;
+ --copy-back)
+ copy_back=1
+ ;;
+ -x|--e|--ex|--ext|--extc|--extcm|--extcmd)
+ if test -z $2
+ then
+ echo You must specify the tool for use with --extcmd
+ usage
+ else
+ diff_tool=$2
+ shift
+ fi
+ ;;
+ --)
+ path_sep=1
+ ;;
+ -*)
+ echo Invalid option: "$1"
+ usage
+ ;;
+ *)
+ # could be commit, commit range or path limiter
+ case "$1" in
+ *...*)
+ left=${1%...*}
+ right=${1#*...}
+ merge_base=1
+ ;;
+ *..*)
+ left=${1%..*}
+ right=${1#*..}
+ ;;
+ *)
+ if test -n "$path_sep"
+ then
+ paths="$paths$1 "
+ elif test -z "$left"
+ then
+ left=$1
+ elif test -z "$right"
+ then
+ right=$1
+ else
+ paths="$paths$1 "
+ fi
+ ;;
+ esac
+ ;;
+ esac
+ shift
+done
+
+# Determine the set of files which changed
+if test -n "$left" && test -n "$right"
+then
+ left_dir="cmt-$(git rev-parse --short $left)"
+ right_dir="cmt-$(git rev-parse --short $right)"
+
+ if test -n "$compare_staged"
+ then
+ usage
+ elif test -n "$merge_base"
+ then
+ git diff --name-only "$left"..."$right" -- $paths >"$tmp/filelist"
+ else
+ git diff --name-only "$left" "$right" -- $paths >"$tmp/filelist"
+ fi
+elif test -n "$left"
+then
+ left_dir="cmt-$(git rev-parse --short $left)"
+
+ if test -n "$compare_staged"
+ then
+ right_dir="staged"
+ git diff --name-only --cached "$left" -- $paths >"$tmp/filelist"
+ else
+ right_dir="working_tree"
+ git diff --name-only "$left" -- $paths >"$tmp/filelist"
+ fi
+else
+ left_dir="HEAD"
+
+ if test -n "$compare_staged"
+ then
+ right_dir="staged"
+ git diff --name-only --cached -- $paths >"$tmp/filelist"
+ else
+ right_dir="working_tree"
+ git diff --name-only -- $paths >"$tmp/filelist"
+ fi
+fi
+
+# Exit immediately if there are no diffs
+if test ! -s "$tmp/filelist"
+then
+ exit 0
+fi
+
+if test -n "$copy_back" && test "$right_dir" != "working_tree"
+then
+ echo "--copy-back is only valid when diff includes the working tree."
+ exit 1
+fi
+
+# Create the named tmp directories that will hold the files to be compared
+mkdir -p "$tmp/$left_dir" "$tmp/$right_dir"
+
+# Populate the tmp/right_dir directory with the files to be compared
+if test -n "$right"
+then
+ while read name
+ do
+ ls_list=$(git ls-tree $right "$name")
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp/$right_dir/$(dirname "$name")"
+ git show "$right":"$name" >"$tmp/$right_dir/$name" || true
+ fi
+ done < "$tmp/filelist"
+elif test -n "$compare_staged"
+then
+ while read name
+ do
+ ls_list=$(git ls-files -- "$name")
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp/$right_dir/$(dirname "$name")"
+ git show :"$name" >"$tmp/$right_dir/$name"
+ fi
+ done < "$tmp/filelist"
+else
+ # Mac users have gnutar rather than tar
+ (tar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && tar -x)) || {
+ gnutar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && gnutar -x)
+ }
+fi
+
+# Populate the tmp/left_dir directory with the files to be compared
+while read name
+do
+ if test -n "$left"
+ then
+ ls_list=$(git ls-tree $left "$name")
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp/$left_dir/$(dirname "$name")"
+ git show "$left":"$name" >"$tmp/$left_dir/$name" || true
+ fi
+ else
+ if test -n "$compare_staged"
+ then
+ ls_list=$(git ls-tree HEAD "$name")
+ if test -n "$ls_list"
+ then
+ mkdir -p "$tmp/$left_dir/$(dirname "$name")"
+ git show HEAD:"$name" >"$tmp/$left_dir/$name"
+ fi
+ else
+ mkdir -p "$tmp/$left_dir/$(dirname "$name")"
+ git show :"$name" >"$tmp/$left_dir/$name"
+ fi
+ fi
+done < "$tmp/filelist"
+
+cd "$tmp"
+LOCAL="$left_dir"
+REMOTE="$right_dir"
+
+if test -n "$diff_tool"
+then
+ export BASE
+ eval $diff_tool '"$LOCAL"' '"$REMOTE"'
+else
+ run_merge_tool "$merge_tool" false
+fi
+
+# Copy files back to the working dir, if requested
+if test -n "$copy_back" && test "$right_dir" = "working_tree"
+then
+ cd "$start_dir"
+ git_top_dir=$(git rev-parse --show-toplevel)
+ find "$tmp/$right_dir" -type f |
+ while read file
+ do
+ cp "$file" "$git_top_dir/${file#$tmp/$right_dir/}"
+ done
+fi
--
1.7.9.GIT
^ permalink raw reply related
* Re: git-subtree Ready #2
From: Avery Pennarun @ 2012-02-24 1:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, David A. Greene, git
In-Reply-To: <7vd399jdwc.fsf@alter.siamese.dyndns.org>
On Mon, Feb 20, 2012 at 6:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
> It sounds like the simplest and cleanest would be to treat it as if its
> current version came as a patch submission, cook it just like any other
> topic in 'pu' down to 'next' down to eventually 'master', with the usual
> review cycle of pointing out what is wrong and needs fixing followed by a
> series of re-rolls.
Yeah, my original intent with git-subtree was to one day submit it as
basically a single patch against git. That's why I have some slightly
suspicious commit messages in there (though in my defense, I think
*most* of the commit messages are quite sensible :)).
> After looking at the history of subtree branch there, however, I agree
> that it would not help anybody to have its history in my tree with log
> messages like these (excerpt from shortlog output):
> [...]
> Docs: when pushing to github, the repo path needs to end in .git
> [...]
That commit message in particular I thought was perfectly fine; it's
specifically a fix to the git-subtree docs to clarify a question from
an actual user.
Overall I agree that there's little benefit in preserving the history,
at least as far as I can see, *except* that some code changes were
submitted by people other than me and squashing those changes might
conceivably cause licensing confusion down the road. It's probably a
fairly quick exercise with git-filter-branch to get rid of the more
egregious commit message problems, if that's what we want to do. (In
particular, just expurgating the entire 'todo' file from the history
probably makes plenty of sense.)
There's no value I can see in being able to do future merges from
outside the tree, so a filter-branch or rebase before merging should
be pretty harmless.
> The total amount of change does not look too bad, either:
>
> $ git diff --stat master...origin/subtree
> contrib/subtree/.gitignore | 5 +
> contrib/subtree/COPYING | 339 +++++++++++++++++
> contrib/subtree/Makefile | 45 +++
> contrib/subtree/README | 8 +
> contrib/subtree/git-subtree.sh | 712 ++++++++++++++++++++++++++++++++++++
> contrib/subtree/git-subtree.txt | 366 ++++++++++++++++++
> contrib/subtree/t/Makefile | 71 ++++
> contrib/subtree/t/t7900-subtree.sh | 508 +++++++++++++++++++++++++
> contrib/subtree/todo | 50 +++
> t/test-lib.sh | 11 +-
> 10 files changed, 2114 insertions(+), 1 deletion(-)
Note that COPYING, .gitignore, Makefile, t/Makefile, todo, and README
should probably be ditched if it weren't going into contrib. The
interesting files are git-subtree.{sh,txt} and t7900-subtree.sh.
> I haven't looked at the script fully, but it has an issue
> from its first line, which is marked with "#!/bin/bash". It is unclear if
> it is infested by bash-isms beyond repair (in which case "#!/bin/bash" is
> fine), or it was written portably but was marked with "#!/bin/bash" just
> by inertia.
I'm generally pretty careful to avoid bashisms, but since my /bin/sh
is bash, I usually mark scripts with /bin/bash just to be safe until
someone has actually verified them with a non-bash shell. I expect
few if any problems with that part.
> A patch that corresponds to the above diffstat immediately
> shows many style issues including trailing eye-sore whitespaces.
>
> It seems that it is even capable of installing from contrib/subtree, so
> keeping it in contrib/ while many issues it may have gets fixed would not
> hurt the original goal of giving the script more visibility.
Personally, I would prefer to just iterate the patch a few times to
correct the coding style problems you see, rather than merging into
contrib where it might be forgotten rather than fixed. As Peff
alluded, people who want to install it separately from git already
can; if we're going to merge it into git, let's do it right.
Have fun,
AVery
^ permalink raw reply
* [PATCH] index-pack: remove real_type from struct object_entry
From: Nguyễn Thái Ngọc Duy @ 2012-02-24 2:42 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Since ce3f6dc (fix multiple issues in index-pack), real_type is
identical to type, there's no reason to keep it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I'm pretty sure I read it right (i.e. type == real_type), but I may
have overlooked something. Not so sure why real_type was introduced
in the first place..
builtin/index-pack.c | 16 ++++++----------
1 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index dd1c5c9..2db9a35 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -18,7 +18,6 @@ struct object_entry {
unsigned long size;
unsigned int hdr_size;
enum object_type type;
- enum object_type real_type;
unsigned delta_depth;
int base_object_no;
};
@@ -581,7 +580,6 @@ static void resolve_delta(struct object_entry *delta_obj,
{
void *base_data, *delta_data;
- delta_obj->real_type = base->obj->real_type;
delta_obj->delta_depth = base->obj->delta_depth + 1;
if (deepest_delta < delta_obj->delta_depth)
deepest_delta = delta_obj->delta_depth;
@@ -594,7 +592,7 @@ static void resolve_delta(struct object_entry *delta_obj,
free(delta_data);
if (!result->data)
bad_object(delta_obj->idx.offset, "failed to apply delta");
- sha1_object(result->data, result->size, delta_obj->real_type,
+ sha1_object(result->data, result->size, delta_obj->type,
delta_obj->idx.sha1);
nr_resolved_deltas++;
}
@@ -626,7 +624,7 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
struct object_entry *child = objects + deltas[base->ref_first].obj_no;
struct base_data *result = alloc_base_data();
- assert(child->real_type == OBJ_REF_DELTA);
+ assert(child->type == OBJ_REF_DELTA);
resolve_delta(child, base, result);
if (base->ref_first == base->ref_last && base->ofs_last == -1)
free_base_data(base);
@@ -639,7 +637,7 @@ static struct base_data *find_unresolved_deltas_1(struct base_data *base,
struct object_entry *child = objects + deltas[base->ofs_first].obj_no;
struct base_data *result = alloc_base_data();
- assert(child->real_type == OBJ_OFS_DELTA);
+ assert(child->type == OBJ_OFS_DELTA);
resolve_delta(child, base, result);
if (base->ofs_first == base->ofs_last)
free_base_data(base);
@@ -702,7 +700,6 @@ static void parse_pack_objects(unsigned char *sha1)
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = &objects[i];
void *data = unpack_raw_entry(obj, &delta->base);
- obj->real_type = obj->type;
if (is_delta_type(obj->type)) {
nr_deltas++;
delta->obj_no = i;
@@ -805,7 +802,6 @@ static struct object_entry *append_obj_to_pack(struct sha1file *f,
obj[0].size = size;
obj[0].hdr_size = n;
obj[0].type = type;
- obj[0].real_type = type;
obj[1].idx.offset = obj[0].idx.offset + n;
obj[1].idx.offset += write_compressed(f, buf, size);
obj[0].idx.crc32 = crc32_end(f);
@@ -838,7 +834,7 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
*/
sorted_by_pos = xmalloc(nr_unresolved * sizeof(*sorted_by_pos));
for (i = 0; i < nr_deltas; i++) {
- if (objects[deltas[i].obj_no].real_type != OBJ_REF_DELTA)
+ if (objects[deltas[i].obj_no].type != OBJ_REF_DELTA)
continue;
sorted_by_pos[n++] = &deltas[i];
}
@@ -849,7 +845,7 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
enum object_type type;
struct base_data *base_obj = alloc_base_data();
- if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
+ if (objects[d->obj_no].type != OBJ_REF_DELTA)
continue;
base_obj->data = read_sha1_file(d->base.sha1, &type, &base_obj->size);
if (!base_obj->data)
@@ -1053,7 +1049,7 @@ static void show_pack_info(int stat_only)
continue;
printf("%s %-6s %lu %lu %"PRIuMAX,
sha1_to_hex(obj->idx.sha1),
- typename(obj->real_type), obj->size,
+ typename(obj->type), obj->size,
(unsigned long)(obj[1].idx.offset - obj->idx.offset),
(uintmax_t)obj->idx.offset);
if (is_delta_type(obj->type)) {
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [BUG?] bulk checkin does not respect filters
From: Jeff King @ 2012-02-24 3:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If I do this:
git init repo &&
cd repo &&
echo foo >small &&
cat small small small small >large &&
echo '* filter=foo2bar' >.gitattributes &&
git config filter.foo2bar.clean 'sed s/foo/bar/' &&
git config core.bigfilethreshold 10 &&
git add . &&
echo "===> small" && git cat-file blob :small
echo "===> large" && git cat-file blob :large
the output I get is:
===> small
bar
===> large
foo
foo
foo
foo
I.e., the clean filter is not applied to the bulk checkin file. Nor can
it be easily, because we need to know the size of the file to write the
blob header, and we don't know that until we see all of the filter's
output.
In practice, I don't know if this is a huge deal, as people aren't going
to be asking to de-CRLF files that actually cross the 512M
bigfilethreshold (OTOH, I seem to recall there are some filters floating
around for normalizing gzip'd files, which could plausibly be gigantic).
But it seems like the right choice when we see this conflict is not
"don't do filters for streaming checkin", but rather "don't do streaming
checkin when filters are in use" (because streaming is an optimization,
and filters are about correctness).
It would be even nicer if filters could play well with bulk checkin, but
I think that would involve streaming to a tempfile, checking the size of
the file, and then streaming that into an object. Which is better than
putting the whole thing in memory if it would involve swapping, but
probably worse than doing so if you can possibly fit the whole thing in
(because you're doing a ton of extra I/O for the tempfile).
Thoughts? Was this intentional, or just overlooked?
-Peff
^ permalink raw reply
* Re: [BUG?] bulk checkin does not respect filters
From: Junio C Hamano @ 2012-02-24 3:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120224030244.GA15742@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Thoughts? Was this intentional, or just overlooked?
This is intentional in the sense it is not worth worrying about (I
personally consider large and binary gunk equivalent and something nobody
should care about anything more than 1. what the exact contents it had at
each point in history and 2. if it did or did not change between two
versions, but definitely not 3. how different these two versions were),
and does not deserve the complexity to support filtering and textconv'ing.
If somebody strongly feels about lifting the limitation with a clean patch
that does not harm the common case codepaths, patches are welcome, but I
am not planning to do it myself ;-)
It is worth documenting this limitation, though.
^ permalink raw reply
* Re: [BUG?] bulk checkin does not respect filters
From: Junio C Hamano @ 2012-02-24 3:42 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <7vvcmw2a3m.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> Thoughts? Was this intentional, or just overlooked?
It is a bit curious that anything filtered even goes to the streaming
codepath, given this piece of code in write_entry() in entry.c:
if (ce_mode_s_ifmt == S_IFREG) {
struct stream_filter *filter = get_stream_filter(path, ce->sha1);
if (filter &&
!streaming_write_entry(ce, path, filter,
state, to_tempfile,
&fstat_done, &st))
goto finish;
}
and get_stream_filter() in convert.c has an explicit exception for this
case at the very beginning:
struct stream_filter *get_stream_filter(const char *path, const unsigned char *sha1)
{
struct conv_attrs ca;
enum crlf_action crlf_action;
struct stream_filter *filter = NULL;
convert_attrs(&ca, path);
if (ca.drv && (ca.drv->smudge || ca.drv->clean))
return filter;
to make sure that it says "No streaming filtering is possible, do not even
attempt to call streaming_write_entry()".
^ permalink raw reply
* Re: [PATCH] index-pack: remove real_type from struct object_entry
From: Nguyen Thai Ngoc Duy @ 2012-02-24 6:08 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1330051320-19043-1-git-send-email-pclouds@gmail.com>
2012/2/24 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> @@ -581,7 +580,6 @@ static void resolve_delta(struct object_entry *delta_obj,
> {
> void *base_data, *delta_data;
>
> - delta_obj->real_type = base->obj->real_type;
> delta_obj->delta_depth = base->obj->delta_depth + 1;
> if (deepest_delta < delta_obj->delta_depth)
> deepest_delta = delta_obj->delta_depth;
This is wrong. Sorry for the noise.
--
Duy
^ permalink raw reply
* Re: [BUG?] bulk checkin does not respect filters
From: Jeff King @ 2012-02-24 7:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4xk28z0.fsf@alter.siamese.dyndns.org>
On Thu, Feb 23, 2012 at 07:42:11PM -0800, Junio C Hamano wrote:
> It is a bit curious that anything filtered even goes to the streaming
> codepath, given this piece of code in write_entry() in entry.c:
>
> if (ce_mode_s_ifmt == S_IFREG) {
> struct stream_filter *filter = get_stream_filter(path, ce->sha1);
> if (filter &&
> !streaming_write_entry(ce, path, filter,
> state, to_tempfile,
> &fstat_done, &st))
> goto finish;
> }
>
> and get_stream_filter() in convert.c has an explicit exception for this
> case at the very beginning:
I think it is because we don't follow that code path at all. The stack trace
for "git add" looks on a large file looks like:
#0 stream_to_pack (...) at bulk-checkin.c:101
#1 deflate_to_pack (...) at bulk-checkin.c:219
#2 index_bulk_checkin (...) at bulk-checkin.c:258
#3 index_stream (...) at sha1_file.c:2712
#4 index_fd (...) at sha1_file.c:2726
#5 index_path (...) at sha1_file.c:2742
#6 add_to_index (...) at read-cache.c:644
#7 add_file_to_index (...) at read-cache.c:673
#8 add_files (...) at builtin/add.c:363
#9 cmd_add (...) at builtin/add.c:474
#10 run_builtin (...) at git.c:324
#11 handle_internal_command (...) at git.c:484
#12 run_argv (...) at git.c:530
#13 main (...) at git.c:605
Isn't write_entry the _other_ side of things. I.e., checking out, not
checking in? That side works fine (making the large file handling
for git-add even more odd. We will fail to apply the "clean" filter when
adding it, but we will apply the smudge filter when we write it out).
-Peff
^ permalink raw reply
* Re: [BUG?] bulk checkin does not respect filters
From: Jeff King @ 2012-02-24 8:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcmw2a3m.fsf@alter.siamese.dyndns.org>
On Thu, Feb 23, 2012 at 07:17:49PM -0800, Junio C Hamano wrote:
> > Thoughts? Was this intentional, or just overlooked?
>
> This is intentional in the sense it is not worth worrying about (I
> personally consider large and binary gunk equivalent and something nobody
> should care about anything more than 1. what the exact contents it had at
> each point in history and 2. if it did or did not change between two
> versions, but definitely not 3. how different these two versions were),
> and does not deserve the complexity to support filtering and textconv'ing.
We're purely in the hypothetical devil's advocate realm here, as I do
not use the filtering feature myself. But what if I had a filter that
canonicalized large binary files in some way (e.g., by re-zipping large
zipfiles using deterministic parameters). Even though they are large
binary gunk and you would never want to diff two versions, the important
thing is getting an identical sha1 when the underlying data has not
changed. I.e., your point 2 above, but applied to the "clean" repository
versions of files.
> If somebody strongly feels about lifting the limitation with a clean patch
> that does not harm the common case codepaths, patches are welcome, but I
> am not planning to do it myself ;-)
Perhaps something like this:
---
convert.c | 18 ++++++++++++++++++
convert.h | 1 +
sha1_file.c | 3 ++-
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/convert.c b/convert.c
index 12868ed..254301a 100644
--- a/convert.c
+++ b/convert.c
@@ -742,6 +742,24 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
}
}
+int want_convert_to_git(const char *path)
+{
+ struct conv_attrs ca;
+ convert_attrs(&ca, path);
+
+ if (ca.drv && ca.drv->clean)
+ return 1;
+ if (ca.ident)
+ return 1;
+
+ ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
+ if (ca.crlf_action == CRLF_BINARY)
+ return 0;
+ if (ca.crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE)
+ return 0;
+ return 1;
+}
+
int convert_to_git(const char *path, const char *src, size_t len,
struct strbuf *dst, enum safe_crlf checksafe)
{
diff --git a/convert.h b/convert.h
index d799a16..4d5936d 100644
--- a/convert.h
+++ b/convert.h
@@ -36,6 +36,7 @@ extern enum eol core_eol;
/* returns 1 if *dst was used */
extern int convert_to_git(const char *path, const char *src, size_t len,
struct strbuf *dst, enum safe_crlf checksafe);
+extern int want_convert_to_git(const char *path);
extern int convert_to_working_tree(const char *path, const char *src,
size_t len, struct strbuf *dst);
extern int renormalize_buffer(const char *path, const char *src, size_t len,
diff --git a/sha1_file.c b/sha1_file.c
index f9f8d5e..6c0e05c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2720,7 +2720,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
if (!S_ISREG(st->st_mode))
ret = index_pipe(sha1, fd, type, path, flags);
- else if (size <= big_file_threshold || type != OBJ_BLOB)
+ else if (size <= big_file_threshold || type != OBJ_BLOB ||
+ want_convert_to_git(path))
ret = index_core(sha1, fd, size, type, path, flags);
else
ret = index_stream(sha1, fd, size, type, path, flags);
There should be no performance impact, as the new code only kicks in for
files that exceed big_file_threshold (and even then, it is just an extra
duplicate attr lookup, which the check_attr code caches anyway).
I don't like repeating all of the convert_to_git policy logic. Perhaps
if you pass a NULL buffer to convert_to_git, it should run through its
usual logic, stopping just short of actually writing anything, and
return a flag indicating whether it _would_ convert (this can't be 100%
accurate, as sometimes conversion depends on looking at the actual
contents of the buffer, but it could at least tell us "yes, I might
convert" versus "no, I will definitely not convert").
-Peff
^ permalink raw reply related
* Re: git log -z doesn't separate commits with NULs
From: Nikolaj Shurkaev @ 2012-02-24 9:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Jakub Narebski, git
In-Reply-To: <7vy5rt2u0c.fsf@alter.siamese.dyndns.org>
Hello.
Thank you for the hint.
git format-patch HEAD~3..HEAD -- SomePath
does exactly what I need. But that way of usage is not described in git
documentation thus I thought there is no way to do that. I've just
double checked
git format-patch --help
doesn't describe that. I'll propose to put something like this into git
documentation
--------------------------------------------------------------------------------------------
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 6ea9be7..63267c6 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -23,6 +23,7 @@ SYNOPSIS
[--cover-letter] [--quiet]
[<common diff options>]
[ <since> | <revision range> ]
+ [[\--] <path>...]
DESCRIPTION
-----------
@@ -219,6 +220,12 @@ you can use `--suffix=-patch` to get
`0001-description-of-my-change-patch`.
range are always formatted as creation patches, independently
of this flag.
+[\--] <path>...::
+ Put in patches only those modifications that affect specified files
+ and folders. It's important to understand that log message of the
+ commit may become inappropriate because some parts of patch may be
+ cut off.
+
CONFIGURATION
-------------
You can specify extra mail header lines to be added to each message,
--------------------------------------------------------------------------------------------
--
Nikolaj.
23.02.2012 23:07, Junio C Hamano пишет:
> Jeff King<peff@peff.net> writes:
>
>> On Thu, Feb 23, 2012 at 04:48:43PM +0300, Nikolaj Shurkaev wrote:
>>
>> Doesn't:
>>
>> git format-patch HEAD~3..HEAD SomePath
>>
>> do what you want? It is certainly designed to, and it seems to work for
>> me.
> It is not quite "designed to", though.
>
> It happens to work that way, and I do not think we want to forbid its use,
> but we would want to discourage anybody from blindly using it without
> thinking if the end results suits his/her purpose (and the reason should
> be obvious to those who think, the hint is "log message").
>
^ permalink raw reply related
* Re: [BUG?] bulk checkin does not respect filters
From: Jeff King @ 2012-02-24 9:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120224082803.GB18688@sigill.intra.peff.net>
On Fri, Feb 24, 2012 at 03:28:03AM -0500, Jeff King wrote:
> I don't like repeating all of the convert_to_git policy logic. Perhaps
> if you pass a NULL buffer to convert_to_git, it should run through its
> usual logic, stopping just short of actually writing anything, and
> return a flag indicating whether it _would_ convert...
So here's a series that does that.
[1/2]: teach convert_to_git a "dry run" mode
[2/2]: do not stream large files to pack when filters are in use
-Peff
^ permalink raw reply
* [PATCH 1/2] teach convert_to_git a "dry run" mode
From: Jeff King @ 2012-02-24 9:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120224093924.GA11680@sigill.intra.peff.net>
Some callers may want to know whether convert_to_git will do
any conversion without actually feeding file data to it
(e.g., if you are using the decision to decide how to
acquire the data that might be converted). Rather than
replicate convert_to_git's logic in a separate function,
this patch lets callers pass a NULL buffer. The function
runs through its usual logic, except that it does not do any
actual conversion; the return value, instead of specifying
whether conversion happened, indicates whether conversion
might occur.
Note the use of the word "might" above. We cannot know for
sure whether conversion will occur without seeing the buffer
itself (because CRLF conversion may decide the file is
binary and do nothing). However, "might" is close enough for
callers which are trying to find out whether filters are in
used; they can be conservative and assume that the filters
will be used in such a case.
Signed-off-by: Jeff King <peff@peff.net>
---
convert.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/convert.c b/convert.c
index 12868ed..33aa72f 100644
--- a/convert.c
+++ b/convert.c
@@ -195,9 +195,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
char *dst;
if (crlf_action == CRLF_BINARY ||
- (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len)
+ (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) ||
+ (src && !len))
return 0;
+ if (!src)
+ return 1;
+
gather_stats(src, len, &stats);
if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {
@@ -391,6 +395,9 @@ static int apply_filter(const char *path, const char *src, size_t len,
if (!cmd)
return 0;
+ if (!src)
+ return 1;
+
memset(&async, 0, sizeof(async));
async.proc = filter_buffer;
async.data = ¶ms;
@@ -522,9 +529,12 @@ static int ident_to_git(const char *path, const char *src, size_t len,
{
char *dst, *dollar;
- if (!ident || !count_ident(src, len))
+ if (!ident || (src && !count_ident(src, len)))
return 0;
+ if (!src)
+ return 1;
+
/* only grow if not in place */
if (strbuf_avail(buf) + buf->len < len)
strbuf_grow(buf, len - buf->len);
@@ -754,13 +764,13 @@ int convert_to_git(const char *path, const char *src, size_t len,
filter = ca.drv->clean;
ret |= apply_filter(path, src, len, dst, filter);
- if (ret) {
+ if (ret && src) {
src = dst->buf;
len = dst->len;
}
ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
- if (ret) {
+ if (ret && src) {
src = dst->buf;
len = dst->len;
}
--
1.7.9.9.g04d94
^ permalink raw reply related
* [PATCH 2/2] do not stream large files to pack when filters are in use
From: Jeff King @ 2012-02-24 9:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120224093924.GA11680@sigill.intra.peff.net>
Because git's object format requires us to specify the
number of bytes in the object in its header, we must know
the size before streaming a blob into the object database.
This is not a problem when adding a regular file, as we can
get the size from stat(). However, when filters are in use
(such as autocrlf, or the ident, filter, or eol
gitattributes), we have no idea what the ultimate size will
be.
The current code just punts on the whole issue and ignores
filter configuration entirely for files larger than
core.bigfilethreshold. This can generate confusing results
if you use filters for large binary files, as the filter
will suddenly stop working as the file goes over a certain
size. Rather than try to handle unknown input sizes with
streaming, this patch just turns off the streaming
optimization when filters are in use.
This has a slight performance regression in a very specific
case: if you have autocrlf on, but no gitattributes, a large
binary file will avoid the streaming code path because we
don't know beforehand whether it will need conversion or
not. But if you are handling large binary files, you should
be marking them as such via attributes (or at least not
using autocrlf, and instead marking your text files as
such). And the flip side is that if you have a large
_non_-binary file, there is a correctness improvement;
before we did not apply the conversion at all.
The first half of the new t1051 script covers these failures
on input. The second half tests the matching output code
paths. These already work correctly, and do not need any
adjustment.
Signed-off-by: Jeff King <peff@peff.net>
---
I'm on the fence about the performance regression above. On the one
hand, if you're telling git to autocrlf your gigantic binary files, you
should fix your attributes, and expect not to use the streaming
optimization until you do. On the other hand, while I can see a remote
possibility of using filter.*.clean on a large binary file, the
probability that autocrlf would apply seems quite absurdly unlikely.
So rather than be pessimistic that filters will be used in the "auto"
case, perhaps we should be optimistic that they would not be. Or another
way of thinking about it is: from the CRLF code's perspective, anything
larger than core.bigfilethreshold should be considered binary without
actually looking at it, and we can say for sure (not the "might" from
the last patch) that such large things would not be converted in the
CRLF_GUESS case.
sha1_file.c | 3 +-
t/t1051-large-conversion.sh | 86 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 1 deletion(-)
create mode 100755 t/t1051-large-conversion.sh
diff --git a/sha1_file.c b/sha1_file.c
index f9f8d5e..61f597b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2720,7 +2720,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
if (!S_ISREG(st->st_mode))
ret = index_pipe(sha1, fd, type, path, flags);
- else if (size <= big_file_threshold || type != OBJ_BLOB)
+ else if (size <= big_file_threshold || type != OBJ_BLOB ||
+ convert_to_git(path, NULL, 0, NULL, 0))
ret = index_core(sha1, fd, size, type, path, flags);
else
ret = index_stream(sha1, fd, size, type, path, flags);
diff --git a/t/t1051-large-conversion.sh b/t/t1051-large-conversion.sh
new file mode 100755
index 0000000..8b7640b
--- /dev/null
+++ b/t/t1051-large-conversion.sh
@@ -0,0 +1,86 @@
+#!/bin/sh
+
+test_description='test conversion filters on large files'
+. ./test-lib.sh
+
+set_attr() {
+ test_when_finished 'rm -f .gitattributes' &&
+ echo "* $*" >.gitattributes
+}
+
+check_input() {
+ git read-tree --empty &&
+ git add small large &&
+ git cat-file blob :small >small.index &&
+ git cat-file blob :large | head -n 1 >large.index &&
+ test_cmp small.index large.index
+}
+
+check_output() {
+ rm -f small large &&
+ git checkout small large &&
+ head -n 1 large >large.head &&
+ test_cmp small large.head
+}
+
+test_expect_success 'setup input tests' '
+ printf "\$Id: foo\$\\r\\n" >small &&
+ cat small small >large &&
+ git config core.bigfilethreshold 20 &&
+ git config filter.test.clean "sed s/.*/CLEAN/"
+'
+
+test_expect_success 'autocrlf=true converts on input' '
+ test_config core.autocrlf true &&
+ check_input
+'
+
+test_expect_success 'eol=crlf converts on input' '
+ set_attr eol=crlf &&
+ check_input
+'
+
+test_expect_success 'ident converts on input' '
+ set_attr ident &&
+ check_input
+'
+
+test_expect_success 'user-defined filters convert on input' '
+ set_attr filter=test &&
+ check_input
+'
+
+test_expect_success 'setup output tests' '
+ echo "\$Id\$" >small &&
+ cat small small >large &&
+ git add small large &&
+ git config core.bigfilethreshold 7 &&
+ git config filter.test.smudge "sed s/.*/SMUDGE/"
+'
+
+test_expect_success 'autocrlf=true converts on output' '
+ test_config core.autocrlf true &&
+ check_output
+'
+
+test_expect_success 'eol=crlf converts on output' '
+ set_attr eol=crlf &&
+ check_output
+'
+
+test_expect_success 'user-defined filters convert on output' '
+ set_attr filter=test &&
+ check_output
+'
+
+test_expect_success 'ident converts on output' '
+ set_attr ident &&
+ rm -f small large &&
+ git checkout small large &&
+ sed -n "s/Id: .*/Id: SHA/p" <small >small.clean &&
+ head -n 1 large >large.head &&
+ sed -n "s/Id: .*/Id: SHA/p" <large.head >large.clean &&
+ test_cmp small.clean large.clean
+'
+
+test_done
--
1.7.9.9.g04d94
^ permalink raw reply related
* Re: git log -z doesn't separate commits with NULs
From: Jeff King @ 2012-02-24 9:52 UTC (permalink / raw)
To: Nikolaj Shurkaev; +Cc: Junio C Hamano, Jakub Narebski, git
In-Reply-To: <4F475689.4040203@gmail.com>
On Fri, Feb 24, 2012 at 12:21:13PM +0300, Nikolaj Shurkaev wrote:
> I'll propose to put something like this into git documentation
> --------------------------------------------------------------------------------------------
> diff --git a/Documentation/git-format-patch.txt
> b/Documentation/git-format-patch.txt
> index 6ea9be7..63267c6 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -23,6 +23,7 @@ SYNOPSIS
> [--cover-letter] [--quiet]
> [<common diff options>]
> [ <since> | <revision range> ]
> + [[\--] <path>...]
>
> DESCRIPTION
> -----------
> @@ -219,6 +220,12 @@ you can use `--suffix=-patch` to get
> `0001-description-of-my-change-patch`.
> range are always formatted as creation patches, independently
> of this flag.
>
> +[\--] <path>...::
> + Put in patches only those modifications that affect specified files
> + and folders. It's important to understand that log message of the
> + commit may become inappropriate because some parts of patch may be
> + cut off.
> +
I think that text looks OK. But to my mind, it is not that format-patch
accepts a path parameter, but rather that it takes arbitrary log-like
arguments. So you could do "git format-patch --grep=whatever", or even
something like "git format-patch --cherry".
I don't know how well tested every option is, though, so maybe it's not
a good idea to encourage the use of random options.
-Peff
^ permalink raw reply
* git-svn: Please improve confusing error message "needs update" (when actually commit is needed) in rebase
From: Jakub Holy @ 2012-02-24 9:55 UTC (permalink / raw)
To: git
In-Reply-To: <CAFXq=J7hoUtb06LBAPhrbdmtHB8FGWpmUoz2yF-UJz8G=_smPA@mail.gmail.com>
Hi,
I hope this is the right way to submit this enhancement request.
When I run rebase with uncommited local changes, I get rather unclear error
message:
$ git svn rebase -v
mymodule/src/main/java/example/MyModifiedFile.java: needs update
rights-lib/src/main/java/example/AnotherModifiedFile.java: needs update
update-index --refresh: command returned error: 1
If the message was something like "You have uncommited changes in your
workspace, commit them before running rebase." (followed by a list of the
files) it would be much easier for newcomers like me to understand what has
to be done.
I tried to understand what has to be changed in the code but it exceeds my
low level of knowledge :-(
Thank you!
Best regards, Jakub Holy
--
Jakub Holy
Solutions Engineer | +47 966 23 666
Iterate AS | www.iterate.no
The Lean Software Development Consultancy
--
Jakub Holy
Solutions Engineer | +47 966 23 666
Iterate AS | www.iterate.no
The Lean Software Development Consultancy
^ permalink raw reply
* Improving merge messages for 1.7.10 and making "pull" easier
From: Ævar Arnfjörð Bjarmason @ 2012-02-24 9:59 UTC (permalink / raw)
To: Git Mailing List; +Cc: Thomas Rast, Junio C Hamano, Linus Torvalds
With 1.7.10 we'll get merge edit messages by default (which I like)
but I wonder if we could better clarify this to users when they see a
merge message pop up.
Firstly (and as a more general thing) I think we should add a mention
of "git merge --abort" to the message, just saving an empty file is
not sufficient to fully clear the merge state:
diff --git a/builtin/merge.c b/builtin/merge.c
index d3e1e8d..9169c74 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -889,8 +889,8 @@ static const char merge_editor_comment[] =
N_("Please enter a commit message to explain why this merge is
necessary,\n"
"especially if it merges an updated upstream into a topic branch.\n"
"\n"
- "Lines starting with '#' will be ignored, and an empty message aborts\n"
- "the commit.\n");
+ "Lines starting with '#' will be ignored, and an empty message
followed\n"
+ "by 'git merge --abort' the merge.\n");
static void prepare_to_commit(void)
{
Additionally, perhaps it would be a good idea to:
* Detect if the user didn't run this explicitly but implicitly from a
"git pull". We could pass some env var along or another option
(e.g. --internal-from-porcelain=pull) and add this:
You've merged implicitly via a "git pull", if you're just
updating some local work in progress to keep up with upstream
you may want to use "git pull --rebase" instead (or set the
pull.rebase configuration variable) to rebase instead of merge.
* Explicitly check if we're merging an updated upstream into the
work-in-progress topic, then mentioning some succinct version of
Junio's #1 point at
http://git-blame.blogspot.com/2012/02/anticipating-git-1710.html
I work with a lot of inexperienced git users and a lot of them are
going to be very confused by this change. I still think it's a good
change to make, but we could do a lot more to mitigate the inevitable
confusion.
One thing that would help these users in particular would be to have
some easy to use replacement for their frequent use of "git
pull".
They don't often commit their work (because of git inexperience) so
rebasing will error out because the tree is unclean. I tried to hack
support for unclean trees into git-rebase(1) the other day but it was
difficult because at various points in the process it'll do a "git
reset --hard".
Maybe having an option to "git pull" that would make it do:
git stash save --include-untracked -- "Automatically created stash
by \"git pull\" at $(date)"
git pull --rebase
git stash pop
Would make things easier. It could be called "git pull --rebase
--stash".
^ permalink raw reply
* Submodule status inside nested submodule fails
From: Charles Brossollet @ 2012-02-24 9:53 UTC (permalink / raw)
To: git
Hello there,
Using MSysGit 1.7.9 on Win7 (64 bit), I have a repo with the following
structure:
main/
src/
ext/
submodule/
modules/module1
modules/module2
submodule is... a submodule, having itself submodules.
When I query submodule status --recursive in main/, no problem.
But when I query submodule status in ext/submodule, I get error "You need to
run this command from the toplevel of the working tree.", which is a wrong
error message.
Searching for message in git source I found the test triggering the message to
be an empty result for "git rev-parse --show-cdup". Running the command in
ext/submodule strangely returns the *absolute* path of ext/submodule, which
should not happen because this command returns path of current dir relative to
the containing working tree.
Is there something wrong with my repo? Or have I found a bug? Any help
appreciated
Charles
^ permalink raw reply
* Re: [FYI] very large text files and their problems.
From: Ian Kumlien @ 2012-02-24 10:11 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8Bdbegs7QdztvsFnKPcpAX5UL7s7uc37wF3_nF4kJQjrQ@mail.gmail.com>
I'm uncertain if you got my reply since i did it out of bounds - so i'll
repeat myself - sorry... =)
On Wed, Feb 22, 2012 at 11:18:19PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Feb 22, 2012 at 10:49 PM, Ian Kumlien <pomac@vapor.com> wrote:
> > Hi,
> >
> > We just saw a interesting issue, git compressed a ~3.4 gb project to ~57 mb.
>
> How big are those files? How many of them? How often do they change?
This was the first check in, there is no deltas yet.
The file in question is ~3.3 gb in size - ie exactly: 3310214313 bytes
(as seen below in the malloc failure)
git show <blob sha1 id> |wc -c gives the same exact result.
> > But when we tried to clone it on a big machine we got:
> >
> > fatal: Out of memory, malloc failed (tried to allocate
> > 18446744072724798634 bytes)
> >
> > This is already fixed in the 1.7.10 mainline - but it also seems like
>
> Does 1.7.9 have this problem?
Only tested 1.7.8 and 1.7.9.1 - works in mainline git (pre-1.7.10)
> > git needs to have atleast the same ammount of memory as the largest
> > file free... Couldn't this be worked around?
> >
> > On a (32 bit) machine with 4GB memory - results in:
> > fatal: Out of memory, malloc failed (tried to allocate 3310214313 bytes)
> >
> > (and i see how this could be a problem, but couldn't it be mitigated? or
> > is it bydesign and intended behaviour?)
>
> I think that it's delta resolving that hogs all your memory. If your
> files are smaller than 512M, try lower core.bigFileThreshold. The
> topic jc/split-blob, which stores a big file are several smaller
> pieces, might solve your problem. Unfortunately the topic is not
> complete yet.
Well, in this case it's just stream unpacking gzip data to disk, i
understand if delta would be a problem... But wouldn't delta be a
problem in the sence of <size_of_change>+<size_of_subdata>+<result> ?
Ie, if the file is mmapped - it shouldn't have to be allocated, right?
> --
> Duy
^ permalink raw reply
* [BugReport] git tag -a / git show
From: Romain Vimont (®om) @ 2012-02-24 10:24 UTC (permalink / raw)
To: git
$ git --version
git version 1.7.5.4
$ git log --pretty=online
0ef41513d0b6d0ad28f21d0ac1da7096ad1dc6ff This is the last commit
a4702c69c28484d357179166cf3b116764da20a4 This is a commit
Now, I edit some files (for example in a config file "mock_data=true"),
then I want to tag without commiting this change.
$ git tag -a v0.1 -m 'My v0.1 with mock data'
$ git show v0.1
tag v0.1
Tagger: Me <me@me.me>
Date: Fri Feb 24 11:23:38 2012 +0100
My v0.1 with mock data
commit 0ef41513d0b6d0ad28f21d0ac1da7096ad1dc6ff
Author: Me <me@me.me>
Date: Fri Feb 24 11:14:19 2012 +0100
This is the last commit
diff --git a/myfile b/myfile
index 8430bf6..20feeb6 100644
...
And it shows the diff between a4702c69c28484d357179166cf3b116764da20a4
and 0ef41513d0b6d0ad28f21d0ac1da7096ad1dc6ff (the two last commits).
Instead, it should show the diff between
0ef41513d0b6d0ad28f21d0ac1da7096ad1dc6ff (the last commit) and v0.1 (the
tag).
Best regards,
®om
^ permalink raw reply
* Re: [PATCH] pretty: add '*' modifier to add LF after non-empty
From: Luc Pionchon @ 2012-02-24 10:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7gzd498r.fsf@alter.siamese.dyndns.org>
On Thu, Feb 23, 2012 at 21:53, Junio C Hamano <gitster@pobox.com> wrote:
>
> Luc Pionchon <pionchon.luc@gmail.com> writes:
>
> > Add the '*' modifier, similar to the '+' modifier,
> > to add a line-feed after a non-empty placeholder.
>
> Hrm, I thought I designed the plus and minus fairly carefully so that
> nobody needs to add this later.
>
> Wouldn't it be sufficient to write
>
> Foo%n%-d
>
>
> that says "We usually have LF after Foo, and write %d after that, but we
> might not have anything interesting in %d at all, in which case we don't
> add that %n"?
What I want is a LF after the non empty, rather than before.
It seems to me that %n%-d is equivalent to %+d,
or did I miss something?
See the 3 examples below, with %n%-d, %+d and %*d
$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%n%-d %C(reset)%s"
*
|\ (HEAD, origin/master, origin/HEAD, master) Sync with 1.7.9.2
| *
| | (v1.7.9.2, origin/maint) Git 1.7.9.2
| * completion: use tabs for indentation
| * completion: remove stale "to submit patches" documentation
* | git-p4: the option to specify 'host' is -H, not -h
$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%+d %C(reset)%s"
*
|\ (HEAD, origin/master, origin/HEAD, master) Sync with 1.7.9.2
| *
| | (v1.7.9.2, origin/maint) Git 1.7.9.2
| * completion: use tabs for indentation
| * completion: remove stale "to submit patches" documentation
* | git-p4: the option to specify 'host' is -H, not -h
$ ./git log -5 --graph --pretty=format:"%C(yellow bold)%*d %C(reset)%s"
* (HEAD, origin/master, origin/HEAD, master)
|\ Sync with 1.7.9.2
| * (v1.7.9.2, origin/maint)
| | Git 1.7.9.2
| * completion: use tabs for indentation
| * completion: remove stale "to submit patches" documentation
* | git-p4: the option to specify 'host' is -H, not -h
Also as a side note, I noticed that color is lost after new lines.
>
> > +test_expect_success 'add LF after non-empty (1) (empty)' '
> > + git show -s --pretty=format:"%*d%s%nfoo%n" HEAD^^ >actual &&
>
> Shouldn't this be equivalent to "%n%-d%s%nfoo%n", which in turn is covered
> by one of the previous tests (del LF before empty)?
I think the later is equivalent to %+d%s%nfoo%n, am I wrong?
^ permalink raw reply
* Re: [FYI] very large text files and their problems.
From: Nguyen Thai Ngoc Duy @ 2012-02-24 11:14 UTC (permalink / raw)
To: Ian Kumlien; +Cc: git
In-Reply-To: <20120224101121.GA9526@pomac.netswarm.net>
On Fri, Feb 24, 2012 at 5:11 PM, Ian Kumlien <pomac@vapor.com> wrote:
> I'm uncertain if you got my reply since i did it out of bounds - so i'll
> repeat myself - sorry... =)
yes I received it, just too busy this week.
>> > git needs to have atleast the same ammount of memory as the largest
>> > file free... Couldn't this be worked around?
>> >
>> > On a (32 bit) machine with 4GB memory - results in:
>> > fatal: Out of memory, malloc failed (tried to allocate 3310214313 bytes)
>> >
>> > (and i see how this could be a problem, but couldn't it be mitigated? or
>> > is it bydesign and intended behaviour?)
>>
>> I think that it's delta resolving that hogs all your memory. If your
>> files are smaller than 512M, try lower core.bigFileThreshold. The
>> topic jc/split-blob, which stores a big file are several smaller
>> pieces, might solve your problem. Unfortunately the topic is not
>> complete yet.
>
> Well, in this case it's just stream unpacking gzip data to disk, i
> understand if delta would be a problem... But wouldn't delta be a
> problem in the sence of <size_of_change>+<size_of_subdata>+<result> ?
>
> Ie, if the file is mmapped - it shouldn't have to be allocated, right?
We should not delta large files. I was worried that the large file
check could go wrong, But I guess your blob's not deltified in this
case.
When you receive a pack during a clone, the pack is streamed to
index-pack, not mmapped, and index-pack checks every object in there
in uncompressed form. I think I have found a way to avoid allocating
that much. Need some more check, then send out.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox