* Re: [PATCH] gitk: fix the display of files when filtered by path
From: Pat Thoyts @ 2011-12-15 21:18 UTC (permalink / raw)
To: Martin von Zweigbergk
Cc: msysGit, Pat Thoyts, Johannes Schindelin, Git, David Aguilar,
Junio C Hamano, Paul Mackerras
[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]
On Dec 15, 2011 8:01 PM, "Martin von Zweigbergk" <
martin.von.zweigbergk@gmail.com> wrote:
>
> Hi,
>
> On Thu, Dec 15, 2011 at 1:24 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Wed, 14 Dec 2011, David Aguilar wrote:
> >
> >> On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
> >> <patthoyts@users.sourceforge.net> wrote:
> >> > Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to
files
> >> > under the given directory but the file list is left empty. This is
because
> >> > the path_filter function fails to match the filenames which are
relative
> >> > to the working tree to the filter which is filessytem relative.
> >> > This solves the problem by making both names fully qualified
filesystem
> >> > paths before performing the comparison.
>
> How is this related to my patches from April? See
> http://thread.gmane.org/gmane.comp.version-control.git/170853. It's
> clearly not the same problem, but will the patches conflict? Will some
> of mine be unnecessary?
>
> > Thanks for reminding me that I did not yet apply and push. Did so now.
>
> What do you mean by this? Push to where?
> git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
This is about msysgit as I also posted this there.
> Paul and Junio, the patches I sent in April are still not in git.git,
> are they? Can we use another repo until the kernel.org one is up? More
> than eight months to get a patch (or eight) merged is way too long,
> IMO.
>
> Martin
I'm not sure how this might relate to your patches. I've got a version
merged on top of the last version of the gitk report that I have which
includes those and it seems fine. As stated I did this work against
git-core as the gitk repository continues to be unavailable. However as I
have a pretty current snapshot I have pushed this to github to provide
some visibility of things I know are not present within git-core. See
http://github.com/patthoyts/gitk.git
On Dec 15, 2011 8:01 PM, "Martin von Zweigbergk" <
martin.von.zweigbergk@gmail.com> wrote:
[-- Attachment #2: Type: text/html, Size: 2962 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] t5704 (bundle): rewrite for larger coverage
From: Jonathan Nieder @ 2011-12-15 21:16 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-2-git-send-email-artagnon@gmail.com>
Hi,
Ramkumar Ramachandra wrote:
> Rewrite
Always a scary word. Very rarely justified, especially when the
original and rewritten versions of something are not going to be able
to coexist for a period while the bugs are ironed out. It doesn't
leave me optimistic.
> the git-bundle testsuite to exercise more of its
> functionality.
Luckily, this goal suggests that I am going to see some new tests
added, without the existing coverage being removed or mangled, so
maybe I can ignore the fears awakened by the word "Rewrite". Let's
see...
[...]
> --- a/t/t5704-bundle.sh
> +++ b/t/t5704-bundle.sh
> @@ -1,56 +1,99 @@
> #!/bin/sh
>
> -test_description='some bundle related tests'
> +test_description='Test git-bundle'
Why?
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> + git config core.logAllRefUpdates false &&
Why?
> + test_commit initial &&
> + git checkout -b branch &&
> + test_commit second &&
> + test_commit third &&
> + git checkout master &&
> + test_commit fourth file
> +'
No explicit tags in the setup this time. Now all commits are referred
to by tags, which worsens the coverage, since if some future change
caused commits not referred to by a tag to be dropped, it would be
missed. Paraphrasing
>file &&
git add file &&
test_tick &&
git commit -m initial &&
git tag -m initial initial
to
test_commit initial file
when not preparing to make some other change in the same place and if
the original was not too confusing feels like gratuitous churn.
[...]
> +test_expect_success 'create succeeds' '
> + git bundle create bundle second third &&
> + cat >expect <<-\EOF &&
> + OBJID refs/tags/third
> + OBJID refs/tags/second
> + EOF
> + {
> + git ls-remote bundle |
> + sed "s/$_x40/OBJID/g"
> + } >actual &&
> + test_cmp expect actual
> +'
A new test. What assertion is it testing? Why censor out the
object names when comparing the expected object names to the
actual ones, instead of computing the appropriate object names
for the expected result? Is this new test useful, or does it
cover ground already tested in t5510-fetch.sh?
> +test_expect_success 'verify succeeds' '
> + git bundle create bundle second third &&
> + git bundle verify bundle
> '
A test for "git bundle verify" is a welcome addition.
> +test_expect_success 'list-heads succeeds' '
> + git bundle create bundle second third &&
> + cat >expect <<-\EOF &&
> + OBJID refs/tags/second
> + OBJID refs/tags/third
> + EOF
> + {
> + git bundle list-heads bundle |
> + sed "s/$_x40/OBJID/g"
> + } >actual &&
> + test_cmp expect actual
> +'
> +
Based on 'git grep -e "git bundle list-heads" -- t', there don't seem
to be any existing tests for "git bundle list-heads" except for
t5510-fetch.sh, but I'm not sure what this adds on top of that one.
> -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
Dropped?
> +test_expect_success 'create dies on invalid bundle filename' '
> + mkdir adir &&
> + test_expect_code 128 git bundle create adir --all
> +'
How is "invalid bundle filename" a clearer explanation than "bundle
file cannot be created"?
>
> +test_expect_success 'disallow stray command-line options' '
> + test_must_fail git bundle create --junk bundle second third
> '
Ok. Might also be useful to check that no "--junk" or "bundle" file
is created.
> +test_expect_failure 'disallow stray command-line arguments' '
> + git bundle create bundle second third &&
> + test_must_fail git bundle verify bundle junk
> +'
In this case, "stray command-line arguments" actually means "extra
arguments to 'verify'", I guess?
What happens if I run "git bundle verify *.bundle" in a directory
with multiple bundles? What should happen?
> +test_expect_success 'create accepts rev-list options to narrow refs' '
> + git bundle create bundle --all -- file &&
I don't understand what "options to narrow refs" means. Does that
mean options like --remotes=origin which yield refs from some subset
of the ref namespace, unlike --all?
[...]
> +test_expect_success 'unbundle succeeds' '
A test for "git bundle unbundle" is a welcome addition.
[...]
> test_expect_failure 'bundle --stdin' '
> -
> echo master | git bundle create stdin-bundle.bdl --stdin &&
> git ls-remote stdin-bundle.bdl >output &&
> grep master output
> -
> '
Seems like a gratuitous change to mix into a patch that introduces
functional changes.
I found this hard to review, since it doesn't seem very focussed ---
it mixes style cleanups, removal of code, and introduction of new
code. I'd be way happier to see a new patch that just adds new tests
to the script without potentially breaking anything on the way. Then
if the style cleanups still seem important to you, they can be
reviewed as a separate patch.
Hoping that clarifies a little,
Jonathan
^ permalink raw reply
* [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD
From: Joe Ratterman @ 2011-12-15 20:58 UTC (permalink / raw)
To: gitster, git, jnareb; +Cc: Joe Ratterman
It is possible that the HEAD reference does not point to an existing
branch. When viewing such a repository in gitweb, a message like this
one was sent to the error log:
gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
Signed-off-by: Joe Ratterman <jratt0@gmail.com>
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4f0c3bd..5af06d6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5440,7 +5440,7 @@ sub git_heads_body {
for (my $i = $from; $i <= $to; $i++) {
my $entry = $headlist->[$i];
my %ref = %$entry;
- my $curr = $ref{'id'} eq $head;
+ my $curr = $head ? ($ref{'id'} eq $head) : 0;
if ($alternate) {
print "<tr class=\"dark\">\n";
} else {
--
1.7.7.1
^ permalink raw reply related
* Re: [msysGit] Windows & executable bit for newly created files
From: Junio C Hamano @ 2011-12-15 20:56 UTC (permalink / raw)
To: Dirk Süsserott; +Cc: Git Mailing List
In-Reply-To: <4EEA5387.5020808@dirk.my1.cc>
Dirk Süsserott <newsletter@dirk.my1.cc> writes:
> Is there a way to convince git that the new mode is 755 instead of 644,
> even with core.filemode set to false? So that the mode is correct when I
> checkout the file under Linux later on?
"git update-index --chmod=+x"?
^ permalink raw reply
* [PATCH] Add '-P' as a synonym for '--no-pager' in the git command
From: Joe Ratterman @ 2011-12-15 20:55 UTC (permalink / raw)
To: gitster, git; +Cc: Joe Ratterman
Also, add both -P|--no-pager to the existing -p|--paginate in bash
completion.
Signed-off-by: Joe Ratterman <jratt0@gmail.com>
---
Documentation/git.txt | 3 ++-
contrib/completion/git-completion.bash | 2 +-
git.c | 4 ++--
| 8 ++++++++
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index e869032..c7f8445 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git' [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
- [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
+ [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]
[--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
[-c <name>=<value>]
[--help] <command> [<args>]
@@ -329,6 +329,7 @@ help ...`.
configuration options (see the "Configuration Mechanism" section
below).
+-P::
--no-pager::
Do not pipe git output into a pager.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index cc1bdf9..8b9ea1b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2640,7 +2640,7 @@ _git ()
case "$i" in
--git-dir=*) __git_dir="${i#--git-dir=}" ;;
--bare) __git_dir="." ;;
- --version|-p|--paginate) ;;
+ --version|-p|--paginate|-P|--no-pager) ;;
--help) command="help"; break ;;
*) command="$i"; break ;;
esac
diff --git a/git.c b/git.c
index 8e34903..baa1613 100644
--- a/git.c
+++ b/git.c
@@ -7,7 +7,7 @@
const char git_usage_string[] =
"git [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
- " [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]\n"
+ " [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]\n"
" [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n"
" [-c name=value] [--help]\n"
" <command> [<args>]";
@@ -103,7 +103,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
exit(0);
} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
use_pager = 1;
- } else if (!strcmp(cmd, "--no-pager")) {
+ } else if (!strcmp(cmd, "-P") || !strcmp(cmd, "--no-pager")) {
use_pager = 0;
if (envchanged)
*envchanged = 1;
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 320e1d1..783915e 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -84,6 +84,14 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '
! test -e paginated.out
'
+test_expect_success TTY 'no pager with -P' '
+ rm -f paginated.out ||
+ cleanup_fail &&
+
+ test_terminal git -P log &&
+ ! test -e paginated.out
+'
+
test_expect_success TTY 'no pager with --no-pager' '
rm -f paginated.out ||
cleanup_fail &&
--
1.7.7.1
^ permalink raw reply related
* Re: [PATCH 0/2] Improve git-bundle builtin
From: Jonathan Nieder @ 2011-12-15 20:54 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-1-git-send-email-artagnon@gmail.com>
Hi Ram,
Ramkumar Ramachandra wrote:
> 1. There's a SP between the OBJID and the ref name in list-heads as
> opposed to the TAB used by other git commands such as ls-remote,
> diff-tree. Will fixing it break someone's parser somewhere?
I don't know. Would there be any advantage at all to changing the
output format of the tool? Bad idea.
If the goal is to avoid confusion, perhaps a note in the documentation
would help.
> 2. Is it worth fixing the "--stdin" tests? What is the usecase?
Is "script that wants to list which revs to bundle, possibly exceeding
the command-line length limit" not enough of a use case? Yes, I think
it is very much worth fixing.
Thanks for looking at this.
Jonathan
^ permalink raw reply
* Re: Branch names with slashes
From: Neal Kreitzinger @ 2011-12-15 20:40 UTC (permalink / raw)
To: Leonardo Kim; +Cc: git
In-Reply-To: <CAGcUY13TOodu1BO3DCoNnVvNZ9QkWAbD-RmyqQX6P1q6tcO+yg@mail.gmail.com>
On 12/14/2011 4:17 AM, Leonardo Kim wrote:
> Branch names can contain slashes, so we can use 'development/foo' as
> a branch name. If I choose 'development' as a branch name, it doesn't
> work. There is a directory named development at '.git/refs/heads'
> directory. So we cannot create a file named development for
> 'refs/heads/development'.
>
> An error message may occurs like below. Unfortunately, It is not of
> help to me. 'error: 'refs/heads/development/foo' exists; cannot
> create 'refs/heads/development'.
>
> I think that dealing with a file system and an error message above is
> not sufficient for a novice like me. I hope that it should be
> improved.
>
FYI, We also use slashes in our branchnames to leverage some of the
benefits of a path-like namespace like pattern matching and the logical
expression of hierarchies using descriptive compound names. (We use git
1.7.1 on linux.) Here's something to keep in mind: You have to plan
(think ahead) for your naming conventions so that the namespaces will
maintain uniqueness at a peer level over time that cannot be confused as
subdirs under .git/refs/heads. For example:
branchnames:
/mysystem/generic
/mysystem/generic/project1
will not work because /mysystem/generic appears to be a parent dir to
/mysystem/generic/project1 under .git/refs/heads. The solution is:
/mysystem/generic/base
/mysystem/generic/project1
these branches can coexist because they are unique without one appearing
to be a parent dir of the other. IOW, their namespaces are peers in
their entirety. To carry the example a little further,
/mysystem/generic/project1/part2
will not work because once again it appears to be a subdir of an
existing branchname (ref). However,
/mysystem/generic/project1-part2
will work.
I think the reason for this is that if you look at .git/refs/heads you
will see that these slash delimited branch names are actually stored as
subdirs in the filesystem sense. Therefore, git will get confused and
error out as it tries to find branchnames that are not entirely unique
by their full path namespace under .git/refs/heads because a branch
namespace that is a prefix (in the filesystem sense) of another branch
name would occupy the same path under .git/refs/heads without being
distinguishable as unique, and vice versa.
Hope this helps. (Maybe someone else will find a clearer way to explain
this.)
v/r,
neal
^ permalink raw reply
* [msysGit] Windows & executable bit for newly created files
From: Dirk Süsserott @ 2011-12-15 20:07 UTC (permalink / raw)
To: Git Mailing List
Hi,
I sometimes write new (shell/perl) scripts under Windows whithin
msysGit's bash. An "ls -l" shows them as executable:
$ ls -l
-rwxr-xr-x ... 11 Dec 15 20:33 test.sh
Ok, "ls" just *pretends* they have the bit set (I think it just peeks
for the she-bang line).
Adding the file and looking at the patch gives:
$ git add test.sh
$ git diff --cached
diff --git a/test.sh b/test.sh
new file mode 100644
index 0000000..13f4793
--- /dev/null
+++ b/test.sh
@@ -0,0 +1,2 @@
+#!/bin/sh
+
Is there a way to convince git that the new mode is 755 instead of 644,
even with core.filemode set to false? So that the mode is correct when I
checkout the file under Linux later on?
My impression is that when git once knows the file has 755, it remembers
that. I'd like to avoid those commits (made under Linux) with "chmod +x
for scripts newly created under Windows".
Sometimes I create a script under Windows, add and commit it, create a
patch (git-format-patch), edit the patch (644 -> 755), reset the branch,
and finally git-am the patch. It works but is not particularly convenient.
Is there sth. like
$ git add --filemode=755 <filepattern> ?
Cheers,
Dirk
^ permalink raw reply
* Re: Git difftool / mergetool on directory tree
From: Neal Kreitzinger @ 2011-12-15 19:53 UTC (permalink / raw)
To: Daniele Segato; +Cc: Git Mailing List
In-Reply-To: <4EE8618E.7020902@gmail.com>
On 12/14/2011 2:42 AM, Daniele Segato wrote:
> many diff / merge tool around have the ability to compare a directory
> tree (meld is one, but there are many).
>
> Is there a way to start a difftool or a mergetool on a folder instead of
> the single file?
>
> It would be an handsome feature to git.
>
> I googled a little before popping out this question and I only found
> suggestion on how to open "many" file at once instead of opening them
> serially but that's not the same thing not as powerful as directory
> comparison.
>
(This works with kdiff3 as the mergetool/difftool on git 1.7.1 on linux):
git difftool <commit>:<dir>/ <commit-2>:<dir>/
e.g., git difftool mycommit:ourdir/ yourcommit:ourdir/
<commit> = pointer to a commit (ie: sha-1, branchname, tag, ref)
<dir> = path of dir relative to worktree
v/r,
neal
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Junio C Hamano @ 2011-12-15 19:50 UTC (permalink / raw)
To: Martin von Zweigbergk, Paul Mackerras; +Cc: git
In-Reply-To: <CAOeW2eHD7Xutf+pHDyMOo=uZC9PSFZi+aMq1Rx80iTKPFApr8A@mail.gmail.com>
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> Paul and Junio, the patches I sent in April are still not in git.git,
> are they? Can we use another repo until the kernel.org one is up? More
> than eight months to get a patch (or eight) merged is way too long,
> IMO.
I tend to agree.
I have this slight suspicion that Paul would appreciate if somebody who
cares about gitk who is capable and willing steps forward and takes over
the maintainership of gitk, as he is busy in his other projects.
^ permalink raw reply
* Re: [msysGit] [PATCH] gitk: fix the display of files when filtered by path
From: Martin von Zweigbergk @ 2011-12-15 19:42 UTC (permalink / raw)
To: Pat Thoyts, Johannes Schindelin, Paul Mackerras, Junio C Hamano
Cc: David Aguilar, Git, msysGit
In-Reply-To: <alpine.DEB.1.00.1112151023280.2615@bonsai2>
Hi,
On Thu, Dec 15, 2011 at 1:24 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 14 Dec 2011, David Aguilar wrote:
>
>> On Tue, Dec 13, 2011 at 8:50 AM, Pat Thoyts
>> <patthoyts@users.sourceforge.net> wrote:
>> > Launching 'gitk -- .' or 'gitk -- ..\t' restricts the display to files
>> > under the given directory but the file list is left empty. This is because
>> > the path_filter function fails to match the filenames which are relative
>> > to the working tree to the filter which is filessytem relative.
>> > This solves the problem by making both names fully qualified filesystem
>> > paths before performing the comparison.
How is this related to my patches from April? See
http://thread.gmane.org/gmane.comp.version-control.git/170853. It's
clearly not the same problem, but will the patches conflict? Will some
of mine be unnecessary?
> Thanks for reminding me that I did not yet apply and push. Did so now.
What do you mean by this? Push to where?
git://git.kernel.org/pub/scm/gitk/gitk.git is still down.
Paul and Junio, the patches I sent in April are still not in git.git,
are they? Can we use another repo until the kernel.org one is up? More
than eight months to get a patch (or eight) merged is way too long,
IMO.
Martin
^ permalink raw reply
* Re: [PATCH 3/3] Do not create commits whose message contains NUL
From: Junio C Hamano @ 2011-12-15 19:35 UTC (permalink / raw)
To: git; +Cc: Jeff King, Nguyễn Thái Ngọc Duy, Miles Bader
In-Reply-To: <7vehw5oepj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> My gut feeling is that it should store the literal binary contents.
>> However, I don't think this has ever been the case. Even in the initial
>> version of commit-tree.c, we read the input line-by-line and sprintf it
>> into place.
>
> Yeah, you are right. Perhaps we should tweak updated 3/3 to check at the
> lower level commit_tree() then.
>
> I've rewrote the log message for 2/3 as follows so we can go either way
> ;-)
s/rewrote/rewritten/ obviously...
> Convert commit_tree() to take strbuf as message
>
> There wan't a way for commit_tree() to notice if the message the caller
> prepared contained a NUL byte, as it did not take the length of the
> message as a parameter. Use a pointer to a strbuf instead, so that we can
> either choose to allow low-level plumbing commands to make commits
> that contain NUL byte in its message, or forbid NUL everywhere by
> adding the check in commit_tree(), in later patches.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
And 3/3 looks like this:
commit_tree(): refuse commit messages that contain NULs
Current implementation sees NUL as terminator. If users give a message
with NUL byte in it (e.g. editor set to save as UTF-16), the new commit
message will have NULs. However following operations (displaying or
amending a commit for example) will not keep anything after the first NUL.
Stop user right when they do this. If NUL is added by mistake, they have
their chance to fix. Otherwise, log messages will no longer be text "git
log" and friends would grok.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [RFC PATCH 2/4] test-lib: allow testing another git build tree
From: Junio C Hamano @ 2011-12-15 19:05 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Michael Haggerty
In-Reply-To: <3016269.uH93UWUbx5@thomas.inf.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> On Wednesday 14 December 2011 19:07:35 Junio C Hamano wrote:
>> Thomas Rast <trast@student.ethz.ch> writes:
>> > The perf-lib work wants this feature, so we may as well do it for
>> > test-lib in general.
>>
>> How is this different from what GIT_TEST_INSTALLED already gives us
>> (other than "needs more diskspace to keep another source tree fully
>> built", that is)?
>
> I was scared away by the note that it would use (among others) perl
> libs from the current build tree. Upon investigation I also see that
> the test-* situation is still not satisfactory. Some (like
> test-chmtime) are used by the tests for a vital task, and if they ever
> have to be fixed, we would want to use the fixed version in any "test
> an old git" run. OTOH, others (e.g., test-dump-cache-tree) are linked
> with the rest of the code and serve to test an otherwise not
> accessible part of it, and testing an old git should use them from the
> tested tree.
Okay, so GIT_BUILD_DIR variant should now use everything from the tree
being tested for consistency to improve the situation, right?
Ehh, not quite, if you worry about an old one having broken test-chmtime,
you actively do not want that consistency; how is that addressed?
Not that I think we should address such issues---these standalone programs
tend to be small and simple, so always using them from the tree being
tested is a much better policy than using them from the tree you are
running the tests from.
> The disk space argument is moot IMO:
I was not really worried about "space" (disks are cheap) but more about
mental burden of having to keep multiple build trees in the source area.
Keeping multiple installed versions is already necessary to be able to
quickly respond to "I am using Git version X and this does not work".
^ permalink raw reply
* Re: [PATCH 3/3] Do not create commits whose message contains NUL
From: Junio C Hamano @ 2011-12-15 18:46 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git, Miles Bader
In-Reply-To: <20111214182953.GA6469@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm happy to ignore custom programs linking against internal git code,
> but what should "git commit-tree" do?
>
> My gut feeling is that it should store the literal binary contents.
> However, I don't think this has ever been the case. Even in the initial
> version of commit-tree.c, we read the input line-by-line and sprintf it
> into place.
Yeah, you are right. Perhaps we should tweak updated 3/3 to check at the
lower level commit_tree() then.
I've rewrote the log message for 2/3 as follows so we can go either way
;-)
Convert commit_tree() to take strbuf as message
There wan't a way for commit_tree() to notice if the message the caller
prepared contained a NUL byte, as it did not take the length of the
message as a parameter. Use a pointer to a strbuf instead, so that we can
either choose to allow low-level plumbing commands to make commits
that contain NUL byte in its message, or forbid NUL everywhere by
adding the check in commit_tree(), in later patches.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH v2 3/4] wrapper: supply xsetenv and xputenv
From: Erik Faye-Lund @ 2011-12-15 18:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, peff, schwab
In-Reply-To: <7vzketohvg.fsf@alter.siamese.dyndns.org>
On Thu, Dec 15, 2011 at 6:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> +extern int xsetenv(const char *name, const char *val, int override);
>> +extern int xputenv(const char *string);
>
> Actually, putenv(3) takes "char *string".
Ah, thanks for spotting.
> I adjusted 3 & 4 locally before queuing, so there is no need for resend,
> and judging from the later part of the discussion, it seems that it may be
> better to use only the first two patches from this series after all.
Yeah, I agree; patch 1 and 2 are the only ones that makes sense.
^ permalink raw reply
* Re: [PATCH v2 1/3] merge: abort if fails to commit
From: Junio C Hamano @ 2011-12-15 18:23 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Miles Bader
In-Reply-To: <1323956843-5326-1-git-send-email-pclouds@gmail.com>
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] push: --ignore-stale option
From: Junio C Hamano @ 2011-12-15 18:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111215073816.GD1327@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But I still see this as solving the wrong problem, and encouraging a
> dangerous workflow.
Yes, that is what I meant, so we are in total agreement that these people
should be encouraged not to use the matching semantics.
How that encouragement is made we may differ, though.
^ permalink raw reply
* Re: tr/pty-all (Re: What's cooking in git.git (Dec 2011, #04; Tue, 13))
From: Junio C Hamano @ 2011-12-15 18:19 UTC (permalink / raw)
To: Jeff King
Cc: Jonathan Nieder, Thomas Rast, git,
Nguyễn Thái Ngọc Duy, Michael Haggerty
In-Reply-To: <20111215065529.GA1327@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Dec 14, 2011 at 07:23:14PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > This correctly detects the bug in t7006. I can't decide if it's clever
>> > or ugly.
>>
>> I would say it falls on the latter side of the line by small margin. Let's
>> do the /dev/null thing and be done with it.
>
> Darn, I wanted to post it on my fridge with an "A+".
Heh.
If it were without the perl "tell" bits, it would have been "clever and
clean", perhaps like
arrange the following to read from t/random-stdin file
- run the test script
- "read x"
compare the $x you just read with t/random-stdin file
But I think it is good enough to be stupid and clean.
> Here's a cleaned-up version of the /dev/null one.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 4/4] use wrapper for unchecked setenv/putenv calls
From: Erik Faye-Lund @ 2011-12-15 18:04 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster, schwab
In-Reply-To: <20111214181658.GA1691@sigill.intra.peff.net>
On Wed, Dec 14, 2011 at 7:16 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Dec 14, 2011 at 03:07:11PM +0100, Erik Faye-Lund wrote:
>
>> This avoids us from accidentally dropping state, possibly leading
>> to unexpected behaviour.
>
> I do think this is fine in a "be extra cautious" kind of way.
>
>> This is especially important on Windows, where the maximum size of
>> the environment is 32 kB.
>
> But does your patch actually detect that? As Andreas pointed out, these
> limits don't typically come into play at setenv time. Instead, the
> environment is allocated on the heap, and then the result is passed to
> exec/spawn, which will fail.
>
> So your patch is really detecting a failure to malloc, not an overflow
> of the environment size, and Windows is just as (un)likely to run out of
> heap as any other platform.
>
You are right; I just assumed that our setenv was implemented on top
of SetEnvironmentVariable, which does impose max-limits on setting (if
I read the documentation correctly):
http://msdn.microsoft.com/en-us/library/windows/desktop/ms686206(v=vs.85).aspx
But we don't. We have mingw_setenv, which simply modifies environ.
> You can check how your platform behaves by applying this patch:
>
> diff --git a/git.c b/git.c
> index f10e434..57f6b12 100644
> --- a/git.c
> +++ b/git.c
> @@ -223,6 +223,16 @@ static int handle_alias(int *argcp, const char ***argv)
> alias_argv[i] = (*argv)[i];
> alias_argv[argc] = NULL;
>
> + /* make gigantic environment */
> + {
> + int len = 256 * 1024;
> + char *buf = xmalloc(len);
> + memset(buf, 'z', len);
> + buf[len-1] = '\0';
> + if (setenv("FOO", buf, 1))
> + die("setenv failed");
> + }
> +
> ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
> if (ret >= 0) /* normal exit */
> exit(ret);
>
> and then running:
>
> git -c alias.foo='!echo ok' foo
>
> which yields:
>
> fatal: cannot exec 'echo ok': Argument list too long
>
> on Linux.
>
Yeah, but I'm getting:
$ git -c alias.foo='!echo ok' foo
ok
Which is strange to me.
But if I do:
$ git -c alias.foo='!echo $FOO' foo
it does echo a bunch of 'z's. And we get the expected amount:
$ git -c alias.foo='!echo $FOO' foo | wc -c
262144
This strikes me as very strange, because we end up calling the ANSI
version of CreateProcess with an environment-block beyond 32767 chars,
which MSDN says should fail:
"The ANSI version of this function, CreateProcessA fails if the total
size of the environment block for the process exceeds 32,767
characters."
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx
Oh well, this is strange behavior in a good way; I'm not going to complain :P
^ permalink raw reply
* Re: [PATCH 2/2] push: --ignore-stale option
From: Marc Branchaud @ 2011-12-15 17:47 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111215073816.GD1327@sigill.intra.peff.net>
On 11-12-15 02:38 AM, Jeff King wrote:
>
> But I still see this as solving the wrong problem, and encouraging a
> dangerous workflow. Why are you using ":" to push matching branches if
> you don't want to push topic1? I think a much more likely scenario is:
>
> $ git clone <<origin>>
> $ git checkout -b topic1 origin/topic1
> $ work work work
> $ : hmm, it's not ready yet
> $ git checkout -b topic2 origin/topic2
> $ work work work
> $ : looking good, let's ship it!
> $ git push ;# we default to matching!
>
> I.e., the user is not actually intending to push all matching branches,
> but does so accidentally because "matching" is the default. And they
> have accidentally just pushed non-ready work to topic1, which would
> happen even with --ignore-stale.
>
> IOW, I am not convinced that people are actually consciously choosing
> the workflow you outlined above, and are instead interested in
> --ignore-stale as a convenient way of guessing which branches should be
> pushed during a matching push. But it's only a guess, and as I showed
> above, it can still cause problems. I think the right solution is to
> switch the default away from matching, which is the root cause of the
> confusion.
100% agree.
In my experience, people are confused by any magic that happens when a local
ref name matches a remote's ref name. I find folks have a much easier time
with git once they embrace the notion that a remote's branches are distinct
from their local clone's, and that explicitly saying
push origin <local>:<remote>
is the clearest way of making sure git does what you want.
It's convenient for advanced users to take advantage of the shortcuts any
ref-matching magic provides, but I find novice users tend to make mistakes as
often as not.
M.
^ permalink raw reply
* Re: [BUG] in rev-parse
From: Junio C Hamano @ 2011-12-15 17:43 UTC (permalink / raw)
To: Jeff King; +Cc: nathan.panike, git
In-Reply-To: <20111215070521.GB1327@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Dropping the implied verify is easy (see below). But handling
> non-existant sha1s is a much more complicated change, as the regular
> abbreviation machinery assumes that they exist.
Hmm, I was very sure that I wrote a logic to do the "no such object" case
at least once, so I looked and found one in find_unique_abbrev(). But that
requires the caller to feed the full 160-bit sha1[] and perhaps it is not
easily used in the context of rev-parse (I didn't look further).
> Anyway, I'm not sure it's worth changing at this point. It's part of the
> plumbing API that has been that way forever,...
Ok.
^ permalink raw reply
* Re: [PATCH v2 3/4] wrapper: supply xsetenv and xputenv
From: Junio C Hamano @ 2011-12-15 17:38 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, peff, schwab
In-Reply-To: <1323871631-2872-4-git-send-email-kusmabite@gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> +extern int xsetenv(const char *name, const char *val, int override);
> +extern int xputenv(const char *string);
Actually, putenv(3) takes "char *string".
I adjusted 3 & 4 locally before queuing, so there is no need for resend,
and judging from the later part of the discussion, it seems that it may be
better to use only the first two patches from this series after all.
^ permalink raw reply
* Re: [bug?] checkout -m doesn't work without a base version
From: Junio C Hamano @ 2011-12-15 17:36 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Michael Schubert, Pete Harlan, git
In-Reply-To: <m2vcpiw1z1.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> The variable "mode" is assigned to when we see an stage #2 entry in the
>> loop, and we should have updated threeway[1] immediately before doing so.
>> If threeway[1] is not updated, we would have already returned before using
>> the variable in make_cache_entry().
>
> How can you be sure that ce_stage(ce) ever returns 2?
You cannot and and there are cases where you exit the loop without finding
a stage #2 entry. But in that case threeway[1] stays 0{40} and control is
returned to the caller without ever getting to the place where the
variable is used.
You could do the usual "unnecessary initialization" trick, though.
builtin/checkout.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 31aa248..064e7a1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -152,7 +152,7 @@ static int checkout_merged(int pos, struct checkout *state)
unsigned char sha1[20];
mmbuffer_t result_buf;
unsigned char threeway[3][20];
- unsigned mode;
+ unsigned mode = 0;
memset(threeway, 0, sizeof(threeway));
while (pos < active_nr) {
^ permalink raw reply related
* [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Ramkumar Ramachandra @ 2011-12-15 16:45 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-1-git-send-email-artagnon@gmail.com>
The git-bundle builtin currently parses command-line options by hand;
this is fragile, and reports cryptic errors on failure. Use the
parse-options library to do the parsing instead.
Encouraged-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/bundle.c | 91 +++++++++++++++++++++++++++++++----------------------
t/t5704-bundle.sh | 2 +-
2 files changed, 54 insertions(+), 39 deletions(-)
diff --git a/builtin/bundle.c b/builtin/bundle.c
index 92a8a60..13ed770 100644
--- a/builtin/bundle.c
+++ b/builtin/bundle.c
@@ -1,5 +1,6 @@
#include "builtin.h"
#include "cache.h"
+#include "parse-options.h"
#include "bundle.h"
/*
@@ -9,57 +10,71 @@
* bundle supporting "fetch", "pull", and "ls-remote".
*/
-static const char builtin_bundle_usage[] =
- "git bundle create <file> <git-rev-list args>\n"
- " or: git bundle verify <file>\n"
- " or: git bundle list-heads <file> [<refname>...]\n"
- " or: git bundle unbundle <file> [<refname>...]";
+static const char * builtin_bundle_usage[] = {
+ "git bundle create <file> <git-rev-list args>",
+ "git bundle verify <file>",
+ "git bundle list-heads <file> [<refname>...]",
+ "git bundle unbundle <file> [<refname>...]",
+ NULL
+};
int cmd_bundle(int argc, const char **argv, const char *prefix)
{
- struct bundle_header header;
- const char *cmd, *bundle_file;
+ int prefix_length;
int bundle_fd = -1;
- char buffer[PATH_MAX];
+ const char *subcommand, *bundle_file;
+ struct bundle_header header;
+ struct option options[] = { OPT_END() };
- if (argc < 3)
- usage(builtin_bundle_usage);
+ argc = parse_options(argc, argv, prefix, options,
+ builtin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);
- cmd = argv[1];
- bundle_file = argv[2];
- argc -= 2;
- argv += 2;
+ if (argc < 2)
+ usage_with_options(builtin_bundle_usage, options);
+ subcommand = argv[0];
- if (prefix && bundle_file[0] != '/') {
- snprintf(buffer, sizeof(buffer), "%s/%s", prefix, bundle_file);
- bundle_file = buffer;
- }
+ argc = parse_options(argc, argv, prefix, options,
+ builtin_bundle_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+ /* Disallow stray arguments */
+ if ((strcmp(subcommand, "create") && argc > 2) ||
+ (!strcmp(subcommand, "verify") && argc > 1))
+ usage_with_options(builtin_bundle_usage, options);
- memset(&header, 0, sizeof(header));
- if (strcmp(cmd, "create") && (bundle_fd =
- read_bundle_header(bundle_file, &header)) < 0)
- return 1;
+ prefix_length = prefix ? strlen(prefix) : 0;
+ bundle_file = prefix_filename(prefix, prefix_length, argv[0]);
- if (!strcmp(cmd, "verify")) {
+ /* Read out bundle header, except in the "create" case */
+ if (strcmp(subcommand, "create")) {
+ memset(&header, 0, sizeof(header));
+ bundle_fd = read_bundle_header(bundle_file, &header);
+ if (bundle_fd < 0)
+ die_errno(_("Failed to open bundle file '%s'"), bundle_file);
+ }
+
+ if (!strcmp(subcommand, "create")) {
+ if (!startup_info->have_repository)
+ die(_("Need a repository to create a bundle."));
+ return create_bundle(&header, bundle_file, argc, argv);
+ } else if (!strcmp(subcommand, "verify")) {
close(bundle_fd);
if (verify_bundle(&header, 1))
- return 1;
+ return -1; /* Error already reported */
fprintf(stderr, _("%s is okay\n"), bundle_file);
- return 0;
- }
- if (!strcmp(cmd, "list-heads")) {
+ } else if (!strcmp(subcommand, "list-heads")) {
close(bundle_fd);
- return !!list_bundle_refs(&header, argc, argv);
- }
- if (!strcmp(cmd, "create")) {
- if (!startup_info->have_repository)
- die(_("Need a repository to create a bundle."));
- return !!create_bundle(&header, bundle_file, argc, argv);
- } else if (!strcmp(cmd, "unbundle")) {
- if (!startup_info->have_repository)
+ return list_bundle_refs(&header, argc, argv);
+ } else if (!strcmp(subcommand, "unbundle")) {
+ if (!startup_info->have_repository) {
+ close(bundle_fd);
die(_("Need a repository to unbundle."));
- return !!unbundle(&header, bundle_fd, 0) ||
+ }
+ return unbundle(&header, bundle_fd, 0) ||
list_bundle_refs(&header, argc, argv);
- } else
- usage(builtin_bundle_usage);
+ } else {
+ close(bundle_fd);
+ usage_with_options(builtin_bundle_usage, options);
+ }
+
+ return 0;
}
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 09ff4f1..8e3f677 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -53,7 +53,7 @@ test_expect_success 'disallow stray command-line options' '
test_must_fail git bundle create --junk bundle second third
'
-test_expect_failure 'disallow stray command-line arguments' '
+test_expect_success 'disallow stray command-line arguments' '
git bundle create bundle second third &&
test_must_fail git bundle verify bundle junk
'
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2] t5704 (bundle): rewrite for larger coverage
From: Ramkumar Ramachandra @ 2011-12-15 16:45 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano, Joey Hess
In-Reply-To: <1323967528-10537-1-git-send-email-artagnon@gmail.com>
Rewrite the git-bundle testsuite to exercise more of its
functionality.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t5704-bundle.sh | 95 ++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 69 insertions(+), 26 deletions(-)
diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 728ccd8..09ff4f1 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -1,56 +1,99 @@
#!/bin/sh
-test_description='some bundle related tests'
+test_description='Test git-bundle'
. ./test-lib.sh
test_expect_success 'setup' '
+ git config core.logAllRefUpdates false &&
+ test_commit initial &&
+ git checkout -b branch &&
+ test_commit second &&
+ test_commit third &&
+ git checkout master &&
+ test_commit fourth file
+'
- : > file &&
- git add file &&
- test_tick &&
- git commit -m initial &&
- test_tick &&
- git tag -m tag tag &&
- : > file2 &&
- git add file2 &&
- : > file3 &&
- test_tick &&
- git commit -m second &&
- git add file3 &&
- test_tick &&
- git commit -m third
+test_expect_success 'create succeeds' '
+ git bundle create bundle second third &&
+ cat >expect <<-\EOF &&
+ OBJID refs/tags/third
+ OBJID refs/tags/second
+ EOF
+ {
+ git ls-remote bundle |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ test_cmp expect actual
+'
+test_expect_success 'verify succeeds' '
+ git bundle create bundle second third &&
+ git bundle verify bundle
'
-test_expect_success 'tags can be excluded by rev-list options' '
+test_expect_success 'list-heads succeeds' '
+ git bundle create bundle second third &&
+ cat >expect <<-\EOF &&
+ OBJID refs/tags/second
+ OBJID refs/tags/third
+ EOF
+ {
+ git bundle list-heads bundle |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ test_cmp expect actual
+'
- git bundle create bundle --all --since=7.Apr.2005.15:16:00.-0700 &&
- git ls-remote bundle > output &&
- ! grep tag output
+test_expect_success 'create dies on invalid bundle filename' '
+ mkdir adir &&
+ test_expect_code 128 git bundle create adir --all
+'
+test_expect_success 'disallow stray command-line options' '
+ test_must_fail git bundle create --junk bundle second third
'
-test_expect_success 'die if bundle file cannot be created' '
+test_expect_failure 'disallow stray command-line arguments' '
+ git bundle create bundle second third &&
+ test_must_fail git bundle verify bundle junk
+'
- mkdir adir &&
- test_must_fail git bundle create adir --all
+test_expect_success 'create accepts rev-list options to narrow refs' '
+ git bundle create bundle --all -- file &&
+ cat >expect <<-\EOF &&
+ OBJID HEAD
+ OBJID refs/tags/fourth
+ OBJID refs/heads/master
+ EOF
+ {
+ git ls-remote bundle |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ test_cmp expect actual
+'
+test_expect_success 'unbundle succeeds' '
+ saved=$(git rev-parse third) &&
+ git bundle create bundle second third fourth &&
+ git tag -d second third fourth &&
+ git branch -D branch &&
+ git reset --hard initial &&
+ git prune &&
+ test_must_fail git reset --hard $saved &&
+ git bundle unbundle bundle &&
+ git reset --hard $saved
'
test_expect_failure 'bundle --stdin' '
-
echo master | git bundle create stdin-bundle.bdl --stdin &&
git ls-remote stdin-bundle.bdl >output &&
grep master output
-
'
test_expect_failure 'bundle --stdin <rev-list options>' '
-
echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
git ls-remote hybrid-bundle.bdl >output &&
grep master output
-
'
test_done
--
1.7.4.1
^ 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