* [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
* Git problem with CIFS/SAMBA URLs
From: Nils Fenner @ 2012-12-04 20:14 UTC (permalink / raw)
To: git
Hi guys,
I tried to use url.insteadof to get URLs mapped correctly, starting with a double-slash (//). Unfortunately, Git doesn't recognize that and still tries to connect to the original URL. I am using Git in a productive environment. On Windows desktops, MsysGit 1.7.11 and 1.8.0. On ubuntu clients Git 1.7.9.0 (as delivered with the distro).
Problem is, that we have lots and lots of submodules, with URLs like //thegitserver/.../therepo.git. Now I'd like to checkout on the Ubuntu machine for the CI system. Should not be much of a preblem - well, in theory.
Let's replace the windows url's (//someserver/repo.git) with a mounted CIFS path (/media/samba_share/repo.git). On MsysGit this works as long as the server is reachable. On Ubuntu it completely doesn't work. All comes down to the url-alias, which seems not to be replaced correctly when using windows double-slash url's.
I'd like to give some more samples to show what's happening:
To access cross-plattform file systems I mounted the windows share via samba (e.g. on /media/samba_share). This works fine and we are able to clone the main repositories by using:
$ git clone /media/samba_share/repo.git
Now let's get the submodules:
$ git submodule init -> writes the "wrong" //thegitserver/... URLs in the config.
$ git submodule update -> fails because of the CIFS double-slash url's.
What to do? Simply define a "url.insteadof" alias
[url "/media/samba_share"]
insteadof = //thegitserver
Uh, oh. Doesn't help.
The following would work like expected, but is not usable for my case. I would have to rewrite all URLs, what I don't want to do (maybe in the future):
[url "/media/samba_share"]
insteadof = file:////serverurl
On MsysGit, replacing the double-slash works, as long as the URL is reachable (meaning the server has to be online). Otherwise it behaves the same like Linux.
Sample error Message:
fatal: repository '//thegitserver/.../repository.git' does not exist
The URL should be: /media/samba_share/.../repository.git
This seems to be the only case. Other URLs work like expected.
Seems like this is a bug in Git. Or did I miss something?
Please help!
Cheers,
Nils
^ permalink raw reply
* Re: [PATCH] gitk: read and write a repository specific configuration file
From: Lukasz Stelmach @ 2012-12-04 20:12 UTC (permalink / raw)
To: Stefan Haller; +Cc: git, paulus, gitster
In-Reply-To: <1kujrnc.1r4khfkkpdoo1M%lists@haller-berlin.de>
W dniu 03.12.2012 22:15, Stefan Haller pisze:
> Lukasz Stelmach <stlman@poczta.fm> 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.
>
> I'm not sure I like this proposal. While it may be desirable to have
> *some* settings stored per repository, for most settings I want them to
> be remembered globally.
The way it works with my patch, gitk reads global settings from ~/.gitk.
So you can treat it as a template. Then, when you exit it saves to local
file if it exists. This of course means you can't override settings from
./.git/k with something from ~/.gitk by simply choosing to on GUI.
However, it takes no more than removing appropriate line from .git/k to
get the value from ~/.gitk.
IMHO this is a reasonable compromise which is available at no cost as
far as data structure complexity is concerned. Choosing where to save
what would require a bit of information per configuration variable. With
a mask saved locally surprises may come when you change a variable,
forget to localise it and it pops in a different repository. My
approach, however simplistic, avoids this particular pitfall.
> Git-gui tries to solve this by presenting two panes in the preferences
> dialog, so that I can choose the scope of every setting I change. This
> still doesn't help for things that are remembered implicitly, like the
> window size.
>
> I don't have good suggestions how to solve this; just pointing out
> problems.
>
>
--
Było mi bardzo miło. Czwarta pospolita klęska, [...]
>Łukasz< Już nie katolicka lecz złodziejska. (c)PP
^ permalink raw reply
* [PATCH v2] t9402: sed -i is not portable
From: Torsten Bögershausen @ 2012-12-04 19:44 UTC (permalink / raw)
To: git, mmogilvi_git; +Cc: tboegi
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.
Use test_cmp, makes the test easier to debug.
Chain all shell commands with && to detect all kinds of failure.
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 &&
+ test_cmp expected actual &&
+ rm expected actual &&
+ sort < "$WORKDIR/check.list" > expected &&
+ sort < "$WORKDIR/check.cvsCount" | sed -e "s%cvswork/%%" >actual &&
+ 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 &&
+ test_cmp expected act2 &&
+ rm expected act1 act2
}
#########
@@ -155,7 +154,8 @@ test_expect_success 'cvs co b1 [cvswork3]' '
test_expect_success 'edit cvswork3 and save diff' '
( cd cvswork3 &&
- sed -i -e "s/line1/line1 - data/" adir/afile &&
+ sed -e "s/line1/line1 - data/" adir/afile >adir/afileNEW &&
+ mv -f adir/afileNEW adir/afile &&
echo "afile5" > adir/afile5 &&
rm t2 &&
cvs -f add adir/afile5 &&
@@ -168,7 +168,8 @@ test_expect_success 'setup v1.2 on b1' '
git checkout b1 &&
echo "new v1.2" > t3 &&
rm t2 &&
- sed -i -e "s/line3/line3 - more data/" adir/afile &&
+ sed -e "s/line3/line3 - more data/" adir/afile >adir/afileNEW &&
+ mv -f adir/afileNEW adir/afile &&
rm adir/a2file &&
echo "a3file" >> adir/a3file &&
echo "bfile line 3" >> adir/bdir/bfile &&
@@ -300,7 +301,8 @@ test_expect_success 'root dir rm file [cvswork2]' '
test_expect_success 'subdir edit/add/rm files [cvswork2' '
( cd cvswork2 &&
- sed -i -e "s/line 1/line 1 (v2)/" adir/bdir/bfile &&
+ sed -e "s/line 1/line 1 (v2)/" adir/bdir/bfile >adir/bdir/bfileNEW &&
+ mv -f adir/bdir/bfileNEW adir/bdir/bfile &&
rm adir/bdir/b2file &&
cd adir &&
cvs -f rm bdir/b2file &&
--
1.8.0.197.g5a90748
^ permalink raw reply related
* Re: [RFC] Add basic syntax check on shell scripts
From: Junio C Hamano @ 2012-12-04 19:39 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Torsten Bögershausen, git
In-Reply-To: <CACsJy8BxviWRHqGvptsJVmkFM6HQa9HnLWsh5V6Ec6Fqv52sGA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>> This (once it gets cleaned up to reduce false positives) belongs to
>> "cd t && make test-lint".
>
> Or a project commit hook?
Surely. It is OK to have "cd t && make test-lint" in your
pre-commit hook.
A few more things in addition to what Torsten's script attempts to
catch that we would want to catch are:
* Do not spell string equality with "test $a == $b"; that is
bash-ism and you only need "=" (which works in bash, too);
* Do not capture output from "wc -l" in a variable and string
compare with a constant, e.g.
lnum=$(wc -l <...) && test "$lnum" = 9
as some wc implementations place extra SP in its output;
* Do not use "test_must_fail" to run non-git command and require it
to fail (instead, just write "! cmd").
* 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.
^ permalink raw reply
* Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
From: Erik Faye-Lund @ 2012-12-04 17:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, msysgit, peff
In-Reply-To: <alpine.DEB.1.00.1212041809150.31987@s15462909.onlinehome-server.info>
On Tue, Dec 4, 2012 at 6:12 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Sat, 1 Dec 2012, Erik Faye-Lund wrote:
>
>> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi,
>> >
>> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
>> >
>> >> Set a control-handler to prevent the process from terminating, and
>> >> simulate SIGINT so it can be handled by a signal-handler as usual.
>> >
>> > One thing you might want to mention is that the fgetc() handling is not
>> > thread-safe, and intentionally so: if two threads read from the same
>> > console, we are in trouble anyway.
>>
>> I'm not entirely sure if I know what you mean. Do you suggest that two
>> threads can race for setting the console ctrl-handler?
>
> That was my idea, yes.
>
>> I don't think that's the case; "SetConsoleCtrlHandler(x, TRUE)" adds a
>> console handler to the handler-chain, and SetConsoleCtrlHandler(x,
>> FALSE) removes it. If two threads add handlers, it is my understanding
>> that one of them will be run, only to report "no, no more ctrl-handling
>> needed". Since both handlers block further ctrl-handling, I don't think
>> there's a problem.
>
> My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the
> handler prematurely iff another thread wanted to install the very same
> handler (but it was already installed by the first thread).
>
Yes, but that's not how SetConsoleCtrlHandler works; if a routine x
gets added twice, it needs to be removed twice as well to not get
called. This little C-program demonstrates that:
---8<---
#include <windows.h>
#include <stdio.h>
BOOL WINAPI HandlerRoutine1(DWORD dwCtrlType)
{
printf("Hello from handler1!\n");
return TRUE;
}
BOOL WINAPI HandlerRoutine2(DWORD dwCtrlType)
{
printf("Hello from handler2!\n");
return FALSE;
}
int main()
{
SetConsoleCtrlHandler(HandlerRoutine1, TRUE);
SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
SetConsoleCtrlHandler(HandlerRoutine2, TRUE);
SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0);
SetConsoleCtrlHandler(HandlerRoutine2, FALSE);
SetConsoleCtrlHandler(HandlerRoutine1, FALSE);
}
---8<---
This program outputs:
Hello from handler2!
Hello from handler1!
So since that other thread would add the console ctrl handler before
it removed it again, this should still be thread-safe as far as I can
tell.
> But as I said: for this to happen, *two* threads need to want to access
> the console for reading. In that case we'd be in bigger trouble than
> thread unsafety... We cannot read two passwords from the same console at
> the same time.
I agree with that, but I'm saying that *even if* we didn't have this
limitation, the code shouldn't be problematic.
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* git-svn with non-standard repository layout
From: Piotr Krukowiecki @ 2012-12-04 17:29 UTC (permalink / raw)
To: Git Mailing List
Hi,
Is there a way to handle svn repository with following layout?
repo/trunk
repo/branches/branch1
repo/branches/branch2
repo/branches/work/developer1/branch3
repo/branches/work/developer1/branch4
repo/branches/work/developer2/branch5
In default configuration it treats "work" dir as a branch. If I
configure it with branches = branches/work/*/* it recognizes them
correctly, but then the "normal" branch1, branch2 are lost.
Ideally, I'd have one git repo with trunk, branches/{branch1,branch2},
work/developerX/branchY branches...
Any ideas?
--
Piotr Krukowiecki
^ permalink raw reply
* Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling
From: Johannes Schindelin @ 2012-12-04 17:12 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, msysgit, peff
In-Reply-To: <CABPQNSY_vHdvBvU_ezjyOzoZeBJAYTJ2829o6Vxs-nJjQVcvDQ@mail.gmail.com>
Hi kusma,
On Sat, 1 Dec 2012, Erik Faye-Lund wrote:
> On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
> >
> >> By moving the echo-disabling code to a separate function, we can
> >> implement OS-specific versions of it for non-POSIX platforms.
> >>
> >> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> >> ---
> >> compat/terminal.c | 43 +++++++++++++++++++++++++------------------
> >> 1 file changed, 25 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/compat/terminal.c b/compat/terminal.c
> >> index bbb038d..3217838 100644
> >> --- a/compat/terminal.c
> >> +++ b/compat/terminal.c
> >> @@ -14,6 +14,7 @@ static void restore_term(void)
> >> return;
> >>
> >> tcsetattr(term_fd, TCSAFLUSH, &old_term);
> >> + close(term_fd);
> >> term_fd = -1;
> >> }
> >
> > That looks like an independent resource leak fix... correct?
>
> It might look like it, but it's not; term_fd used to be returned by
> "fileno(fh)", and fh did get properly closed.
>
> With my refactoring, disable_echo/restore_term takes opens /dev/tty a
> second time, like Jeff points out. And that second file descriptor
> needs to be closed.
Thanks for clarifying,
Dscho
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
From: Johannes Schindelin @ 2012-12-04 17:12 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, msysgit, peff
In-Reply-To: <CABPQNSaCV820zhJGdW++LMf2U7AeODSbfukix3fMfffmNex4YA@mail.gmail.com>
Hi kusma,
On Sat, 1 Dec 2012, Erik Faye-Lund wrote:
> On Fri, Nov 30, 2012 at 6:58 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
> >
> >> Set a control-handler to prevent the process from terminating, and
> >> simulate SIGINT so it can be handled by a signal-handler as usual.
> >
> > One thing you might want to mention is that the fgetc() handling is not
> > thread-safe, and intentionally so: if two threads read from the same
> > console, we are in trouble anyway.
>
> I'm not entirely sure if I know what you mean. Do you suggest that two
> threads can race for setting the console ctrl-handler?
That was my idea, yes.
> I don't think that's the case; "SetConsoleCtrlHandler(x, TRUE)" adds a
> console handler to the handler-chain, and SetConsoleCtrlHandler(x,
> FALSE) removes it. If two threads add handlers, it is my understanding
> that one of them will be run, only to report "no, no more ctrl-handling
> needed". Since both handlers block further ctrl-handling, I don't think
> there's a problem.
My idea was that the SetConsoleCtrlHandler(x, FALSE) could remove the
handler prematurely iff another thread wanted to install the very same
handler (but it was already installed by the first thread).
But as I said: for this to happen, *two* threads need to want to access
the console for reading. In that case we'd be in bigger trouble than
thread unsafety... We cannot read two passwords from the same console at
the same time.
Ciao,
Dscho
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: [PATCH] mingw_rmdir: do not prompt for retry when non-empty
From: Johannes Schindelin @ 2012-12-04 16:35 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, msysgit
In-Reply-To: <1354617713-7436-1-git-send-email-kusmabite@gmail.com>
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);
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ 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