* Re: [PATCH v2] gitk: read and write a repository specific configuration file
From: Marc Branchaud @ 2012-12-05 15:20 UTC (permalink / raw)
To: Łukasz Stelmach; +Cc: git, paulus, gitster
In-Reply-To: <1354668583-4893-1-git-send-email-stlman@poczta.fm>
On 12-12-04 07:49 PM, Łukasz Stelmach wrote:
> Enable gitk read and write repository specific configuration
> file: ".git/k" if the file exists. To make gitk use the local
> file simply create one, e.g. with the touch(1) command.
>
> This is very useful if one uses different views for different
> repositories. Now there is no need to store all of them in
> ~/.gitk and make the views list needlesly long.
s/needlesly/needlessly/
M.
^ permalink raw reply
* Re: [PATCH] mingw_rmdir: do not prompt for retry when non-empty
From: Erik Faye-Lund @ 2012-12-05 15:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, msysgit
In-Reply-To: <alpine.DEB.1.00.1212041728210.31987@s15462909.onlinehome-server.info>
Sorry for a late reply.
On Tue, Dec 4, 2012 at 5:35 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Tue, 4 Dec 2012, Erik Faye-Lund wrote:
>
>> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
>> a check was added to prevent us from retrying to delete a directory
>> that is both in use and non-empty.
>>
>> However, this logic was slightly flawed; since we didn't return
>> immediately, we end up falling out of the retry-loop, but right into
>> the prompting loop.
>>
>> Fix this by simply returning from the function instead of breaking
>> the loop.
>>
>> While we're at it, change the second break to a return as well; we
>> already know that we won't enter the prompting-loop, beacuse
>> is_file_in_use_error(GetLastError()) already evaluated to false.
>
> I usually prefer to break from the loop, to be able to add whatever
> cleanup code we might need in the future after the loop.
>
> So does this fix the problem for you?
>
> -- snipsnap --
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 04af3dc..504495a 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -259,7 +259,8 @@ int mingw_rmdir(const char *pathname)
> return -1;
>
> while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
> - if (!is_file_in_use_error(GetLastError()))
> + errno = err_win_to_posix(GetLastError());
> + if (errno != EACCESS)
> break;
> if (!is_dir_empty(wpathname)) {
> errno = ENOTEMPTY;
> @@ -275,7 +276,7 @@ int mingw_rmdir(const char *pathname)
> Sleep(delay[tries]);
> tries++;
> }
> - while (ret == -1 && is_file_in_use_error(GetLastError()) &&
> + while (ret == -1 && errno == EACCESS &&
> ask_yes_no_if_possible("Deletion of directory '%s' failed. "
> "Should I try again?", pathname))
> ret = _wrmdir(wpathname);
Yes, as long as you do s/EACCESS/EACCES/ first. I don't mind such a
version instead.
^ permalink raw reply
* Re: remote-testsvn: Hangs at revision
From: Ramkumar Ramachandra @ 2012-12-05 13:57 UTC (permalink / raw)
To: David Michael Barr; +Cc: Git List, Florian Achleitner
In-Reply-To: <D435D46EAB6F419CA0608075751C351C@rr-dav.id.au>
David Michael Barr wrote:
> On Wednesday, 5 December 2012 at 5:20 PM, Ramkumar Ramachandra wrote:
>> $ git clone testsvn::http://python-lastfm.googlecode.com/svn/trunk/
> I attempted to clone the same repository and was able to fetch 152 revisions.
> So the issue Ram saw might have been temporal.
Nope, I'm able to reproduce it everytime. I wonder what's going on.
Ram
^ permalink raw reply
* Re: remote-testsvn: Hangs at revision
From: David Michael Barr @ 2012-12-05 13:28 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Florian Achleitner
In-Reply-To: <CALkWK0meveeKQe81hHyojPX0GH_WRrv7ob9-NA1Q7-TuKso+1w@mail.gmail.com>
On Wednesday, 5 December 2012 at 5:20 PM, Ramkumar Ramachandra wrote:
> Hi,
>
> I tried out the testsvn remote helper on a simple Subversion
> repository, but it seems to hang at Revision 8 indefinitely without
> any indication of progress. I'm currently digging in to see what went
> wrong. The repository I'm cloning is:
>
> $ git clone testsvn::http://python-lastfm.googlecode.com/svn/trunk/
I attempted to clone the same repository and was able to fetch 152 revisions.
So the issue Ram saw might have been temporal.
I did however receive a warning at the end of the clone:
warning: remote HEAD refers to nonexistent ref, unable to checkout.
--
David Michael Barr
^ permalink raw reply
* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Martin von Zweigbergk @ 2012-12-05 12:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtxs1kq4z.fsf@alter.siamese.dyndns.org>
On Tue, Dec 4, 2012 at 9:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> More importantly, when is it desirable not to delete deleted entries?
>
> When I am trying to check out contents of Documentation/ directory
> as of an older edition because we made mistakes updating the files
> in recent versions, with "git checkout v1.9.0 Documentation/", for
> example. Perhaps somebody had this bright idea of reformatting our
> docs with "= Newer Style =" section headers, replacing the underline
> style, and we found our toolchain depend on the underline style, or
> something. The new files in the same directory added since v1.9.0
> may share the same mistake as the files whose recent such changes I
> am nuking with this operation, but that does not mean I want to
> retype the contents of them from scratch; I'd rather keep them
> around so that I can fix them up by hand.
I think I follow, but why, then, would you not have the save problem
with hunks *within* files that have been added in the new version? Or
is the only change to Documentation/ since the "broken" commit that a
new file has been added? That seems like a rather narrow use case in
that case? "git checkout -p" seems more generally useful (whether that
command deleted deleted files or not). It feels like I'm missing
something...
> I would have to say that it is more common; I do not recall a time I
> wanted to replace everything in a directory (and only there without
> touching other parts of the tree) with an old version, removing new
> ones.
It has happened a few times for me. I think this usually happens when
I realize that I had a better solution for some subsystem (under some
path) in some other branch (perhaps from a previous attempt at the
same problem) or an in older commit. Knowing that "git checkout $rev
$path" doesn't do what I expect and that "git reset --hard $rev $path"
is not allowed, I think I would usually do "git reset $rev $path &&
git checkout $path".
^ permalink raw reply
* [PATCH v3 4/4] git-svn: Note about tags.
From: Sebastian Leske @ 2012-11-23 7:29 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Junio C Hamano
In-Reply-To: <cover.1354693001.git.Sebastian.Leske@sleske.name>
Document that 'git svn' will import SVN tags as branches.
Signed-off-by: Sebastian Leske <sebastian.leske@sleske.name>
---
Documentation/git-svn.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 021fb0e..445b033 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -968,6 +968,12 @@ the possible corner cases (git doesn't do it, either). Committing
renamed and copied files is fully supported if they're similar enough
for git to detect them.
+In SVN, it is possible (though discouraged) to commit changes to a tag
+(because a tag is just a directory copy, thus technically the same as a
+branch). When cloning an SVN repository, 'git svn' cannot know if such a
+commit to a tag will happen in the future. Thus it acts conservatively
+and imports all SVN tags as branches, prefixing the tag name with 'tags/'.
+
CONFIGURATION
-------------
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 3/4] git-svn: Expand documentation for --follow-parent
From: Sebastian Leske @ 2012-11-30 7:16 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Junio C Hamano
In-Reply-To: <cover.1354693001.git.Sebastian.Leske@sleske.name>
Describe what the option --follow-parent does, and what happens if it is
set or unset.
Signed-off-by: Sebastian Leske <sebastian.leske@sleske.name>
---
Documentation/git-svn.txt | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index d8e5082..021fb0e 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -628,10 +628,19 @@ ADVANCED OPTIONS
Default: "svn"
--follow-parent::
+ This option is only relevant if we are tracking branches (using
+ one of the repository layout options --trunk, --tags,
+ --branches, --stdlayout). For each tracked branch, try to find
+ out where its revision was copied from, and set
+ a suitable parent in the first git commit for the branch.
This is especially helpful when we're tracking a directory
- that has been moved around within the repository, or if we
- started tracking a branch and never tracked the trunk it was
- descended from. This feature is enabled by default, use
+ that has been moved around within the repository. If this
+ feature is disabled, the branches created by 'git svn' will all
+ be linear and not share any history, meaning that there will be
+ no information on where branches were branched off or merged.
+ However, following long/convoluted histories can take a long
+ time, so disabling this feature may speed up the cloning
+ process. This feature is enabled by default, use
--no-follow-parent to disable it.
+
[verse]
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 2/4] git-svn: Recommend use of structure options.
From: Sebastian Leske @ 2012-11-30 7:16 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Junio C Hamano
In-Reply-To: <cover.1354693001.git.Sebastian.Leske@sleske.name>
Document that when using git svn, one should usually either use the
directory structure options to import branches as branches, or only
import one subdirectory. The default behaviour of cloning all branches
and tags as subdirectories in the working copy is usually not what the
user wants.
Signed-off-by: Sebastian Leske <sebastian.leske@sleske.name>
---
Documentation/git-svn.txt | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 55bed53..d8e5082 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -739,7 +739,8 @@ for rewriteRoot and rewriteUUID which can be used together.
BASIC EXAMPLES
--------------
-Tracking and contributing to the trunk of a Subversion-managed project:
+Tracking and contributing to the trunk of a Subversion-managed project
+(ignoring tags and branches):
------------------------------------------------------------------------
# Clone a repo (like git clone):
@@ -764,8 +765,10 @@ Tracking and contributing to an entire Subversion-managed project
(complete with a trunk, tags and branches):
------------------------------------------------------------------------
-# Clone a repo (like git clone):
- git svn clone http://svn.example.com/project -T trunk -b branches -t tags
+# Clone a repo with standard SVN directory layout (like git clone):
+ git svn clone http://svn.example.com/project --stdlayout
+# Or, if the repo uses a non-standard directory layout:
+ git svn clone http://svn.example.com/project -T tr -b branch -t tag
# View all branches and tags you have cloned:
git branch -r
# Create a new branch in SVN
@@ -918,6 +921,21 @@ already dcommitted. It is considered bad practice to --amend commits
you've already pushed to a remote repository for other users, and
dcommit with SVN is analogous to that.
+When cloning an SVN repository, if none of the options for describing
+the repository layout is used (--trunk, --tags, --branches,
+--stdlayout), 'git svn clone' will create a git repository with
+completely linear history, where branches and tags appear as separate
+directories in the working copy. While this is the easiest way to get a
+copy of a complete repository, for projects with many branches it will
+lead to a working copy many times larger than just the trunk. Thus for
+projects using the standard directory structure (trunk/branches/tags),
+it is recommended to clone with option '--stdlayout'. If the project
+uses a non-standard structure, and/or if branches and tags are not
+required, it is easiest to only clone one directory (typically trunk),
+without giving any repository layout options. If the full history with
+branches and tags is required, the options '--trunk' / '--branches' /
+'--tags' must be used.
+
When using multiple --branches or --tags, 'git svn' does not automatically
handle name collisions (for example, if two branches from different paths have
the same name, or if a branch and a tag have the same name). In these cases,
--
1.7.10.4
^ permalink raw reply related
* [PATCH v3 0/4] git-svn: More docs for branch handling
From: Sebastian Leske @ 2012-12-05 7:36 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Junio C Hamano
Updated version of my documentation patch for git-svn. Thanks to Junio C
Hamano for pointing out improvements.
Sebastian Leske (4):
git-svn: Document branches with at-sign(@).
git-svn: Recommend use of structure options.
git-svn: Expand documentation for --follow-parent
git-svn: Note about tags.
Documentation/git-svn.txt | 92 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 86 insertions(+), 6 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH v3 1/4] git-svn: Document branches with at-sign(@).
From: Sebastian Leske @ 2012-11-30 7:16 UTC (permalink / raw)
To: git; +Cc: Eric Wong, Junio C Hamano
In-Reply-To: <cover.1354693001.git.Sebastian.Leske@sleske.name>
git svn sometimes creates branches with an at-sign in the name
(branchname@revision). These branches confuse many users and it is a FAQ
why they are created. Document when git svn creates them.
Signed-off-by: Sebastian Leske <sebastian.leske@sleske.name>
---
Documentation/git-svn.txt | 47 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 8b0d3ad..55bed53 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -830,6 +830,53 @@ inside git back upstream to SVN users. Therefore it is advised that
users keep history as linear as possible inside git to ease
compatibility with SVN (see the CAVEATS section below).
+HANDLING OF SVN BRANCHES
+------------------------
+If 'git svn' is configured to fetch branches (and --follow-branches
+is in effect), it will sometimes create multiple git branches for one
+SVN branch, where the addtional branches have names of the form
+'branchname@nnn' (with nnn an SVN revision number). These additional
+branches are created if 'git svn' cannot find a parent commit for the
+first commit in an SVN branch, to connect the branch to the history of
+the other branches.
+
+Normally, the first commit in an SVN branch consists
+of a copy operation. 'git svn' will read this commit to get the SVN
+revision the branch was created (copied) from. It will then try to find the
+git commit that corresponds to this SVN revision, and use that as the
+parent of the branch. However, it is possible that there is no suitable
+git commit to serve as parent. This will happen, among other reasons,
+if the SVN branch is a copy of a revision that was not fetched by 'git
+svn' (e.g. because it is an old revision that was skipped with
+'--revision'), or if in SVN a directory was copied that is not tracked
+by 'git svn' (such as a branch that is not tracked at all, or a
+subdirectory of a tracked branch). In these cases, 'git svn' will still
+create a git branch, but instead of using an existing git commit as the
+parent of the branch, it will read the SVN history of the directory the
+branch was copied from and create appropriate git commits (this is
+indicated by the message "Initializing parent: <branchname>").
+
+Additionally, it will create a special branch named
+'<branchname>@<SVN-Revision>', where <SVN-Revision> is the SVN revision
+number the branch was copied from. This branch will point to the newly
+created parent commit of the branch. If in SVN the branch was deleted
+and later recreated from a different version, there will be multiple
+such branches with an '@'.
+
+Note that this may mean that multiple git commits are created for a
+single SVN revision.
+
+An example: In an SVN repository with a standard
+trunk/tags/branches layout, a directory trunk/sub is created in r.100.
+In r.200, trunk/sub is branched by copying it to branches/. 'git svn
+clone -s' will then create a branch 'sub'. It will also create new git
+commits for r.100 through r.199 and use these as the history of branch
+'sub'. Thus there will be two git commits for each revision from r.100
+to r.199 (one containing trunk/, one containing trunk/sub/). Finally,
+it will create a branch 'sub@200' pointing to the new parent commit of
+branch 'sub' (i.e. the commit for r.200 and trunk/sub/).
+
CAVEATS
-------
--
1.7.10.4
^ permalink raw reply related
* Re: Exploiting SHA1's "XOR weakness" allows for faster hash calculation
From: Marko Kreen @ 2012-12-05 12:26 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: git
In-Reply-To: <k9n3jd$akg$1@ger.gmane.org>
On Wed, Dec 5, 2012 at 11:19 AM, Sebastian Schuberth
<sschuberth@gmail.com> wrote:
> to say it in advance: I do not want to trigger any bogus security discussion
> here. Instead, I believe the findings from [1] allow for an up to 20% faster
> SHA1 calculation, if my brief reading of the presentation is correct. Any
> opinions on integration this optimization into Git?
>
> [1] https://hashcat.net/p12/js-sha1exp_169.pdf
Pretty cool find. Although it's not actual cryptographic weakness, it does
show some gaps in designers thinking - as there are simple optimizations
available to crackers but not users.
It does seem unusable for real implementation - the 20% win
is available only after the data is processed properly once.
Then after changing the data a little, you can calculate next
hash faster.
There still small possibility that there is way to optimize W calculation
for the first run, but it does seem really hard, and even impossible
while trying to keep the cache usage small.
--
marko
^ permalink raw reply
* Exploiting SHA1's "XOR weakness" allows for faster hash calculation
From: Sebastian Schuberth @ 2012-12-05 9:19 UTC (permalink / raw)
To: git
Hi,
to say it in advance: I do not want to trigger any bogus security
discussion here. Instead, I believe the findings from [1] allow for an
up to 20% faster SHA1 calculation, if my brief reading of the
presentation is correct. Any opinions on integration this optimization
into Git?
[1] https://hashcat.net/p12/js-sha1exp_169.pdf
--
Sebastian Schuberth
^ permalink raw reply
* Re: [RFC] Add basic syntax check on shell scripts
From: Sebastian Schuberth @ 2012-12-05 9:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vobi9mwt4.fsf@alter.siamese.dyndns.org>
On 2012/12/04 20:39 , Junio C Hamano wrote:
> A few more things in addition to what Torsten's script attempts to
> catch that we would want to catch are:
[...]
> * Do not write ERE with backslashes and expect "grep" to grok them;
> that's GNUism. e.g.
>
> grep "^\(author\|committer\) "
>
> is bad. Use egrep (or "grep -E") if you want to use ERE.
Yet more thing that is probably worth catching, although not related to
bashism: Avoid the use of "which" in favor of e.g. "type".
In any case, having this check as a local pre-commit hook would be great!
--
Sebastian Schuberth
^ permalink raw reply
* Re: [RFC] Add basic syntax check on shell scripts
From: Jeff King @ 2012-12-05 7:54 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, Torsten Bögershausen, git
In-Reply-To: <20121205073055.GA5776@sigill.intra.peff.net>
On Wed, Dec 05, 2012 at 02:30:56AM -0500, Jeff King wrote:
> Anyway, I do think a "shell portability lint" would be a great addition
> to "test-lint", but I am slightly skeptical that it will be easy to
> write a good one that does not have false positives. Still, there may be
> some low-hanging fruit. I have not looked carefully at Torsten's patch
> yet.
Hrm. I had the impression initially that Torsten's patch was about
testing the test scripts themselves. But it is really about testing the
installed shell scripts. In that sense, test-lint is not the right
place.
You would want a "check shell script portability" script, and you would
probably want to run it:
- on the regular built scripts; possibly during build time (I have done
this before with "perl -c" for perl scripts and it is reasonably
successful). Or in a test script, as added in his patch (though I
note it does not seem to pass as posted, getting confused by trying
to grep "git-gui").
- on the test scripts themselves via test-lint
I think as long as such a script erred on the side of false negatives,
it would be OK (because false positives are a giant headache, and
ultimately the real test is people exercising the code itself on their
shells; this is just an early check to help contributors who do not have
such shells).
-Peff
PS Debian developers use a checkbashisms script to find some portability
problems. It might be worth looking at, though I notice it generates
a lot of bogus "unterminated string" results for our t/t*.sh scripts.
^ permalink raw reply
* Re: [RFC] Add basic syntax check on shell scripts
From: Jeff King @ 2012-12-05 7:30 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, Torsten Bögershausen, git
In-Reply-To: <CACsJy8BtX9fMkGDoVGKzgz7SSinbt0561B1ZKHu6fs+n8ewKGg@mail.gmail.com>
On Wed, Dec 05, 2012 at 12:43:30PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Wed, Dec 5, 2012 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Or a project commit hook?
> >
> > Surely. It is OK to have "cd t && make test-lint" in your
> > pre-commit hook.
>
> No, what I meant is a shared pre-commit script that all git devs are
> encouraged (or forced) to install so bugs are found locally rather
> than after patches are sent to you. The hook content does not really
> matter.
I think that is orthogonal. You would want to implement the guts of such
a hook outside the hook itself, so that it could be run at arbitrary
times. So even if we want such a hook, the development should probably
look like:
1. Implement checks in t/Makefile, triggered by "make test-lint" or
similar.
2. Run "make test-lint" in a hook.
I do not use such a hook myself, but I do run "test-lint" as part of my
"make test", and I "make test" each series I send (and if the series has
non-trivial refactoring, each individual patch of the series to catch
breakages that come and go during refactoring). But I decide when to run
those checks, not a hook.
Anyway, I do think a "shell portability lint" would be a great addition
to "test-lint", but I am slightly skeptical that it will be easy to
write a good one that does not have false positives. Still, there may be
some low-hanging fruit. I have not looked carefully at Torsten's patch
yet.
-Peff
^ permalink raw reply
* remote-testsvn: Hangs at revision
From: Ramkumar Ramachandra @ 2012-12-05 6:20 UTC (permalink / raw)
To: Git List; +Cc: Florian Achleitner, David Barr
Hi,
I tried out the testsvn remote helper on a simple Subversion
repository, but it seems to hang at Revision 8 indefinitely without
any indication of progress. I'm currently digging in to see what went
wrong. The repository I'm cloning is:
$ git clone testsvn::http://python-lastfm.googlecode.com/svn/trunk/
Thanks.
Ram
^ permalink raw reply
* Re: [RFC] Add basic syntax check on shell scripts
From: Junio C Hamano @ 2012-12-05 6:02 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Torsten Bögershausen, git
In-Reply-To: <CACsJy8BtX9fMkGDoVGKzgz7SSinbt0561B1ZKHu6fs+n8ewKGg@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Wed, Dec 5, 2012 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Or a project commit hook?
>>
>> Surely. It is OK to have "cd t && make test-lint" in your
>> pre-commit hook.
>
> No, what I meant is a shared pre-commit script that all git devs are
> encouraged (or forced) to install so bugs are found locally rather
> than after patches are sent to you. The hook content does not really
> matter.
Honestly, I do not really care (yet); what you are talkng about is
merely an addition to Documentation/SubmittingPatches, which is
trivial.
The content of the hook is much more important.
If it has too many false positives, it is not worth even encouraging
its use to less experienced ones, as they will have hard time to
figure out which errors matter and which erros can be ignored. It
will make contributing to the project harder, not easier.
As I do not think (1) we would be able to do a good job reducing
false positives without writing a full POSIX shell parser, and (2)
we would want to be in the business of writing POSIX shell parser
[*1*], I am somewhat skeptical.
And if we cannot come up with a reliable hook in the first place,
there isn't much point in discussing a policy to encourage such a
hook, is it?
[Footnote]
*1* Is there a free one that is portable, perhaps written in either
Perl or Python, to which we can easily plug our own logic? In order
to catch basic errors, I think it is sufficient to tokenize the
script into independent series of simple commands, even ignoring
variable substitutions and evals, and just have a bunch of "we do
not allow option X to command Y" rules (e.g. "no -i to sed", "the
first argument must be 'git' for "test_must_fail").
^ permalink raw reply
* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Junio C Hamano @ 2012-12-05 5:46 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <CANiSa6iMxzQGM8mZYdfR-drPGgydwVpM5JsQ-8oO09MX5XDH+g@mail.gmail.com>
Martin von Zweigbergk <martinvonz@gmail.com> writes:
> More importantly, when is it desirable not to delete deleted entries?
When I am trying to check out contents of Documentation/ directory
as of an older edition because we made mistakes updating the files
in recent versions, with "git checkout v1.9.0 Documentation/", for
example. Perhaps somebody had this bright idea of reformatting our
docs with "= Newer Style =" section headers, replacing the underline
style, and we found our toolchain depend on the underline style, or
something. The new files in the same directory added since v1.9.0
may share the same mistake as the files whose recent such changes I
am nuking with this operation, but that does not mean I want to
retype the contents of them from scratch; I'd rather keep them
around so that I can fix them up by hand.
I would have to say that it is more common; I do not recall a time I
wanted to replace everything in a directory (and only there without
touching other parts of the tree) with an old version, removing new
ones. "git checkout [$commit] $paths" is still an operation to help
me build new history forward starting from HEAD, and is not about
start building on top of the old $commit. Losing the work I've done
to the files that did not exist in $commit:$paths is almost always
*not* what I would expect to happen with the command.
^ permalink raw reply
* Re: [RFC] Add basic syntax check on shell scripts
From: Nguyen Thai Ngoc Duy @ 2012-12-05 5:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vobi9mwt4.fsf@alter.siamese.dyndns.org>
On Wed, Dec 5, 2012 at 2:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Or a project commit hook?
>
> Surely. It is OK to have "cd t && make test-lint" in your
> pre-commit hook.
No, what I meant is a shared pre-commit script that all git devs are
encouraged (or forced) to install so bugs are found locally rather
than after patches are sent to you. The hook content does not really
matter.
--
Duy
^ permalink raw reply
* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Martin von Zweigbergk @ 2012-12-05 3:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2yunn0e.fsf@alter.siamese.dyndns.org>
On Sat, Dec 1, 2012 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>> Slightly off topic, but another difference (or somehow another aspect
>>> of the same difference?) that has tripped me up a few times is that
>>> "git checkout $rev ." only affects added and modified files...
>
> "checkout $commit pathspec" has always been about ...
I suppose the "has always been" is meant to say that it's hard to
change at this point, not that it's more intuitive the way it works..?
> ...checking out the
> contents stored in the paths that match the pathspec from the named
> commit to the index and also o the working tree.
I think I have always thought that "git checkout $commit $pathspec"
would replace the section(s) of the tree defined by $pathspec. (I'm
using "tree" in the more general sense here, as I'm understood the
index is not stored as a tree.)
> When pathspec is "dir/", it does not match the directory whose name
> is "dir". The pathspec matches the paths that store blobs under
> that directory.
Ah, right. Unlike "git reset dir/", IIUC.
More importantly, when is it desirable not to delete deleted entries?
I find it much easier to imagine uses a "git checkout $commit
$pathspec" that does delete deleted entries. It seems like this must
have been discussed in depth before, so feel free to point me to an
old thread.
If it doesn't seem too strange to you and others if I make "git reset
--hard [$commit] $pathspec" work just like had expected "git checkout
$commit $pathspec", I might look into that when I get some time.
> ...The "please
> remove everything in dir/" part is not the job of "checkout"; of
> course, you can do it as a separate step (e.g. "rm -fr dir/").
"rm -rf dir/" would of course delete everything in there, including
e.g. build artifacts....
^ permalink raw reply
* [PATCH v2] gitk: read and write a repository specific configuration file
From: Łukasz Stelmach @ 2012-12-05 0:49 UTC (permalink / raw)
To: git; +Cc: paulus, gitster, Łukasz Stelmach
In-Reply-To: <1354483766-13925-1-git-send-email-stlman@poczta.fm>
Enable gitk read and write repository specific configuration
file: ".git/k" if the file exists. To make gitk use the local
file simply create one, e.g. with the touch(1) command.
This is very useful if one uses different views for different
repositories. Now there is no need to store all of them in
~/.gitk and make the views list needlesly long.
Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
Same as before but rebased onto Paul's repository.
gitk | 25 ++++++++++++++-----------
1 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/gitk b/gitk
index 379582a..c6b7dc3 100755
--- a/gitk
+++ b/gitk
@@ -2703,7 +2703,7 @@ proc doprogupdate {} {
proc savestuff {w} {
global canv canv2 canv3 mainfont textfont uifont tabstop
- global stuffsaved findmergefiles maxgraphpct
+ global stuffsaved findmergefiles maxgraphpct gitdir
global maxwidth showneartags showlocalchanges
global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
global cmitmode wrapcomment datetimeformat limitdiffs
@@ -2714,10 +2714,12 @@ proc savestuff {w} {
if {$stuffsaved} return
if {![winfo viewable .]} return
catch {
- if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
- set f [open "~/.gitk-new" w]
+ set fn [expr [file exists [file join $gitdir k]] ? \
+ {[file join $gitdir k-new]} : {"~/.gitk-new"}]
+ if {[file exists $fn]} {file delete -force $fn}
+ set f [open $fn w]
if {$::tcl_platform(platform) eq {windows}} {
- file attributes "~/.gitk-new" -hidden true
+ catch {file attributes "~/.gitk-new" -hidden true}
}
puts $f [list set mainfont $mainfont]
puts $f [list set textfont $textfont]
@@ -2769,7 +2771,7 @@ proc savestuff {w} {
}
puts $f "}"
close $f
- file rename -force "~/.gitk-new" "~/.gitk"
+ file rename -force $fn [regsub {\-new$} $fn {}]
}
set stuffsaved 1
}
@@ -11723,7 +11725,14 @@ namespace import ::msgcat::mc
## And eventually load the actual message catalog
::msgcat::mcload $gitk_msgsdir
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+ show_error {} . [mc "Cannot find a git repository here."]
+ exit 1
+}
+
catch {source ~/.gitk}
+catch {source [file join $gitdir k]}
parsefont mainfont $mainfont
eval font create mainfont [fontflags mainfont]
@@ -11740,12 +11749,6 @@ setui $uicolor
setoptions
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
- show_error {} . [mc "Cannot find a git repository here."]
- exit 1
-}
-
set selecthead {}
set selectheadid {}
--
1.7.8.6
^ permalink raw reply related
* [PATCH v2] gitk: add a checkbox to control the visibility of tags
From: Łukasz Stelmach @ 2012-12-05 0:48 UTC (permalink / raw)
To: git; +Cc: paulus, gitster, Łukasz Stelmach
In-Reply-To: <7vwqwztv75.fsf@alter.siamese.dyndns.org>
Enable hiding of tags displayed in the tree as yellow labels.
If a repository is used together with a system like Gerrit
there may be quite a lot of tags used to control building
and there may be hardly any place left for commit subjects.
Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
Rebased onto Paul's repository.
gitk | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/gitk b/gitk
index 379582a..46c1a7c 100755
--- a/gitk
+++ b/gitk
@@ -1754,7 +1754,7 @@ proc readrefs {} {
global tagids idtags headids idheads tagobjid
global otherrefids idotherrefs mainhead mainheadid
global selecthead selectheadid
- global hideremotes
+ global hideremotes hidetags
foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
catch {unset $v}
@@ -1776,6 +1776,7 @@ proc readrefs {} {
set headids($name) $id
lappend idheads($id) $name
} elseif {[string match "tags/*" $name]} {
+ if {$hidetags} continue
# this lets refs/tags/foo^{} overwrite refs/tags/foo,
# which is what we want since the former is the commit ID
set name [string range $name 5 end]
@@ -2709,7 +2710,7 @@ proc savestuff {w} {
global cmitmode wrapcomment datetimeformat limitdiffs
global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor
global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk
- global hideremotes want_ttk
+ global hideremotes hidetags want_ttk
if {$stuffsaved} return
if {![winfo viewable .]} return
@@ -2732,6 +2733,7 @@ proc savestuff {w} {
puts $f [list set autosellen $autosellen]
puts $f [list set showneartags $showneartags]
puts $f [list set hideremotes $hideremotes]
+ puts $f [list set hidetags $hidetags]
puts $f [list set showlocalchanges $showlocalchanges]
puts $f [list set datetimeformat $datetimeformat]
puts $f [list set limitdiffs $limitdiffs]
@@ -10924,7 +10926,7 @@ proc create_prefs_page {w} {
proc prefspage_general {notebook} {
global NS maxwidth maxgraphpct showneartags showlocalchanges
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk
+ global hideremotes hidetags want_ttk have_ttk
set page [create_prefs_page $notebook.general]
@@ -10947,6 +10949,9 @@ proc prefspage_general {notebook} {
${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \
-variable hideremotes
grid x $page.hideremotes -sticky w
+ ${NS}::checkbutton $page.hidetags -text [mc "Hide tag labels"] \
+ -variable hidetags
+ grid x $page.hidetags -sticky w
${NS}::label $page.ddisp -text [mc "Diff display options"]
grid $page.ddisp - -sticky w -pady 10
@@ -11048,7 +11053,7 @@ proc doprefs {} {
global oldprefs prefstop showneartags showlocalchanges
global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
- global hideremotes want_ttk have_ttk
+ global hideremotes hidetags want_ttk have_ttk
set top .gitkprefs
set prefstop $top
@@ -11057,7 +11062,7 @@ proc doprefs {} {
return
}
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
set oldprefs($v) [set $v]
}
ttk_toplevel $top
@@ -11177,7 +11182,7 @@ proc prefscan {} {
global oldprefs prefstop
foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
- limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
+ limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
global $v
set $v $oldprefs($v)
}
@@ -11191,7 +11196,7 @@ proc prefsok {} {
global oldprefs prefstop showneartags showlocalchanges
global fontpref mainfont textfont uifont
global limitdiffs treediffs perfile_attrs
- global hideremotes
+ global hideremotes hidetags
catch {destroy $prefstop}
unset prefstop
@@ -11237,7 +11242,8 @@ proc prefsok {} {
$limitdiffs != $oldprefs(limitdiffs)} {
reselectline
}
- if {$hideremotes != $oldprefs(hideremotes)} {
+ if {$hideremotes != $oldprefs(hideremotes) ||
+ $hidetags != $oldprefs(hidetags)} {
rereadrefs
}
}
@@ -11661,6 +11667,7 @@ set cmitmode "patch"
set wrapcomment "none"
set showneartags 1
set hideremotes 0
+set hidetags 0
set maxrefs 20
set maxlinelen 200
set showlocalchanges 1
--
1.7.8.6
^ permalink raw reply related
* Re: [PATCH v2] submodule: add 'deinit' command
From: Junio C Hamano @ 2012-12-04 23:06 UTC (permalink / raw)
To: Jens Lehmann
Cc: Phil Hord, W. Trevor King, Git, Heiko Voigt, Jeff King,
Shawn Pearce, Nahor
In-Reply-To: <50BE6FB9.70301@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> +If you only want to remove the local checkout of a submodule from your
> +work tree without committing that use `git submodule deinit` instead
> +(see linkgit:git-submodule[1]).
I'll add a comma between "without commiting that" and "use X
instead"; it will read better, I think.
> +test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
> + git config submodule.example.foo bar &&
> + git submodule deinit &&
> + test -z "$(git config submodule.example.url)" &&
> + test -z "$(git config submodule.example.foo)"
> +'
This is sufficient, but it might be cleaner to see if
git config --get-regexp "^submodule\.example\."
results in empty. Does not make much difference to warrant a re-roll.
> +test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
> + git submodule deinit &&
> + test_must_fail git submodule deinit example
> +'
> +
> test_done
Thanks; will queue.
^ permalink raw reply
* [PATCH v2] submodule: add 'deinit' command
From: Jens Lehmann @ 2012-12-04 21:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phil Hord, W. Trevor King, Git, Heiko Voigt, Jeff King,
Shawn Pearce, Nahor
In-Reply-To: <7vhao31s9e.fsf@alter.siamese.dyndns.org>
With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config himself).
Help those users by providing a 'deinit' command. This removes the whole
submodule.<name> section from .git/config either for the given
submodule(s) or for all those which have been initialized if none were
given. Complain only when for a submodule given on the command line the
url setting can't be found in .git/config.
Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Am 03.12.2012 08:58, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> Maybe the principle of least surprise is better followed when we
>> nuke the whole section, as it might surprise the user more to have
>> a setting resurrected he customized in the last life cycle of the
>> submodule than seeing that after an deinit followed by an init all
>> former customizations are consistently gone. So I tend to think now
>> that removing the whole section would be the better solution here.
>
> I tend to agree; I suspect that a "deinit" would be mostly done
> either to
>
> (1) correct mistakes the user made during a recent "init" and
> perhaps "sync"; or
>
> (2) tell Git that the user has finished woing with this particular
> submodule and does not intend to use it for quite a while.
>
> For both (1) and (2), I think it would be easier to users if we gave
> them a clean slate, the same state as the one the user who never had
> ran "init" on it would be in. A user in situation (1) is asking for
> a clean slate, and a user in situation (2) is better served if he
> does not have to worry about leftover entries in $GIT_DIR/config he
> has long forgotten from many months ago (during which time the way
> the project uses the particular submodule may well have changed)
> giving non-standard experience different from what other project
> participants would get.
Changes in v2:
- Remove the whole submodule section instead of only removing the
"url" setting and explain why we do that in a comment
- Reworded commit message and git-submodule.txt to reflect that
- Extend the test to check that a custom settings are removed
Documentation/git-rm.txt | 4 ++++
Documentation/git-submodule.txt | 12 ++++++++++
git-submodule.sh | 52 ++++++++++++++++++++++++++++++++++++++++-
t/t7400-submodule-basic.sh | 12 ++++++++++
4 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 262436b..ec42bf5 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
Ignored files are deemed expendable and won't stop a submodule's work
tree from being removed.
+If you only want to remove the local checkout of a submodule from your
+work tree without committing that use `git submodule deinit` instead
+(see linkgit:git-submodule[1]).
+
EXAMPLES
--------
`git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index b1de3ba..08b55a7 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[--reference <repository>] [--] <repository> [<path>]
'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [--] [<path>...]
'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
[--reference <repository>] [--merge] [--recursive] [--] [<path>...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
@@ -134,6 +135,17 @@ init::
the explicit 'init' step if you do not intend to customize
any submodule locations.
+deinit::
+ Unregister the submodules, i.e. remove the whole `submodule.$name`
+ section from .git/config. Further calls to `git submodule update`,
+ `git submodule foreach` and `git submodule sync` will skip any
+ unregistered submodules until they are initialized again, so use
+ this command if you don't want to have a local checkout of the
+ submodule in your work tree anymore (but note that this command
+ does not remove the submodule work tree). If you really want to
+ remove a submodule from the repository and commit that use
+ linkgit:git-rm[1] instead.
+
update::
Update the registered submodules, i.e. clone missing submodules and
checkout the commit specified in the index of the containing repository.
diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..3f558ed 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
or: $dashless [--quiet] init [--] [<path>...]
+ or: $dashless [--quiet] deinit [--] [<path>...]
or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
or: $dashless [--quiet] foreach [--recursive] <command>
@@ -516,6 +517,55 @@ cmd_init()
}
#
+# Unregister submodules from .git/config
+#
+# $@ = requested paths (default to all)
+#
+cmd_deinit()
+{
+ # parse $args after "submodule ... init".
+ while test $# -ne 0
+ do
+ case "$1" in
+ -q|--quiet)
+ GIT_QUIET=1
+ ;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+
+ module_list "$@" |
+ while read mode sha1 stage sm_path
+ do
+ die_if_unmatched "$mode"
+ name=$(module_name "$sm_path") || exit
+ url=$(git config submodule."$name".url)
+ if test -z "$url"
+ then
+ # Only mention uninitialized submodules when its
+ # path have been specified
+ test "$#" != "0" &&
+ say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+ continue
+ fi
+ # Remove the whole section so we have a clean state when the user
+ # later decides to init this submodule again
+ git config --remove-section submodule."$name" &&
+ say "$(eval_gettext "Submodule '\$name' (\$url) unregistered")"
+ done
+}
+
+#
# Update each submodule path to correct revision, using clone and checkout as needed
#
# $@ = requested paths (default to all)
@@ -1108,7 +1158,7 @@ cmd_sync()
while test $# != 0 && test -z "$command"
do
case "$1" in
- add | foreach | init | update | status | summary | sync)
+ add | foreach | init | deinit | update | status | summary | sync)
command=$1
;;
-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index de7d453..ee4f0ab 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -756,4 +756,16 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
)
'
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+ git config submodule.example.foo bar &&
+ git submodule deinit &&
+ test -z "$(git config submodule.example.url)" &&
+ test -z "$(git config submodule.example.foo)"
+'
+
+test_expect_success 'submodule deinit complains only when explicitly used on an uninitialized submodule' '
+ git submodule deinit &&
+ test_must_fail git submodule deinit example
+'
+
test_done
--
1.8.0.1.348.gc64da69
^ permalink raw reply related
* Re: [PATCH v2] t9402: sed -i is not portable
From: Junio C Hamano @ 2012-12-04 20:52 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, mmogilvi_git
In-Reply-To: <201212042044.49477.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> On some systems sed allows the usage of e.g.
> sed -i -e "s/line1/line2/" afile
> to edit the file "in place".
> Other systems don't allow that: one observed behaviour is that
> sed -i -e "s/line1/line2/" afile
> creates a backup file called afile-e, which breaks the test.
> As sed -i is not part of POSIX, avoid it.
Thanks.
The intention is good, but see comments on this part in the patch
below.
> Use test_cmp, makes the test easier to debug.
I see a few other remaining issues in the code after this patch is
applied. If we are doing other fixes like these, we may want to fix
them as well:
- test_must_fail is used to run cvs; it shouldn't (instead of
saying "test_must_fail cvs ...", just say "! cvs ...").
- A shell function should begin like this: "shellfunc () {"
- Redirection should not have SP before the filename (i.e. ">out",
not "> out").
- It is OK to spell single-liner subshell like this:
( cd cvswork3 && ! cvs -f diff -N -u ) &&
...
but a multi-line subshell should start its first command on a
separate line, like this:
(
cd cvswork3 &&
cvs update foo
) &&
...
- quite a few "[cvswork$n]" comments are truncated to "[cvswork$n"
in the test titles.
> Chain all shell commands with && to detect all kinds of failure.
I think you missed the one in 'merge early [cvswork3] b3 with b1'
where a merge is done in a subshell; a failed merge is not detected.
This is another reason why I asked you not to mix different things
in a single patch; if this patch is rerolled to cover missing "&&"
that you missed, it needs to be re-reviewed for the "sed -i" fix we
review during this round again.
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> Changes since v1:
> No correction of TABs to make it easier to review
>
> If this is OK:
> Matthew would you like to send a complete re-roll,
> because the credit should be on you ?
>
> t/t9402-git-cvsserver-refs.sh | 44 ++++++++++++++++++++++---------------------
> 1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
> index 858ef0f..5138f14 100755
> --- a/t/t9402-git-cvsserver-refs.sh
> +++ b/t/t9402-git-cvsserver-refs.sh
> @@ -28,27 +28,26 @@ check_file() {
> }
>
> check_end_tree() {
> - sandbox="$1"
> - expectCount=$(wc -l < "$WORKDIR/check.list")
> - cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
> - test x"$cvsCount" = x"$expectCount"
> - stat=$?
> - echo "check_end $sandbox : $stat cvs=$cvsCount expect=$expectCount" \
> - >> "$WORKDIR/check.log"
> - return $stat
> + sandbox="$1" &&
> + wc -l < "$WORKDIR/check.list" > expected &&
> + find "$sandbox" -type f | grep -v "/CVS" > "$WORKDIR/check.cvsCount" &&
> + wc -l < "$WORKDIR/check.cvsCount" >actual &&
You replaced the original "find" that was perfectly correct with
"pipe to grep -v" which is more expensive. Why?
The file stores the lines to be counted; why is it called a "Count"?
As "check.list" is really the "expected list of files", it may want
to be renamed to "list.expected" or something like that. Then the
output from this "find" would naturally become "list.actual", i.e.
find "$sandbox" -name CVS -type d -prune \
-o -type f -print >"$WORKDIR/list.actual"
Even though you are comparing only the number of lines in the file
in this function, are they expected to name the same set of paths?
The other function check_end_full_tree used to compare only the
count, but you changed it to compare the actul names of paths, which
may be a more robust and correct test, so it might make more sense
to redefine $WORKDIR/list.{expect,actual} to be a sorted list of
paths relative to the top of the working tree and do something like
this:
(
cd "$sandbox" &&
find . -name CVS -type d -prune -o -type f -print
) | sed -e "s|^./||" | sort >"$WORKDIR/list.actual"
and make sure "$WORKDIR/list.expect" is also sorted. Then you can
lose the "wc -l" based check from here and compare the list of
paths.
> + test_cmp expected actual &&
> + rm expected actual &&
> + sort < "$WORKDIR/check.list" > expected &&
> + sort < "$WORKDIR/check.cvsCount" | sed -e "s%cvswork/%%" >actual &&
It is probably easier for a later patch to fix the indentation, if
these new lines followed the existing indentation.
> + test_cmp expected actual &&
> + rm expected actual
> }
>
> check_end_full_tree() {
> - sandbox="$1"
> - ver="$2"
> - expectCount=$(wc -l < "$WORKDIR/check.list")
> - cvsCount=$(find "$sandbox" -name CVS -prune -o -type f -print | wc -l)
> - gitCount=$(git ls-tree -r "$2" | wc -l)
> - test x"$cvsCount" = x"$expectCount" -a x"$gitCount" = x"$expectCount"
> - stat=$?
> - echo "check_end $sandbox : $stat cvs=$cvsCount git=$gitCount expect=$expectCount" \
> - >> "$WORKDIR/check.log"
> - return $stat
> + sandbox="$1" &&
> + sort < "$WORKDIR/check.list" >expected &&
> + find "$sandbox" -name CVS -prune -o -type f -print | sed -e "s%$sandbox/%%" | sort >act1 &&
> + test_cmp expected act1 &&
> + git ls-tree -r "$2" | sed -e "s/^.*blob [0-9a-fA-F]*[ ]*//" | sort > act2 &&
You can say "git ls-tree --name-only -r" and lose the sed.
^ 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