Re: [PATCH 1/3] Make test using invalid commit with -C more strict
Junio C Hamano wrote: Kacper Kornet wrote: In the test 'using invalid commit with -C' git-commit would have failed even if the -C option had been given the correct commit, as there was nothing to commit. [...] Also it would be much simpler to say git commit --allow-empty. Sounds good. ;-) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] t/t7106-reset-unborn-branch.sh: Add PERL prerequisite
Junio C Hamano wrote: The change to the one that feeds 'y' to reset -p may be a bit too pedantic, as we are not in the business of testing echo y, though. Yeah, that's true. Here's a patch for squashing in. diff --git i/t/t7106-reset-unborn-branch.sh w/t/t7106-reset-unborn-branch.sh index af00ab4d..bd28feba 100755 --- i/t/t7106-reset-unborn-branch.sh +++ w/t/t7106-reset-unborn-branch.sh @@ -12,9 +12,8 @@ test_expect_success 'reset' ' git add a b git reset - expect git ls-files actual - test_cmp expect actual + test_must_be_empty actual ' test_expect_success 'reset HEAD' ' @@ -36,12 +35,10 @@ test_expect_success 'reset $file' ' test_expect_success PERL 'reset -p' ' rm .git/index git add a - echo y yes - git reset -p yes + echo y | git reset -p - expect git ls-files actual - test_cmp expect actual + test_must_be_empty actual ' test_expect_success 'reset --soft is a no-op' ' @@ -60,9 +57,8 @@ test_expect_success 'reset --hard' ' test_when_finished echo a a git reset --hard - expect git ls-files actual - test_cmp expect actual + test_must_be_empty actual test_path_is_missing a ' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test
Eric Sunshine wrote: On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder jrnie...@gmail.com wrote: Would be clearer if the code in a subshell were indented: ( unset test_tick test_commit ... ) I considered it, but decided against it for a couple reasons: * In this script, there already is a mix between the two styles: indented vs. unindented. * In this particular patch, the test_commit line creating commit3 wrapped beyond 80 columns when indented. I'm just one person, but I find the indented form way more readable. Long lines or lines with \ continuation are not a big deal. [...] Should this be worth a re-roll? Since the file already has a mixture of styles, if there's no other reason to reroll, I'd suggest leaving it alone. The next time it bugs me or someone else, that person can write a patch that cleans up the whole file on top. :) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] write_index: optionally allow broken null sha1s
On Sun, Aug 25, 2013 at 05:58:18AM -0400, Jeff King wrote:
On Sat, Aug 24, 2013 at 11:15:00PM -0700, Jonathan Nieder wrote:
I was tempted to not involve filter-branch in this commit at all, and
instead require the user to manually invoke
GIT_ALLOW_NULL_SHA1=1 git filter-branch ...
to perform such a filter. That would be slightly safer, but requires
some specialized knowledge from the user (and advice on using
filter-branch to remove such entries already exists on places like
stackoverflow, and this patch makes it Just Work on recent versions of
git).
The above few paragraphs explained the most mysterious part of the
patch to me. I think they would be fine to include in the commit
message.
I rolled them into the commit message.
Hm --- the most useful part was advice on using filter-branch to
remove such entries already exists on places like stackoverflow,
which was dropped. From my point of view, that is exactly the
motivation of the patch.
I also found the I was tempted to ... That would be slightly safer,
but requires ... structure easier to read.
In other words, why not use something like this?
write_index: optionally allow broken null sha1s
Commit 4337b58 (do not write null sha1s to on-disk index, 2012-07-28)
added a safety check preventing git from writing null sha1s into the
index. The intent was to catch errors in other parts of the code that
might let such an entry slip into the index (or worse, a tree).
Some existing repositories have some invalid trees that contain null
sha1s already, though. Until 4337b58, a common way to clean this up
would be to use git-filter-branch's index-filter to repair such broken
entries. That now fails when filter-branch tries to write out the
index.
Introduce a GIT_ALLOW_NULL_SHA1 environment variable to relax this check
and make it easier to recover from such a history.
It is tempting to not involve filter-branch in this commit at all, and
instead require the user to manually invoke
GIT_ALLOW_NULL_SHA1=1 git filter-branch ...
to perform an index-filter on a history with trees with null sha1s.
That would be slightly safer, but requires some specialized knowledge
from the user. So let's set the GIT_ALLOW_NULL_SHA1 variable
automatically when checking out the to-be-filtered trees. Advice on
using filter-branch to remove such entries already exists on places like
stackoverflow, and this patch makes it Just Work again on recent
versions of git.
Further commands that touch the index will still notice and fail,
unless
they actually remove the broken entries. A filter-branch whose filters
do not touch the index at all will not error out (since we complain of
the null sha1 only on writing, not when making a tree out of the index),
but this is acceptable, as we still print a loud warning, so the problem
is unlikely to go unnoticed.
With or without such a change,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
After this patch, do you think (in a separate change) it would make
sense for cache-tree.c::update_one() to check for null sha1 and error
out unless GIT_ALLOW_NULL_SHA1 is true? That would let us get rid of
the caveat from the last paragraph.
[...]
--- /dev/null
+++ b/t/t7009-filter-branch-null-sha1.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='filter-branch removal of trees with null sha1'
+. ./test-lib.sh
+
+test_expect_success 'create base commits' '
+ test_commit one
+ test_commit two
+ test_commit three
+'
+
+test_expect_success 'create a commit with a bogus null sha1 in the tree' '
+ {
+ git ls-tree HEAD
+ printf 16 commit $_z40\\tbroken\\n
+ } broken-tree
+ echo add broken entry msg
+
+ tree=$(git mktree broken-tree)
+ test_tick
+ commit=$(git commit-tree $tree -p HEAD msg)
+ git update-ref HEAD $commit
+'
+
+# we have to make one more commit on top removing the broken
+# entry, since otherwise our index does not match HEAD (and filter-branch
will
+# complain). We could make the index match HEAD, but doing so would involve
+# writing a null sha1 into the index.
+test_expect_success 'create a commit dropping the broken entry' '
+ test_tick
+ git commit -a -m back to normal
+'
This is kind of an old-fashioned test, since each step of the setup is
treated as a separate test assertion. I don't really mind until we
get better automation to make it easy to skip or rearrange tests.
Just for reference, I think the usual way to do this now is
test_expect_success 'setup' '
# create base commits
...
# create a commit with bogus null sha1 in the tree
...
# We have to make one more
Re: Re: [PATCH 09/13] Improve section Merge multiple trees
Thomas Ackermann wrote: Maybe the intent is Git can help you perform a three-way merge, which can in turn be [...] If you don't mind I will use your text No problem. Signed-off-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn - canonicalize: Assertion `*src != '/'' failed.
Hi, Bruce Korb wrote: I was trying to clone a SVN repo, but not having luck: $ git svn clone $PWD/private-lustre-svn $PWD/private-lustre-git perl: subversion/libsvn_subr/dirent_uri.c:321: canonicalize: Assertion `*src != '/'' failed. error: git-svn died of signal 6 What is Perl or Subversion or GIT trying to tell me, please? Thank you! What version of git are you using? And what version of libsvn (and the libsvn perl bindings)? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] write_index: optionally allow broken null sha1s
Jeff King wrote: On Sun, Aug 25, 2013 at 12:54:12PM -0700, Jonathan Nieder wrote: [setup split across three tests] This is kind of an old-fashioned test, since each step of the setup is treated as a separate test assertion. I don't really mind until we get better automation to make it easy to skip or rearrange tests. Just for reference, I think the usual way to do this now is I don't see that splitting it up more hurts this. If we wanted more automatic rearranging or skipping of tests, we would need tests to declare dependencies on their setup. And we would need to be able to declare dependencies on multiple tests, since having a single setup test does not work in all cases (e.g., sometimes you are testing each step, and the final step relies on earlier steps). Actually dependencies can already be inferred for most test scripts using the following rule: Each test depends on all tests with the word setup or the phrase set up in their title that precede it. And while there's no existing automation for testing that that's all the tests rely on (by automatically skipping or reordering tests, or running non-setup tests in separate sandboxes in parallel), in practice it is still already useful since it makes it safe to use GIT_SKIP_TESTS subject to the following constraint: In each test script, for every setup test skipped, all later tests are skipped as well. I don't care as much about GIT_SKIP_TESTS as about being able to introduce new tests in the middle of a file. Of course splitting up the setup into 3 steps neither helps nor hurts that. What I was complaining about is splitting the filter-branch from the verification that filter-branch had the right result. Sorry for the confusion, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-svn - canonicalize: Assertion `*src != '/'' failed.
Bruce Korb wrote: $ git svn --version git-svn version 1.8.1.4 (svn 1.7.11) Hm. Two ideas: * Does 1.8.2 or newer work better? (It contains v1.8.2-rc0~110^2, git-svn: do not escape certain characters in paths, 2013-01-17, which at first glance looks unlikely to help but might be worth a try.) * Does git svn clone file://$PWD/private-lustre-svn $PWD/private-lustre-git work? Ciao, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/11] Use git merge instead of git pull .
On Tue, Aug 27, 2013 at 12:06:33PM -0700, Junio C Hamano wrote: Thomas Ackermann th.ac...@arcor.de writes: git pull . works, but git merge is the recommended way for new users to do things. (The old description also should have read The former is actually *not* very commonly used.) It does not matter that you are unaware other people use it often. I'd suggest dropping the first hunk altogether. Eh, the claim The former is actually very commonly used. is confusing on its own (even though it used to be true) and elaborating wouldn't help much with education, so the first hunk makes sense to me. But maybe it should have been done in a separate patch. ;-) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
Matthieu Moy wrote: Historically, git status needed to prefix each output line with '#' so that the output could be added as comment to the commit message. This prefix comment has no real purpose when git status is ran from the command-line, and this may distract users from the real content. Allow the user to disable this prefix comment. Why not do this unconditionally? In the long run, if users like the non-prefix output, it may make sense to flip the default value to true. Hmm, do you mean that the configuration is to give the change-averse some time to get used to the new output? I think it would make sense to do it all at once (and even think that that would be less confusing). The rest of the idea and execution seem sane. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Stalled git cloning and possible solutions
V.Krishn wrote: Quite sometimes when cloning a large repo stalls, hitting Ctrl+c cleans what been downloaded, and process needs re-start. Is there a way to recover or continue from already downloaded files during cloning ? No, sadly. The pack sent for a clone is generated dynamically, so there's no easy way to support the equivalent of an HTTP Range request to resume. Someone might implement an appropriate protocol extension to tackle this (e.g., peff's seed-with-clone.bundle hack) some day, but for now it doesn't exist. What you *can* do today is create a bundle from the large repo somewhere with a reliable connection and then grab that using a resumable transport such as HTTP. A kind person made a service to do that. http://thread.gmane.org/gmane.comp.version-control.git/181380 Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
Hi, Philip Oakley wrote: Does `hash-object` do the inverese of `cat-file commit`? I didn't find the hash-object(1) man page very informative on that matter Hm. The manpage says: Computes the object ID value for an object with specified type with the contents of the named file [...], and optionally writes the resulting object into the object database. And then: -w Actually write the object into the object database. Any ideas for making this clearer? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check
Philip Oakley wrote: The problem is the file format, in the sense that the earlier `git cat-file commit $orig` has a human readable output which is a description of the commit header, rather than the specific binary content. Ah. That's the actual raw commit object format, though. The manpage for git-cat-file(1) says: SYNOPSIS git cat-file (-t | -s | -e | -p | type | --textconv ) object git cat-file (--batch | --batch-check) list-of-objects DESCRIPTION In its first form, the command provides the content or the type of an object in the repository. [...] OUTPUT ... If type is specified, the raw (though uncompressed) contents of the object will be returned. I agree that this isn't as clear as it should be. I see a few problems: (1) The synopsis treats git cat-file -t/-s/-e/-p object, git cat-file --textconv tree:path, and git cat-file type object as the same form of the command. It would be easier to explain these as three different forms. (2) There is no EXAMPLES section and no examples. (3) There is no pointer to the git object formats. A pointer to a new gitobject(5) manpage would presumably make everything clearer. https://www.kernel.org/pub/software/scm/git/docs/user-manual.html#examining-the-data might be a good source of text to start from for solving (1), since it explains the command a little better. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: the pager
Hi, Dale R. Worley wrote: That's true, but it would change the effect of using cat as a value: cat as a value of DEFAULT_PAGER would cause git_pager() to return NULL, whereas now it causes git_pager() to return cat. (All other places where cat can be a value are translated to NULL already.) This is why I want to know what the *intended* behavior is, because we might be changing Git's behavior, and I want to know that if we do that, we're changing it to what it should be. And I haven't seen anyone venture an opinion on what the intended behavior is. I don't really follow. For all practical purposes, cat is equivalent to no pager at all, no? And the git-var(1) manpage describes the intended order of precedence, as far as I can tell, except that it was written before v1.7.4-rc0~76^2 (allow command-specific pagers in pager.cmd, 2010-11-17) which forgot to update some documentation. Suggested wording for improving the documentation or its organization would of course be welcome. And I agree with Matthieu that the name of the pager_program global variable is needlessly confusing --- perhaps it should be called config_pager_program or similar. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions
SZEDER Gábor wrote: On Tue, Sep 03, 2013 at 08:39:54AM -0500, Felipe Contreras wrote: There are two ways to fix an inconsistency, the other way is to fix test_cmp. But that would be a change, and change is not welcome in Git. It depends on the change, I suppose. I agree, changing 3k+ lines just to avoid yoda conditions... I doubt the gain worth the code churn. Especially when the idiom being changed is not even being made better. ;-) test_cmp_rev follows the same order of arguments a diff -u and produces the same output as plain git diff. It's perfectly readable and normal. I think Felipe is pushing buttons and testing boundaries. But in the process came a report of a missing test_cmp_rev use, which is useful. Patch follows. While at it, I rerolled the other patches from the series to clarify their commit messages (replacing fix something with a fuller description). Improvements welcome, as always. Thanks. Felipe Contreras (3): rev-parse test: modernize quoting and whitespace rev-parse test: use test_must_fail, not if command; then false; fi rev-parse test: use standard test functions for setup Jonathan Nieder (1): rev-parse test: use test_cmp instead of test builtin t/t6101-rev-parse-parents.sh | 110 +++ 1 file changed, 79 insertions(+), 31 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] rev-parse test: modernize quoting and whitespace
From: Felipe Contreras felipe.contre...@gmail.com
Instead of cramming everything in one line, put the test body in an
indented block after the opening test_expect_success line and quote
and put the closing quote on a line by itself.
Use single-quote instead of double-quote to quote the test body
for more useful --verbose output.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
t/t6101-rev-parse-parents.sh | 74 ++--
1 file changed, 58 insertions(+), 16 deletions(-)
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index e673c25..c47b869 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -17,21 +17,62 @@ hide_error save_tag start2 unique_commit start2 tree
save_tag two_parents unique_commit next tree -p second -p start2
save_tag final unique_commit final tree -p two_parents
-test_expect_success 'start is valid' 'git rev-parse start | grep
^[0-9a-f]\{40\}$'
-test_expect_success 'start^0' test $(cat .git/refs/tags/start) = $(git
rev-parse start^0)
-test_expect_success 'start^1 not valid' if git rev-parse --verify start^1;
then false; else :; fi
-test_expect_success 'second^1 = second^' test $(git rev-parse second^1) =
$(git rev-parse second^)
-test_expect_success 'final^1^1^1' test $(git rev-parse start) = $(git
rev-parse final^1^1^1)
-test_expect_success 'final^1^1^1 = final^^^' test $(git rev-parse
final^1^1^1) = $(git rev-parse final^^^)
-test_expect_success 'final^1^2' test $(git rev-parse start2) = $(git
rev-parse final^1^2)
-test_expect_success 'final^1^2 != final^1^1' test $(git rev-parse final^1^2)
!= $(git rev-parse final^1^1)
-test_expect_success 'final^1^3 not valid' if git rev-parse --verify
final^1^3; then false; else :; fi
-test_expect_success '--verify start2^1' 'test_must_fail git rev-parse --verify
start2^1'
-test_expect_success '--verify start2^0' 'git rev-parse --verify start2^0'
-test_expect_success 'final^1^@ = final^1^1 final^1^2' test \$(git rev-parse
final^1^@)\ = \$(git rev-parse final^1^1 final^1^2)\
-test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' test \$(git
rev-parse final^1^\!)\ = \$(git rev-parse final^1 ^final^1^1 ^final^1^2)\
-
-test_expect_success 'repack for next test' 'git repack -a -d'
+test_expect_success 'start is valid' '
+ git rev-parse start | grep ^[0-9a-f]\{40\}$
+'
+
+test_expect_success 'start^0' '
+ test $(cat .git/refs/tags/start) = $(git rev-parse start^0)
+'
+
+test_expect_success 'start^1 not valid' '
+ if git rev-parse --verify start^1; then false; else :; fi
+'
+
+test_expect_success 'second^1 = second^' '
+ test $(git rev-parse second^1) = $(git rev-parse second^)
+'
+
+test_expect_success 'final^1^1^1' '
+ test $(git rev-parse start) = $(git rev-parse final^1^1^1)
+'
+
+test_expect_success 'final^1^1^1 = final^^^' '
+ test $(git rev-parse final^1^1^1) = $(git rev-parse final^^^)
+'
+
+test_expect_success 'final^1^2' '
+ test $(git rev-parse start2) = $(git rev-parse final^1^2)
+'
+
+test_expect_success 'final^1^2 != final^1^1' '
+ test $(git rev-parse final^1^2) != $(git rev-parse final^1^1)
+'
+
+test_expect_success 'final^1^3 not valid' '
+ if git rev-parse --verify final^1^3; then false; else :; fi
+'
+
+test_expect_success '--verify start2^1' '
+ test_must_fail git rev-parse --verify start2^1
+'
+
+test_expect_success '--verify start2^0' '
+ git rev-parse --verify start2^0
+'
+
+test_expect_success 'final^1^@ = final^1^1 final^1^2' '
+ test $(git rev-parse final^1^@) = $(git rev-parse final^1^1
final^1^2)
+'
+
+test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' '
+ test $(git rev-parse final^1^!) = $(git rev-parse final^1 ^final^1^1
^final^1^2)
+'
+
+test_expect_success 'repack for next test' '
+ git repack -a -d
+'
+
test_expect_success 'short SHA-1 works' '
start=`git rev-parse --verify start`
echo $start
@@ -39,6 +80,7 @@ test_expect_success 'short SHA-1 works' '
echo $abbrv
abbrv=`git rev-parse --verify $abbrv`
echo $abbrv
- test $start = $abbrv'
+ test $start = $abbrv
+'
test_done
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] rev-parse test: use test_must_fail, not if command; then false; fi
From: Felipe Contreras felipe.contre...@gmail.com This way, if rev-parse segfaults then the test will fail instead of treating it the same way as a controlled failure. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- t/t6101-rev-parse-parents.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh index c47b869..416067c 100755 --- a/t/t6101-rev-parse-parents.sh +++ b/t/t6101-rev-parse-parents.sh @@ -26,7 +26,7 @@ test_expect_success 'start^0' ' ' test_expect_success 'start^1 not valid' ' - if git rev-parse --verify start^1; then false; else :; fi + test_must_fail git rev-parse --verify start^1 ' test_expect_success 'second^1 = second^' ' @@ -50,7 +50,7 @@ test_expect_success 'final^1^2 != final^1^1' ' ' test_expect_success 'final^1^3 not valid' ' - if git rev-parse --verify final^1^3; then false; else :; fi + test_must_fail git rev-parse --verify final^1^3 ' test_expect_success '--verify start2^1' ' -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] rev-parse test: use test_cmp instead of test builtin
Use test_cmp instead of passing two command substitutions to the
test builtin. This way:
- when tests fail, they can print a helpful diff if run with
--verbose
- the argument order test_cmp expect actual feels natural,
unlike test known = unknown that seems a little backwards
- the exit status from invoking git is checked, so if rev-parse
starts segfaulting then the test will notice and fail
Use a custom function for this instead of test_cmp_rev to emphasize
that we are testing the output from git rev-parse with certain
arguments, not checking that the revisions are equal in abstract.
Reported-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
t/t6101-rev-parse-parents.sh | 33 +++--
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 416067c..8a6ff66 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -8,6 +8,12 @@ test_description='Test git rev-parse with different parent
options'
. ./test-lib.sh
. $TEST_DIRECTORY/lib-t6000.sh # t6xxx specific functions
+test_cmp_rev_output () {
+ git rev-parse --verify $1 expect
+ eval $2 actual
+ test_cmp expect actual
+}
+
date path0
git update-index --add path0
save_tag tree git write-tree
@@ -22,7 +28,7 @@ test_expect_success 'start is valid' '
'
test_expect_success 'start^0' '
- test $(cat .git/refs/tags/start) = $(git rev-parse start^0)
+ test_cmp_rev_output tags/start git rev-parse start^0
'
test_expect_success 'start^1 not valid' '
@@ -30,19 +36,19 @@ test_expect_success 'start^1 not valid' '
'
test_expect_success 'second^1 = second^' '
- test $(git rev-parse second^1) = $(git rev-parse second^)
+ test_cmp_rev_output second^ git rev-parse second^1
'
test_expect_success 'final^1^1^1' '
- test $(git rev-parse start) = $(git rev-parse final^1^1^1)
+ test_cmp_rev_output start git rev-parse final^1^1^1
'
test_expect_success 'final^1^1^1 = final^^^' '
- test $(git rev-parse final^1^1^1) = $(git rev-parse final^^^)
+ test_cmp_rev_output final^^^ git rev-parse final^1^1^1
'
test_expect_success 'final^1^2' '
- test $(git rev-parse start2) = $(git rev-parse final^1^2)
+ test_cmp_rev_output start2 git rev-parse final^1^2
'
test_expect_success 'final^1^2 != final^1^1' '
@@ -62,11 +68,15 @@ test_expect_success '--verify start2^0' '
'
test_expect_success 'final^1^@ = final^1^1 final^1^2' '
- test $(git rev-parse final^1^@) = $(git rev-parse final^1^1
final^1^2)
+ git rev-parse final^1^1 final^1^2 expect
+ git rev-parse final^1^@ actual
+ test_cmp expect actual
'
test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' '
- test $(git rev-parse final^1^!) = $(git rev-parse final^1 ^final^1^1
^final^1^2)
+ git rev-parse final^1 ^final^1^1 ^final^1^2 expect
+ git rev-parse final^1^! actual
+ test_cmp expect actual
'
test_expect_success 'repack for next test' '
@@ -74,13 +84,8 @@ test_expect_success 'repack for next test' '
'
test_expect_success 'short SHA-1 works' '
- start=`git rev-parse --verify start`
- echo $start
- abbrv=`echo $start | sed s/.\$//`
- echo $abbrv
- abbrv=`git rev-parse --verify $abbrv`
- echo $abbrv
- test $start = $abbrv
+ start=$(git rev-parse --verify start)
+ test_cmp_rev_output start git rev-parse ${start%?}
'
test_done
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] rev-parse test: use standard test functions for setup
From: Felipe Contreras felipe.contre...@gmail.com
Save the reader from learning specialized t6* setup functions
where familiar commands like test_commit, git checkout --orphan,
and git merge will do.
While at it, wrap the setup commands in a test assertion so errors can
be caught and stray output suppressed when running without --verbose
as in other tests.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Lost a line from the patch last time (removal of the
'. $TEST_DIRECTORY/lib-t6000.sh' directive. Sorry about that ---
fixed now.
t/t6101-rev-parse-parents.sh | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 8a6ff66..7ea14ce 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -6,7 +6,6 @@
test_description='Test git rev-parse with different parent options'
. ./test-lib.sh
-. $TEST_DIRECTORY/lib-t6000.sh # t6xxx specific functions
test_cmp_rev_output () {
git rev-parse --verify $1 expect
@@ -14,14 +13,15 @@ test_cmp_rev_output () {
test_cmp expect actual
}
-date path0
-git update-index --add path0
-save_tag tree git write-tree
-hide_error save_tag start unique_commit start tree
-save_tag second unique_commit second tree -p start
-hide_error save_tag start2 unique_commit start2 tree
-save_tag two_parents unique_commit next tree -p second -p start2
-save_tag final unique_commit final tree -p two_parents
+test_expect_success 'setup' '
+ test_commit start
+ test_commit second
+ git checkout --orphan tmp
+ test_commit start2
+ git checkout master
+ git merge -m next start2
+ test_commit final
+'
test_expect_success 'start is valid' '
git rev-parse start | grep ^[0-9a-f]\{40\}$
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] rev-parse test: use standard test functions for setup
From: Felipe Contreras felipe.contre...@gmail.com
Save the reader from learning specialized t6* setup functions
where familiar commands like test_commit, git checkout --orphan,
and git merge will do.
While at it, wrap the setup commands in a test assertion so errors can
be caught and stray output suppressed when running without --verbose
as in other tests.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Thanks for reading.
t/t6101-rev-parse-parents.sh | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 8a6ff66..b5fa9e4 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -14,14 +14,15 @@ test_cmp_rev_output () {
test_cmp expect actual
}
-date path0
-git update-index --add path0
-save_tag tree git write-tree
-hide_error save_tag start unique_commit start tree
-save_tag second unique_commit second tree -p start
-hide_error save_tag start2 unique_commit start2 tree
-save_tag two_parents unique_commit next tree -p second -p start2
-save_tag final unique_commit final tree -p two_parents
+test_expect_success 'setup' '
+ test_commit start
+ test_commit second
+ git checkout --orphan tmp
+ test_commit start2
+ git checkout master
+ git merge -m next start2
+ test_commit final
+'
test_expect_success 'start is valid' '
git rev-parse start | grep ^[0-9a-f]\{40\}$
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] rev-parse test: use test_cmp instead of test builtin
Junio C Hamano wrote:
After applying this patch and running git show | grep test_cmp_rev_output,
I notice that the second is always git rev-parse something. Do
we still need to eval these, or would it be sufficient to do
test_cmp_rev_output () {
git rev-parse --verify $1 expect
git rev-parse --verify $2 actual
test_cmp expect actual
}
here, and make users like so:
- test_cmp_rev_output tags/start git rev-parse start^0
+ test_cmp_rev_output tags/start start^0
Am I missing something?
I was tempted to use test_cmp_rev, which would have the same effect.
The original test checked the output of git rev-parse start^0, while
the test as modified above checks the output of git rev-parse
--verify start^0.
I slightly prefer the version without --verify because git rev-parse
--verify is well exercised elsewhere in the testsuite (but then, so
is rev-parse without --verify, so it's not a big deal).
Abbreviating as follows
foo () {
git rev-parse --verify $1 expect
git rev-parse $2 actual
test_cmp expect actual
}
would also be fine with me. All it would take is a good replacement
for the placeholder foo. The added flexibility of compare a rev
to the output of an arbitrary command doesn't get used.
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/5] Disable git status comment prefix
Jeff King wrote: On Thu, Sep 05, 2013 at 09:36:47PM +0200, Matthieu Moy wrote: I'm fine with any name actually (since it is enabled by default, people don't need to know the name to benefit from the new output). Maybe status.displayCommentPrefix was the best name after all. FWIW, I had the same thought as Junio. I much prefer something like status.displayCommentPrefix for clarity and future-proofing. Sounds fine, but I don't understand why we'd want this to be an option with a future in the first place. Why not just fix the remaining bugs before merging to master and make it unconditional? [...] after: On branch private Your branch and 'origin/next' have diverged, and have 472 and 59 different commits each, respectively. Untracked files: t/foo test-obj-pool test-string-pool test-treap test-url-normalize nothing added to commit but untracked files present The blank before Untracked files was dropped (was this intentional? I didn't see any note of it in the discussion), That's a bug. and the bottom nothing added line butts against the untracked list more obviously, because they now all have the same comment indentation. I wonder if it would look a little nicer as: On branch private Your branch and 'origin/next' have diverged, and have 472 and 59 different commits each, respectively. Untracked files: t/foo test-obj-pool test-string-pool test-treap test-url-normalize nothing added to commit but untracked files present The added blank line before nothing added sounds like a good idea. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
John Keeping wrote:
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
upstream parameter to rebase.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] http-backend: provide Allow header for 405
Hi,
brian m. carlson wrote:
--- a/http-backend.c
+++ b/http-backend.c
@@ -594,8 +594,11 @@ int main(int argc, char **argv)
if (strcmp(method, c-method)) {
const char *proto = getenv(SERVER_PROTOCOL);
- if (proto !strcmp(proto, HTTP/1.1))
+ if (proto !strcmp(proto, HTTP/1.1)) {
http_status(405, Method Not Allowed);
+ hdr_str(Allow, !strcmp(GET,
c-method) ?
+ GET, HEAD : c-method);
+ }
else
Small style nit: the closing brace should go on the same line as the
else, like so:
if (proto ...) {
...
} else
http_status(400, Bad Request);
Another micronit: the comparison should be !strcmp(c-method, GET)
--- variable first, then constant it is being compared to.
The functional change looks good.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Disabling status hints in COMMIT_EDITMSG
Matthieu Moy wrote: I just noticed that the template COMMIT_EDITMSG was containing status hints, and that they were not particularty helpfull _during_ a commit. I think it would be sensible to ignore advice.statusHints and disable hints unconditionally when writting to COMMIT_EDITMSG. Any objection? No objection from me. It sounds like a good change. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Kevin Bracey wrote: On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x [...] 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. [...] 2) If rule 1 doesn't activate, and it remains as a merge, hide it if treesame to all on-graph parents. Previously this rule was hide if treesame to any parent, and so that would have hidden the merge. Now, when I changed rule 2, I did not think this would affect the default log. See my commit message: [...] I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Thanks for this analysis. Interesting. The rule (1) comes from v1.3.0-rc1~13^2~6: commit f3219fbbba32b5100430c17468524b776eb869d6 Author: Junio C Hamano jun...@cox.net Date: Fri Mar 10 21:59:37 2006 -0800 try_to_simplify_commit(): do not skip inspecting tree change at boundary. When git-rev-list (and git-log) collapsed ancestry chain to commits that touch specified paths, we failed to inspect and notice tree changes when we are about to hit uninteresting parent. This resulted in git rev-list since.. -- file to always show the child commit after the lower bound, even if it does not touch the file. This commit fixes it. Thanks for Catalin for reporting this. See also: 461cf59f8924f174d7a0dcc3d77f576d93ed29a4 Signed-off-by: Junio C Hamano jun...@cox.net I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth wrote: This is necessary so that read_mailmap() can obtain a pointer to the function. Hm, what platform has strcasecmp() as an inline function? Is this allowed by POSIX? Even if it isn't, should we perhaps just work around it by providing our own thin static function wrapper in mailmap.c? Curious, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-checkout: Move `--detach` flag in synopsis to correct command
Hi,
Junio C Hamano wrote:
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -9,7 +9,8 @@ SYNOPSIS
[verse]
'git checkout' [-q] [-f] [-m] [branch]
-'git checkout' [-q] [-f] [-m] [--detach] [commit]
+'git checkout' [-q] [-f] [-m] --detach [branch]
+'git checkout' [-q] [-f] [-m] [--detach] commit
'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] new_branch] [start_point]
'git checkout' [-f|--ours|--theirs|-m|--conflict=style] [tree-ish] [--]
paths...
'git checkout' [-p|--patch] [tree-ish] [--] [paths...]
@@ -62,7 +63,7 @@ that is to say, the branch is not reset/created unless git
checkout is
successful.
'git checkout' --detach [branch]::
-'git checkout' commit::
+'git checkout' [--detach] commit::
Looks sensible.
[...]
@@ -71,10 +72,13 @@ successful.
tree will be the state recorded in the commit plus the local
modifications.
+
-Passing `--detach` forces this behavior in the case of a branch (without
-the option, giving a branch name to the command would check out the branch,
-instead of detaching HEAD at it), or the current commit,
-if no branch is specified.
+Even though a branch name can be used to name a commit, you have to
+explicitly say `git checkout --detach branch` when you want to
+detach HEAD at the tip of the branch (`git checkout branch` will
+check out that branch without detaching HEAD). Omitting branch,
+i.e. `git checkout --detach` detaches HEAD at the tip of the current
+branch. When naming the commit in a form other than just a branch
+name, e.g. `master^0`, `HEAD~4`, `c2f3bf071e`, you can omit --detach.
Hm. I agree that the old explanation is overly convoluted, but I don't
think the replacement is clear enough yet. The Even though a branch
name can be used to name a commit, part forced me to pause for too
long --- why is this telling me that a branch can be used to name a
commit, and in what context?
I think the main problem with the old text is that it tried to say too
much in one sentence.
The explanation lower down of the --detach option does this rather
well:
--detach
Rather than checking out a branch to work on it, check
out a commit for inspection and discardable
experiments. This is the default behavior of
git checkout commit when commit is not a branch
name. See the DETACHED HEAD section below for
details.
How about splitting this into multiple paragraphs, like so? In the
suggestion below I also cleaned up the language a little.
git checkout --detach [branch], git checkout [--detach] commit
Prepare to work on top of commit, by detaching [...]
When the commit argument is a branch name, the --detach
option can be used to detach HEAD at the tip of the
branch ('git checkout branch' would check out that
branch without detaching HEAD).
Omitting branch detaches HEAD at the tip of the
current branch.
I'd leave out the last sentence about commits other than branch names,
since it is already implied by the [--detach] in the syntax.
Thanks and hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] http-backend: provide Allow header for 405
brian m. carlson wrote: Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Thanks again for noticing. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Junio C Hamano wrote: I think we would want something like below. Looks good to me, but -- 8 -- Subject: [PATCH] mailmap: work around implementations with pure inline strcasecmp On some systems, string.h has _only_ inline definition of strcasecmp Please specify which system we are talking about: s/some systems/MinGW 4.0/ [...] --- a/mailmap.c +++ b/mailmap.c @@ -52,6 +52,19 @@ static void free_mailmap_entry(void *p, const char *s) string_list_clear_func(me-namemap, free_mailmap_info); } +/* + * On some systems, string.h has _only_ inline definition of strcasecmp Likewise. With or without that change, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth wrote: And that's exactly what defining __NO_INLINE__ does. Granted, defining __NO_INLINE__ in the scope of string.h will also add a #define strcasecmp _stricmp; but despite it's name, defining __NO_INLINE__ does not imply a performance hit due to functions not being inlined because it's just the strncasecmp wrapper around _strnicmp that's being inlined, not _strnicmp itself. What I don't understand is why the header doesn't use static inline instead of extern inline. The former would seem to be better in every way for this particular use case. See also http://www.greenend.org.uk/rjk/tech/inline.html, section GNU C inline rules. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth wrote: I'm not too happy with the wording either. As I see it, even on MinGW runtime version 4.0 it's not true that string.h has _only_ inline definition of strcasecmp; there's also #define strncasecmp _strnicmp I assume you mean #define strcasecmp _stricmp, which is guarded by defined(__NO_INLINE__). I think what Junio meant is that by default (i.e., in the !defined(__NO_INLINE__) case) string.h uses __CRT_INLINE, defined as extern inline __attribute__((__gnu_inline__)) to suppress the non-inline function definition. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] urlmatch.c: recompute ptr after append_normalized_escapes
Kyle J. McKay wrote: The longer comment looks good to me. If you think the code will be safe from simplification patches without a comment, that works for me too. I think if we can't trust reviewers to catch this kind of thing, we're in trouble (i.e., moving too fast). :) So FWIW my instinct is to leave the comment out, since I actually find it more readable that way (otherwise I would wonder, Why am I being told that a strbuf's buffer has a nonconstant address? Do some other strbufs have a constant address or something?) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/4 v2] Implement a basic remote helper for svn in C.
Hi again, Florian Achleitner wrote: back to the pipe-topic. Ok, thanks. [...] The way it's supposed to work is that in a bidi-import, the remote helper reads in the entire list of refs to be imported and only once the newline indicating that that list is over arrives starts writing its fast-import stream. [...] This would require all existing remote helpers that use 'import' to be ported to the new concept, right? Probably there is no other.. You mean all existing remote helpers that use 'bidi-import', right? There are none. [...] I still don't believe that sharing the input pipe of the remote helper is worth the hazzle. It still requires an additional pipe to be setup, the one from fast-import to the remote-helper, sharing one FD at the remote helper. If I understand correctly, you misunderstood how sharing the input pipe works. Have you tried it? It does not involve setting up an additional pipe. Standard input for the remote helper is already a pipe. That pipe is what allows transport-helper.c to communicate with the remote helper. Letting fast-import share that pipe involves passing that file descriptor to git fast-import. No additional pipe() calls. Do you mean that it would be too much work to implement? This explanation just doesn't make sense to me, given that the version using pipe() *already* *exists* and is *tested*. I get the feeling I am missing something very basic. I would welcome input from others that shows what I am missing. [...] Another alternative would be to use the existing --cat-pipe-fd argument. But that requires to open the fifo before execing fast-import and makes us dependent on the posix model of forking and inheriting file descriptors, while opening a fifo in fast-import would not. I'm getting kind of frustrated with this conversation going nowhere. Here is a compromise to explain why. Suppose: - fast-import learns a --cat-blob-file parameter, which tells it to open a file to write responses to cat-blob and ls commands to instead of using an inherited file descriptor - transport-helper.c sets up a named pipe and passes it using --cat-blob-file. - transport-helper.c reads from that named pipe and copies everything it sees to the remote helper's standard input, until fast-import exits. This would: - allow us to continue to use a very simple protocol for communicating with the remote helper, where commands and fast-import backflow both come on standard input - work fine on both Windows and Unix Meanwhile it would: - be 100% functionally equivalent to the solution where fast-import writes directly to the remote helper's standard input. Two programs can have the same pipe open for writing at the same time for a few seconds and that is *perfectly fine*. On Unix and on Windows. On Windows the only complication with the pipe()-based is that we haven't wired up the low-level logic to pass file descriptors other than stdin, stdout, stderr to child processes; and if I have understood earlier messages correctly, the operating system *does* have a concept of that and this is just a todo item in msys implementation. - be more complicated than the code that already exists for this stuff. So while I presented this as a compromise, I don't see the point. Is your goal portability, a dislike of the interface, some implementation detail I have missed, or something else? Could you explain the problem as concisely but clearly as possible (perhaps using an example) so that others like Sverre, Peff, or David can help think through it and to explain it in a way that dim people like me understand what's going on? Puzzled, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/4 v2] Implement a basic remote helper for svn in C.
Hi again, Florian Achleitner wrote: Another alternative would be to use the existing --cat-pipe-fd argument. But that requires to open the fifo before execing fast-import and makes us dependent on the posix model of forking and inheriting file descriptors Let me elaborate on this, which I think is the real point (though I could easily be wrong). Probably instead of just sending feedback I should have been sending suggested patches to compare. You do not work for me and your time is not my time, and I would not want to have the responsibility of dictating how your code works, anyway. Generally speaking, as long as code is useful and not hurting maintainability, the best thing I can do is to make it easy to incorporate. That has side benefits, too, like giving an example of what it is like to have your code incorporated and creating momentum. Small mistakes can be fixed later. Unfortunately here we are talking about two interfaces that need to be stable. Mistakes stay --- once there is a UI wart in interfaces people are using, we are stuck continuing to support it. To make life even more complicated, there are two interfaces involved here: - the remote-helper protocol, which I think it is very important to keep sane and stable. Despite the remote-helper protocol existing for a long time, it hasn't seen a lot of adoption outside git yet, and complicating the protocol will not help that. I am worried about what it would mean to add an additional stream on top of the standard input and output used in the current remote-helper protocol. Receiving responses to fast-import commands on stdin seems very simple. Maybe I am wrong? - the fast-import interface, which is also important but I'm not as worried about. Adding a new command-line parameter means importers that call fast-import can unnecessarily depend on newer versions of git, so it's still something to be avoided. Given a particular interface, there may be multiple choices of how to implement it. For example, on Unix the remote-helper protocol could be fulfilled by letting fast-import inherit the file descriptor helper-in, while on Windows it could for example be fulfilled by using DuplicateHandle() to transmit the pipe handle before or after fast-import has already started running. The implementation strategy can easily be changed later. What is important is that the interface be pleasant and *possible* to implement sanely. Hoping that clarifies a little, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/4 v2] Implement a basic remote helper for svn in C.
Florian Achleitner wrote: This is how I see it, probably it's all wrong: I thought the main problem is, that we don't want processes to have *more than three pipes attached*, i.e. stdout, stdin, stderr, because existing APIs don't allow it. Oh, that makes sense. Thanks for explaining, and sorry to have been so dense. At the Windows API level, Set/GetStdHandle() is only advertised to handle stdin, stdout, and stderr, so on Windows there would indeed need to be some magic to communicate the inherited HANDLE value to fast-import. But I am confident in the Windows porters, and if a fast-import interface change ends up being needed, I think we can deal with it when the moment comes and it wouldn't necessitate changing the remote helper interface. You also mentioned before that passing fast-import responses to the remote helper stdin means that the remote helper has to slurp up the whole list of refs to import before starting to talk to fast-import. That was good practice already anyway, but it is a real pitfall and a real downside to the single-input approach. Thanks for bringing it up. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 01/16] GSOC remote-svn
Hi, Florian Achleitner wrote: What version would you prefer? I think git-remote-svn should move out of contrib to the toplevel of git.git (as I think I've mentioned before). Since it's just a new git command in incubation, I don't want the maintenance hassle of keeping it in contrib/svn-fe. It can be a test program for now (e.g., git-remote-testsvn). Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/5] GSOC: prepare svndump for branch detection
Florian Achleitner wrote: Currently, the mark number is equal to the svn revision number the commit corresponds to. I didn't want to break that, but not mandatory. We could also split the mark namespace by reserving one or more of the most significant bits as a type specifier. I'll develop a marks-based version .. Have we already exhausted possibilities that don't involve changing vcs-svn/ code quite so much? One possibility mentioned before was to post-process the stream that svn-fe produces, which seemed appealing from a debuggability point of view. Curious, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
How to request a fast-forward pull
Hi gitsters, Paul Gortmaker wrote: When you have a moment, would you please migrate this across to your main linux-stable repository? Both a branch and signed tag are present and pointing at the same commit, but git request-pull does favour output of the tag over the branch name. But merging the tag will want to create a merge commit. So, to avoid a merge commit in your repo, you can fetch (fast fwd) into your (local) branch from my branch at: git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux-stable.git linux-2.6.34.y and then fetch the signed tag listed below after that. Can this be made easier? I could imagine request-pull learning --ff-only that generates a message like Greg, Please pull --ff-only git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux-stable.git linux-2.6.34.y to get the following changes [...] which could work ok if the recipient notices the --ff-only, but I wonder if there is a simpler way. Thanks for the food for thought, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] Fix tests under GETTEXT_POISON on pack-object
Hi, Nguyễn Thái Ngọc Duy wrote: From: Jiang Xin worldhello@gmail.com Use i18n-specific test functions in test scripts for pack-object. Thanks for resending, and sorry I haven't made time to polish the translation-based poison implementation you sent before (which seemed very useful and pleasant to work with). [...] --- a/t/t5530-upload-pack-error.sh +++ b/t/t5530-upload-pack-error.sh @@ -35,7 +35,7 @@ test_expect_success 'upload-pack fails due to error in pack-objects packing' ' printf 0032want %s\n0009done\n \ $(git rev-parse HEAD) input test_must_fail git upload-pack . input /dev/null 2output.err - grep unable to read output.err + test_i18ngrep unable to read output.err grep pack-objects died output.err Wouldn't it make sense to change the second grep of output intended for humans to test_i18ngrep while at it? With or without that change, this and the rest of the series looks good. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Build in gettext poison feature unconditionally
Nguyễn Thái Ngọc Duy wrote: The runtime cost should be small. gcc -O2 inlines _() and use_gettext_poison(). But even if it does not, performance should not be impacted as _() calls are usually not on critical path. If some of them are, we better fix there as gettext() may or may not be cheap anyway. That seems reasonable to me. The increase in code size of a commonly inlined function and the extra if in a common if not performance-critical codepath is annoying (and I'd prefer to keep use_gettext_poison() un-inlined), but in any event the cost would go away once the translation-based implementation of poison lands. [...] I don't know the story behind this compile-time switch. The series [1] that introduces it does not say much. I think it was just paranoia about performance regressions. This at least makes it easier for me to run poison tests instead of building another binary, if I remember it. Next step could be make make test run both normal and poison modes, but I'm not sure how to do it nicely yet. Yes, that would be nice (or perhaps a mode to run most tests in the current locale and rerun test assertions that use a test_i18n* helper or C_LOCALE_OUTPUT prerequisite in the C locale). Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] Fix tests under GETTEXT_POISON on pack-object
Nguyen Thai Ngoc Duy wrote: On Tue, Aug 21, 2012 at 12:17 PM, Jonathan Nieder jrnie...@gmail.com wrote: @@ -35,7 +35,7 @@ test_expect_success 'upload-pack fails due to error in pack-objects packing' ' printf 0032want %s\n0009done\n \ $(git rev-parse HEAD) input test_must_fail git upload-pack . input /dev/null 2output.err - grep unable to read output.err + test_i18ngrep unable to read output.err grep pack-objects died output.err Wouldn't it make sense to change the second grep of output intended for humans to test_i18ngrep while at it? This comes from error(git upload-pack: git-pack-objects died with error.) in unpack-trees.c, which is not i18n-ized yet. There's another test in t5530 that does the same grep. I think we should leave it as is until we mark the string for translation, then gettext poison will spot it (verified) and we can fix it. I don't understand the distinction you're making. Isn't the message intended for humans, and wouldn't changing that one line to test_i18ngrep now save trouble later? Tests are meant to check git's intended behavior, not to exactly match its current behavior. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] Make Git::SVN use accessors internally for path.
Hi Eric et al,
Michael G. Schwern wrote:
Then later it can be canonicalized automatically rather than everywhere
its used.
Later patch will make other things use it.
Wow am I slow. I've finally got around to starting to parse these
patches to apply to a 1.7.10.y tree so they can (hopefully) be part of
Debian 7.0 when it comes out.
Do I understand correctly that this patch splits logically into the
following steps? The result is only cosmetically different from the
original patch --- interdiff below the shortlog.
The completeness of the conversion to accessors is checked by renaming
the underlying variable in patch 5.
Jonathan Nieder (1):
Git::SVN: rename private path field
Michael G. Schwern (4):
Git::SVN: introduce path accessor
Git::SVN: use accessor to read path
Git::SVN: use accessor to write path
Git::SVN::_new: use accessor to write path field
git-svn.perl| 12 +++
perl/Git/SVN.pm | 84 ---
perl/Git/SVN/Fetcher.pm |2 +-
perl/Git/SVN/Ra.pm |8 ++---
4 files changed, 62 insertions(+), 44 deletions(-)
---
diff --git c/git-svn.perl w/git-svn.perl
index 5711c571..af7d5308 100755
--- c/git-svn.perl
+++ w/git-svn.perl
@@ -1195,7 +1195,7 @@ sub cmd_show_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:ignore'} or return;
@@ -1211,7 +1211,7 @@ sub cmd_show_externals {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:externals'} or return;
@@ -1226,7 +1226,7 @@ sub cmd_create_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
# $path is of the form /path/to/dir/
$path = '.' . $path;
@@ -1294,7 +1294,7 @@ sub get_svnprops {
$path = $cmd_dir_prefix . $path;
fatal(No such file or directory: $path) unless -e $path;
my $is_dir = -d $path ? 1 : 0;
- $path = $gs-{path} . '/' . $path;
+ $path = $gs-path . '/' . $path;
# canonicalize the path (otherwise libsvn will abort or fail to
# find the file)
@@ -1396,7 +1396,7 @@ sub cmd_commit_diff {
the command-line\n, $usage);
}
$url = $gs-{url};
- $svn_path = $gs-{path};
+ $svn_path = $gs-path;
}
unless (defined $_revision) {
fatal(-r|--revision is a required argument\n, $usage);
@@ -1670,7 +1670,7 @@ sub complete_url_ls_init {
wanted to set to: $gs-{url}\n;
}
command_oneline('config', $k, $gs-{url}) unless $orig_url;
- my $remote_path = $gs-{path}/$repo_path;
+ my $remote_path = $gs-path . /$repo_path;
$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
$remote_path =~ s#/+#/#g;
$remote_path =~ s#^/##g;
diff --git c/perl/Git/SVN.pm w/perl/Git/SVN.pm
index a93ac61b..3aa20109 100644
--- c/perl/Git/SVN.pm
+++ w/perl/Git/SVN.pm
@@ -437,22 +437,19 @@ sub new {
}
}
my $self = _new($class, $repo_id, $ref_id, $path);
- if (!defined $self-path || !length $self-path) {
+ $path = $self-path;
+ if (!defined $path || !length $path) {
my $fetch = command_oneline('config', '--get',
svn-remote.$repo_id.fetch,
:$ref_id\$) or
die Failed to read \svn-remote.$repo_id.fetch\ ,
\:$ref_id\$\ in config\n;
- my($path) = split(/\s*:\s*/, $fetch);
- $self-path($path);
- }
- {
- my $path = $self-path;
- $path =~ s{/+}{/}g;
- $path =~ s{\A/}{};
- $path =~ s{/\z}{};
- $self-path($path);
+ ($path, undef) = split(/\s*:\s*/, $fetch);
}
+ $path =~ s{/+}{/}g;
+ $path =~ s{\A/}{};
+ $path =~ s{/\z}{};
+ $self-path($path);
$self-{url} = command_oneline('config', '--get',
svn-remote.$repo_id.url
[FYI/PATCH 2/5] Git::SVN: use accessor to read path
From: Michael G. Schwern schw...@pobox.com
Date: Fri, 27 Jul 2012 13:00:48 -0700
This patch only touches the simplest cases that simply read the
Git::SVN field rather than assigning to or applying a substitution to
it.
Code to change found by searching for the term {path}.
[jn: extracted from a larger patch]
Signed-off-by: Eric Wong normalper...@yhbt.net
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
git-svn.perl| 12 ++--
perl/Git/SVN.pm | 44 ++--
perl/Git/SVN/Fetcher.pm |2 +-
perl/Git/SVN/Ra.pm |8
4 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 5711c571..af7d5308 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1195,7 +1195,7 @@ sub cmd_show_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:ignore'} or return;
@@ -1211,7 +1211,7 @@ sub cmd_show_externals {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
print STDOUT \n# $path\n;
my $s = $props-{'svn:externals'} or return;
@@ -1226,7 +1226,7 @@ sub cmd_create_ignore {
my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
$gs ||= Git::SVN-new;
my $r = (defined $_revision ? $_revision : $gs-ra-get_latest_revnum);
- $gs-prop_walk($gs-{path}, $r, sub {
+ $gs-prop_walk($gs-path, $r, sub {
my ($gs, $path, $props) = @_;
# $path is of the form /path/to/dir/
$path = '.' . $path;
@@ -1294,7 +1294,7 @@ sub get_svnprops {
$path = $cmd_dir_prefix . $path;
fatal(No such file or directory: $path) unless -e $path;
my $is_dir = -d $path ? 1 : 0;
- $path = $gs-{path} . '/' . $path;
+ $path = $gs-path . '/' . $path;
# canonicalize the path (otherwise libsvn will abort or fail to
# find the file)
@@ -1396,7 +1396,7 @@ sub cmd_commit_diff {
the command-line\n, $usage);
}
$url = $gs-{url};
- $svn_path = $gs-{path};
+ $svn_path = $gs-path;
}
unless (defined $_revision) {
fatal(-r|--revision is a required argument\n, $usage);
@@ -1670,7 +1670,7 @@ sub complete_url_ls_init {
wanted to set to: $gs-{url}\n;
}
command_oneline('config', $k, $gs-{url}) unless $orig_url;
- my $remote_path = $gs-{path}/$repo_path;
+ my $remote_path = $gs-path . /$repo_path;
$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
$remote_path =~ s#/+#/#g;
$remote_path =~ s#^/##g;
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 268e0e84..02d5abc0 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -314,7 +314,7 @@ sub init_remote_config {
print STDERR Using higher level of URL: ,
$url = $min_url\n;
}
- my $old_path = $self-{path};
+ my $old_path = $self-path;
$self-{path} = $url;
$self-{path} =~ s!^\Q$min_url\E(/|$)!!;
if (length $old_path) {
@@ -347,7 +347,7 @@ sub init_remote_config {
$self-{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
command_noisy('config', '--add',
svn-remote.$self-{repo_id}.fetch,
- $self-{path}:.$self-refname);
+ $self-path.:.$self-refname);
}
$self-{url} = $url;
}
@@ -435,7 +435,7 @@ sub new {
}
}
my $self = _new($class, $repo_id, $ref_id, $path);
- if (!defined $self-{path} || !length $self-{path}) {
+ if (!defined $self-path || !length $self-path) {
my $fetch = command_oneline('config', '--get',
svn-remote.$repo_id.fetch,
:$ref_id\$) or
@@ -567,7 +567,7 @@ sub _set_svm_vars {
}
my $r = $ra-get_latest_revnum;
- my $path = $self-{path};
+ my $path = $self-path;
my %tried;
while (length $path) {
unless ($tried{$self-{url}/$path}) {
@@ -728,7 +728,7 @@ sub prop_walk {
$path =~ s#^/*#/#g;
my $p = $path
[FYI/PATCH 4/5] Git::SVN::_new: use accessor to write path field
From: Michael G. Schwern schw...@pobox.com
Date: Fri, 27 Jul 2012 13:00:48 -0700
If some day the setter is taught to canonicalize paths, make sure the
path gets canonicalized at construction time, too.
[jn: split from a larger patch]
Signed-off-by: Eric Wong normalper...@yhbt.net
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
perl/Git/SVN.pm |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 826a7fa6..3aa20109 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -2280,10 +2280,12 @@ sub _new {
$_[3] = $path = '' unless (defined $path);
mkpath([$dir]);
- bless {
+ my $obj = bless {
ref_id = $ref_id, dir = $dir, index = $dir/index,
- path = $path, config = $ENV{GIT_DIR}/svn/config,
+ config = $ENV{GIT_DIR}/svn/config,
map_root = $dir/.rev_map, repo_id = $repo_id }, $class;
+ $obj-path($path);
+ return $obj;
}
sub path {
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.
Hi, Michael G Schwern wrote: On 2012.7.30 12:51 PM, Eric Wong wrote: Michael G Schwern wrote: _collapse_dotdot() works better than the existing regex did. I don't dispute it's better, but it's worth explaining in the commit message to reviewers why something is better. Yeah. I figured the tests covered that. Now I'm tripping up on the same thing. Eric, did you ever find out what the motivation for this patch was? Is SVN 1.7 more persnickety about runs of multiple slashes in a row or something, or is it more of an aesthetic thing? Puzzled, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] Add join_paths() to safely concatenate paths.
Hi,
Michael G. Schwern wrote:
Otherwise you might wind up with things like...
my $path1 = undef;
my $path2 = 'foo';
my $path = $path1 . '/' . $path2;
creating '/foo'. Or this...
my $path1 = 'foo/';
my $path2 = 'bar';
my $path = $path1 . '/' . $path2;
creating 'foo//bar'.
I'm still puzzled by this one, too. I don't understand the
motivation. Is this to make joining paths less fragile, by preserving
the property that join_paths($a, $b) names the directory you would get
to by first chdir-ing into $a and then into $b?
It would be easier to understand as two patches: first, one that
extracts join_paths without any functional change, and then one that
changes its implementation with an explanation for what positive
functional effect that would have.
--- a/git-svn.perl
+++ b/git-svn.perl
[...]
@@ -1275,7 +1276,7 @@ sub get_svnprops {
$path = $cmd_dir_prefix . $path;
fatal(No such file or directory: $path) unless -e $path;
my $is_dir = -d $path ? 1 : 0;
- $path = $gs-{path} . '/' . $path;
+ $path = join_paths($gs-{path}, $path);
# canonicalize the path (otherwise libsvn will abort or fail to
# find the file)
This can't be for the //-collapsing effect since the path is about
to be canonicalized. It can't be for the initial-/ effect since
that is stripped away by canonicalization, too.
So no functional effect here, good or bad.
[...]
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
[...]
@@ -316,9 +320,7 @@ sub init_remote_config {
}
my $old_path = $self-path;
$url =~ s!^\Q$min_url\E(/|$)!!;
- if (length $old_path) {
- $url .= /$old_path;
- }
+ $url = join_paths($url, $old_path);
$self-path($url);
This is probably not for the normal //-collapsing effect because
$url already has its trailing / stripped off. Maybe it is for
cases where $old_path has leading slashes or $min_url has multiple
trailing ones?
In the end it shouldn't make a difference, once a later patch teaches
Git::SVN-path to canonicalize.
Is the functional change in this patch for aesthetic reasons, or is
there some other component (perhaps in a later patch) that relies on
it?
Thanks again for your help,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.
Eric Wong wrote:
That said, I'd favor an implementation that split on m{/+} and
collapsed as Michael mentioned.
Sounds sensible. Is canonicalize_path responsible for collapsing
runs of slashes? What should _collapse_dotdot do to
c:/.. or http://www.example.com/..;?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.
Eric Wong wrote:
It should probably just return the root path (c:/ and
http://www.example.com/; respectively).
That means recognizing drive letters and URLs. Hm.
Subversion commands seem to use svn_client_args_to_target_array2
to canonicalize arguments. It does something like the following:
1. split at @PEG revision
2. check if it looks like a URL (svn_path_is_url). If so:
i. urlencode characters with high bit set (svn_path_uri_from_iri)
ii. urlencode some other special characters (svn_path_uri_autoescape)
(that is: [ \\^`{|}])
iii. (on Windows) convert backslashes to forward slashes
iv. complain if there are any '..' (svn_path_is_backpath_present)
v. make url scheme and hostname lowercase, strip default portnumber,
strip trailing '/', collapse '/'-es including urlencoded %2F,
strip '.' and '%2E' components, make drive letter in file:///
URLs uppercase, urlencode uri-special characters
(svn_uri_canonicalize)
Otherwise:
i. canonicalize case, handle '..' and '.' components, and make
path separators into '/'
(apr_filepath_merge(APR_FILEPATH_TRUENAME))
ii. strip trailing '/', collapse '/'-es except in UNC paths,
strip '.' components, make drive letter uppercase
(svn_dirent_canonicalize)
iii. deal with truepath collisions (unwanted case
canonicalizations)
iv. reject .svn and _svn directories
Maybe we can use apr_filepath_merge() to avoid reinventing the wheel?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.
Eric Wong wrote: Ideally, yes. Is there an easy way to access that from Perl? (and for the older versions of SVN folks people are running). Subversion's swig bindings only wrap a few apr functions and do not depend on fuller apr bindings. Something like svn_dirent_is_under_root() could be useful. It uses whatever base path you choose. I haven't tried using base_path= yet. New in Subversion 1.7, but that would be ok --- an imperfect fallback for older Subversion versions would be fine. Unfortunately, from swig/core.i: SWIG can't digest these functions yet, so ignore them for now. TODO: make them work. svn_client_args_to_target_array2() is exposed and would probably be perfect. I can't seem to find how to create an apr_array_header_t to pass as its first argument, alas. Perhaps we can expose equivalent functionality in git via git-rev-parse instead? It might be possible to add a git-svn--helper command that links to libsvn or apr, but ick. The trouble is it's not clear at all how Subversion's perl API was *designed* to be used. Hm. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: refresh the index before running diff-files
Hi Jeff, Jeff King wrote: On Sun, Sep 30, 2012 at 10:05:27AM +1000, Paul Mackerras wrote: Unfortunately this will wait for the git update-index command to complete, making the GUI unresponsive while it executes, and that can take minutes on a large repository (e.g. the linux kernel) on a machine with a slow disk and a cold disk cache. We will need to make the git update-index execute asynchronously. Good point. We're getting out of my very limited tcl cargo-culting skills now, so I'll let somebody more clueful do that fix. You might find the following patch and discussion entertaining: http://thread.gmane.org/gmane.comp.version-control.git/144182 Not my itch, but it was fun to write back then. ;-) Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fa/remote-svn (Re: What's cooking in git.git (Oct 2012, #01; Tue, 2))
David Michael Barr wrote: On Wednesday, 3 October 2012 at 9:20 AM, Junio C Hamano wrote: * fa/remote-svn (2012-09-19) 16 commits - Add a test script for remote-svn - remote-svn: add marks-file regeneration - Add a svnrdump-simulator replaying a dump file for testing - remote-svn: add incremental import - remote-svn: Activate import/export-marks for fast-import - Create a note for every imported commit containing svn metadata - vcs-svn: add fast_export_note to create notes - Allow reading svn dumps from files via file:// urls - remote-svn, vcs-svn: Enable fetching to private refs - When debug==1, start fast-import with --stats instead of --quiet - Add documentation for the 'bidi-import' capability of remote-helpers - Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability - Add argv_array_detach and argv_array_free_detached - Add svndump_init_fd to allow reading dumps from arbitrary FDs - Add git-remote-testsvn to Makefile - Implement a remote helper for svn in C (this branch is used by fa/vcs-svn.) A GSoC project. Waiting for comments from mentors and stakeholders. I have reviewed this topic and am happy with the design and implementation. I support this topic for inclusion. Thanks! I'll try moving the tests to the first patch and trying it and hopefully send out a branch to pull tomorrow. If I don't send anything tomorrow, that's probably a sign that I never will, so since I like the goal of the series I guess it would be a kind of implied ack. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-svn: keep leading slash when canonicalizing paths (fallback case)
Subversion's svn_dirent_canonicalize() and svn_path_canonicalize()
APIs keep a leading slash in the return value if one was present on
the argument, which can be useful since it allows relative and
absolute paths to be distinguished.
When git-svn's canonicalize_path() learned to use these functions if
available, its semantics changed in the corresponding way. Some new
callers rely on the leading slash --- for example, if the slash is
stripped out then _canonicalize_url_ourselves() will transform
proto://host/path/to/resource to proto://hostpath/to/resource.
Unfortunately the fallback _canonicalize_path_ourselves(), used when
the appropriate SVN APIs are not usable, still follows the old
semantics, so if that code path is exercised then it breaks. Fix it
to follow the new convention.
Noticed by forcing the fallback on and running tests. Without this
patch, t9101.4 fails:
Bad URL passed to RA layer: Unable to open an ra_local session to \
URL: Local URL 'file://homejrnsrcgit-scratch/t/trash%20directory.\
t9101-git-svn-props/svnrepo' contains unsupported hostname at \
/home/jrn/src/git-scratch/perl/blib/lib/Git/SVN.pm line 148
With it, the git-svn tests pass again.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Hi Eric,
Michael G Schwern wrote:
On 2012.7.28 6:55 AM, Jonathan Nieder wrote:
When would this else case trip?
When svn_path_canonicalize() does not exist in the SVN API, presumably because
their SVN is too old.
I accidentally tested this else branch by making the other cases
false. t9101.4 failed as described above, or in other words,
canonicalize_url_ourselves() stripped out a few too many slashes. For
reference:
| sub _canonicalize_url_ourselves {
| my ($url) = @_;
| if ($url =~ m#^([^:]+)://([^/]*)(.*)$#) {
| my ($scheme, $domain, $uri) = ($1, $2,
_canonicalize_url_path(canonicalize_path($3)));
| $url = $scheme://$domain$uri;
| }
| $url;
| }
When $url is http://host/path/to/resource,
$1 = http, $2 = host, $3 = /path/to/resource
canonicalize_path($3) = path/to/resource --- (??)
_canonicalize_url_path(ditto) = path/to/resource
$url = http://hostpath/to/resource;
How about this patch?
perl/Git/SVN/Utils.pm |1 -
1 file changed, 1 deletion(-)
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 4bb4dde8..8b8cf375 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -122,7 +122,6 @@ sub _canonicalize_path_ourselves {
$path = _collapse_dotdot($path);
$path =~ s#/$##g;
$path =~ s#^\./## if $dot_slash_added;
- $path =~ s#^/##;
$path =~ s#^\.$##;
return $path;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] test: work around SVN 1.7 mishandling of svn:special changes
Subversion represents symlinks as ordinary files with content
starting with link and the svn:special property set to *. Thus a
file can switch between being a symlink and a non-symlink simply by
toggling its svn:special property, and new checkouts will
automatically write a file of the appropriate type. Likewise, in
subversion 1.6 and older, running svn update would notice changes
in filetype and update the working copy appropriately.
Unfortunately, starting in subversion 1.7 ,changes to the svn:special
property trip an assertion instead:
$ svn up svn-tree
Updating 'svn-tree':
svn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \
line 1583: assertion failed (action == svn_wc_conflict_action_edit \
|| action == svn_wc_conflict_action_delete || action == \
svn_wc_conflict_action_replace)
This is a known bug in svn update (Subversion issue 4091) and for
the sake of old repositories it will need to be fixed some day.
Revisions prepared with ordinary svn commands (svn add and not svn
propset) don't trip this because they represent filetype changes
using a replace operation, which is approximately equivalent to
removal followed by adding a new file and works fine. Perhaps git
svn should mimic that, but for now let's teach the test suite to
recover from the bug by testing the content of HEAD with a new
checkout.
After this change, tests t9100.11-13 pass again.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Hi Eric,
Michael G. Schwern wrote:
At this point SVN 1.7 passes except for 3 tests in
t9100-git-svn-basic.sh that look like an SVN bug to do with
symlinks.
How about this patch?
I didn't add a new xfail test for svn up working because I'm not yet
sure what good git-svn behavior would be. Probably it would be better
to track down that svn bug and get a fix backported to the 1.7.x
branch.
Reference: http://subversion.tigris.org/issues/show_bug.cgi?id=4160
t/t9100-git-svn-basic.sh | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 749b75e8..34d3485f 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -19,6 +19,15 @@ case $GIT_SVN_LC_ALL in
;;
esac
+svn_up_avoiding_issue4091 () {
+ if ! svn_cmd_up $SVN_TREE
+ then
+ # work around Subversion issue 4091
+ rm -r $SVN_TREE
+ svn_cmd checkout $svnrepo $SVN_TREE
+ fi
+}
+
test_expect_success \
'initialize git svn' '
mkdir import
@@ -148,7 +157,7 @@ test_expect_success $name '
git commit -m $name
git svn set-tree --find-copies-harder --rmdir \
${remotes_git_svn}..mybranch5
- svn_cmd up $SVN_TREE
+ svn_up_avoiding_issue4091
test -h $SVN_TREE/exec.sh'
name='new symlink is added to a file that was also just made executable'
@@ -173,7 +182,7 @@ test_expect_success $name '
git commit -m $name
git svn set-tree --find-copies-harder --rmdir \
${remotes_git_svn}..mybranch5
- svn_cmd up $SVN_TREE
+ svn_up_avoiding_issue4091
test -f $SVN_TREE/exec-2.sh
test ! -h $SVN_TREE/exec-2.sh
test_cmp help $SVN_TREE/exec-2.sh'
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [git-users] How do I git-push to an FTP server?
Ramkumar Ramachandra wrote: Thomas Ferris Nicolaisen tfn...@gmail.com writes: At least according to the documentation[1], Git natively supports [...] ftp. This could need some clarification if pushing over ftp is not supported. [...] -Git natively supports ssh, git, http, https, ftp, ftps, and rsync -protocols. The following syntaxes may be used with them: +Git natively supports ssh, git, http, https, and rsync protocols. The +following syntaxes may be used with them: Perhaps the initial list should not be exhaustive, in which case we could say: Git natively supports ssh, git, http, and https protocols. The following syntaxes may be used with them: ... Git also has (less efficient) support for fetching and pushing over rsync protocol and fetching over ftp or ftps, using the same protocol://host/path/to/repo.git/ syntax. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [git-users] How do I git-push to an FTP server?
Ramkumar Ramachandra wrote: I see. Will we remove ftp[s] support too? I hope this is in order. I don't see why that would be desirable, as long as libcurl continues to support it for free. [...] Fetching and pushing over rsync, and fetching over ftp or ftps are deprecated, and will soon be removed. Add a note saying this. I thought the real rationale was to avoid creating the illusion of supporting push over ftp. Having a paper trail to comfort people who notice when rsync support vanishes is just an added bonus. [...] @@ -31,6 +29,11 @@ syntaxes may be used: - /path/to/repo.git/ - file:///path/to/repo.git/ + Git also has (less efficient) support for fetching and pushing over + rsync protocol and fetching over ftp or ftps, using the same + protocol://host/path/to/repo.git/ syntax. However, these are + deprecated, and will soon be removed. I'd suggest dropping , and will soon be removed. or replacing it with . Don't use them. to avoid the question of how soon soon is. With that change and with a clearer commit message, this will probably be good to go imho. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [git-users] How do I git-push to an FTP server?
Junio C Hamano wrote: Let's do this, then. I think it would be nicer to start with the important info (git supports ssh, git, http, https) and deal with less important parts like rsync support later in the document, but this looks like a good minimal fix. Thanks for pushing it to completion. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] svn test: escape peg revision separator using empty peg rev
This test script uses svn cp to create a branch with an @-sign in
its name:
svn cp pr ject/trunk pr ject/branches/not-a@{0}reflog
That sets up for later tests that fetch the branch and check that git
svn mangles the refname appropriately.
Unfortunately, modern svn versions interpret path arguments with an
@-sign as an example of path@revision syntax (which pegs a path to a
particular revision) and truncate the path or error out with message
svn: E205000: Syntax error parsing peg revision '{0}reflog'.
When using subversion 1.6.x, escaping the @ sign as %40 avoids trouble
(see 08fd28bb, 2010-07-08). Newer versions are stricter:
$ svn cp $repo/pr ject/trunk $repo/pr ject/branches/not-a%40{reflog}
svn: E205000: Syntax error parsing peg revision '%7B0%7Dreflog'
The recommended method for escaping a literal @ sign in a path passed
to subversion is to add an empty peg revision at the end of the path
(branches/not-a@{0}reflog@). Do that.
Pre-1.6.12 versions of Subversion probably treat the trailing @ as
another literal @-sign (svn issue 3651). Luckily ever since
v1.8.0-rc0~155^2~7 (t9118: workaround inconsistency between SVN
versions, 2012-07-28) the test can survive that.
Tested with Debian Subversion 1.6.12dfsg-6 and 1.7.5-1 and r1395837
of Subversion trunk (1.8.x).
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Michael G Schwern wrote:
On 2012.7.28 7:16 AM, Jonathan Nieder wrote:
Michael G. Schwern wrote:
- git rev-parse refs/remotes/not-a%40{0}reflog
+ git rev-parse refs/remotes/$non_reflog
Doesn't this defeat the point of the testcase (checking that git-svn
is able to avoid creating git refs containing @{, following the rules
from git-check-ref-format(1))?
Unless I messed up, entirely possible as I'm not a shell programmer, the test
is still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be
'not-a%40{0}reflog' as before.
Here's a patch to make the test useful again for SVN 1.7. Sensible?
t/t9118-git-svn-funky-branch-names.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9118-git-svn-funky-branch-names.sh
b/t/t9118-git-svn-funky-branch-names.sh
index 193d3cab..15f93b4c 100755
--- a/t/t9118-git-svn-funky-branch-names.sh
+++ b/t/t9118-git-svn-funky-branch-names.sh
@@ -28,7 +28,7 @@ test_expect_success 'setup svnrepo' '
svn_cmd cp -m trailing .lock $svnrepo/pr ject/trunk \
$svnrepo/pr ject/branches/trailing_dotlock.lock
svn_cmd cp -m reflog $svnrepo/pr ject/trunk \
- $svnrepo/pr ject/branches/not-a%40{0}reflog
+ $svnrepo/pr ject/branches/not-a@{0}reflog@
start_httpd
'
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2] git svn: work around SVN 1.7 mishandling of svn:special changes
Subversion represents symlinks as ordinary files with content starting
with link and the svn:special property set to *. Thus a file can
switch between being a symlink and a non-symlink simply by toggling
its svn:special property, and new checkouts will automatically write a
file of the appropriate type. Likewise, in subversion 1.6 and older,
running svn update would notice changes in filetype and update the
working copy appropriately.
Starting in subversion 1.7 (issue 4091), changes to the svn:special
property trip an assertion instead:
$ svn up svn-tree
Updating 'svn-tree':
svn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \
line 1583: assertion failed (action == svn_wc_conflict_action_edit \
|| action == svn_wc_conflict_action_delete || action == \
svn_wc_conflict_action_replace)
Revisions prepared with ordinary svn commands (svn add and not svn
propset) don't trip this because they represent these filetype
changes using a replace operation, which is approximately equivalent
to removal followed by adding a new file and works fine. Follow suit.
Noticed using t9100. After this change, git-svn's file-to-symlink
changes are sent in a format that modern svn update can handle and
tests t9100.11-13 pass again.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Jonathan Nieder wrote:
Revisions prepared with ordinary svn commands (svn add and not svn
propset) don't trip this because they represent filetype changes
using a replace operation [...]
Perhaps git
svn should mimic that,
... and here's what that looks like. I like this more than ignoring
the problem in tests, and I suppose something like this is necessary
regardless of how quickly issue 4091 is fixed for compatibility with
the broken versions of svn.
It would be nice if the patch could be more concise, though. What do
you think?
git-svn.perl | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/git-svn.perl b/git-svn.perl
index 9d57aa0c..40ccdd50 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5439,7 +5439,30 @@ sub M {
$self-close_file($fbat,undef,$self-{pool});
}
-sub T { shift-M(@_) }
+sub T {
+ my ($self, $m, $deletions) = @_;
+
+ # Work around subversion issue 4091: toggling the is a
+ # symlink property requires removing and re-adding a
+ # file or else svn up on affected clients trips an
+ # assertion and aborts.
+ if (($m-{mode_b} =~ /^120/ $m-{mode_a} !~ /^120/) ||
+ ($m-{mode_b} !~ /^120/ $m-{mode_a} =~ /^120/)) {
+ $self-D({
+ mode_a = $m-{mode_a}, mode_b = '00',
+ sha1_a = $m-{sha1_a}, sha1_b = '0' x 40,
+ chg = 'D', file_b = $m-{file_b}
+ });
+ $self-A({
+ mode_a = '00', mode_b = $m-{mode_b},
+ sha1_a = '0' x 40, sha1_b = $m-{sha1_b},
+ chg = 'A', file_b = $m-{file_b}
+ });
+ return;
+ }
+
+ $self-M($m, $deletions);
+}
sub change_file_prop {
my ($self, $fbat, $pname, $pval) = @_;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Michael J Gruber wrote: Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41: Signed-off-by: Jonathan Nieder jrnie...@gmail.com Tested with Subversion 1.6.18. [...] I haven't checked other svn versions but this approach looks perfectly sensible. It makes us test branch names which can't even be created easily with current svn. Does svn really deserve this much attention? ;) Thanks for the quick and thorough feedback. I'm glad to hear it seems sane. ;-) Seriously, our tests prepare us well for an svn remote helper... That might be a good reason to make a mock implementation of the existing git-svn interface on top of git-remote-svn. Sounds fun but hard. Ciao, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] git svn: work around SVN 1.7 mishandling of svn:special changes
Subversion represents symlinks as ordinary files with content starting
with link and the svn:special property set to *. Thus a file can
switch between being a symlink and a non-symlink simply by toggling
its svn:special property, and new checkouts will automatically write a
file of the appropriate type. Likewise, in subversion 1.6 and older,
running svn update would notice changes in filetype and update the
working copy appropriately.
Starting in subversion 1.7 (issue 4091), changes to the svn:special
property trip an assertion instead:
$ svn up svn-tree
Updating 'svn-tree':
svn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \
line 1583: assertion failed (action == svn_wc_conflict_action_edit \
|| action == svn_wc_conflict_action_delete || action == \
svn_wc_conflict_action_replace)
Revisions prepared with ordinary svn commands (svn add and not svn
propset) don't trip this because they represent these filetype
changes using a replace operation, which is approximately equivalent
to removal followed by adding a new file and works fine. Follow suit.
Noticed using t9100. After this change, git-svn's file-to-symlink
changes are sent in a format that modern svn update can handle and
tests t9100.11-13 pass again.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Eric Wong wrote:
I needed to filter the patch through:
s,git-svn\.perl,perl/Git/SVN/Editor.pm,g
though...
Yeah, good catch. Here's a v3 tested against master. Unlike in v2,
it remembers to pass the $deletions parameter to D() and A() --- which
shouldn't make a difference because we are not adding a directory, but
it's nice to be consistent to make reading smoother.
perl/Git/SVN/Editor.pm | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 755092fd..178920c8 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -345,7 +345,30 @@ sub M {
$self-close_file($fbat,undef,$self-{pool});
}
-sub T { shift-M(@_) }
+sub T {
+ my ($self, $m, $deletions) = @_;
+
+ # Work around subversion issue 4091: toggling the is a
+ # symlink property requires removing and re-adding a
+ # file or else svn up on affected clients trips an
+ # assertion and aborts.
+ if (($m-{mode_b} =~ /^120/ $m-{mode_a} !~ /^120/) ||
+ ($m-{mode_b} !~ /^120/ $m-{mode_a} =~ /^120/)) {
+ $self-D({
+ mode_a = $m-{mode_a}, mode_b = '00',
+ sha1_a = $m-{sha1_a}, sha1_b = '0' x 40,
+ chg = 'D', file_b = $m-{file_b}
+ }, $deletions);
+ $self-A({
+ mode_a = '00', mode_b = $m-{mode_b},
+ sha1_a = '0' x 40, sha1_b = $m-{sha1_b},
+ chg = 'A', file_b = $m-{file_b}
+ }, $deletions);
+ return;
+ }
+
+ $self-M($m, $deletions);
+}
sub change_file_prop {
my ($self, $fbat, $pname, $pval) = @_;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Eric Wong wrote:
Thanks both. Also pushed to master on git://bogomips.org/git-svn.git
(commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)
Jonathan Nieder (2):
git svn: work around SVN 1.7 mishandling of svn:special changes
svn test: escape peg revision separator using empty peg rev
Thanks. Here's the $deletions nit as a patch on top.
-- 8 --
Subject: Git::SVN::Editor::T: pass $deletions to -A and -D
This shouldn't make a difference because the $deletions hash is
only used when adding a directory (see 379862ec, 2012-02-20) but
it's nice to be consistent to make reading smoother anyway. No
functional change intended.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
perl/Git/SVN/Editor.pm |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 3bbc20a0..178920c8 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -358,12 +358,12 @@ sub T {
mode_a = $m-{mode_a}, mode_b = '00',
sha1_a = $m-{sha1_a}, sha1_b = '0' x 40,
chg = 'D', file_b = $m-{file_b}
- });
+ }, $deletions);
$self-A({
mode_a = '00', mode_b = $m-{mode_b},
sha1_a = '0' x 40, sha1_b = $m-{sha1_b},
chg = 'A', file_b = $m-{file_b}
- });
+ }, $deletions);
return;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev
Eric Wong wrote: Jonathan Nieder jrnie...@gmail.com wrote: -- 8 -- Subject: Git::SVN::Editor::T: pass $deletions to -A and -D For future reference, it'd be slightly easier for me to apply if you included the From: (and Date:) headers so I don't have to yank+paste them myself : Ah, I assumed you were using git am --scissors. Will do next time. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files
Hi Jeff, In August, Jeff King wrote: Before reading a config file, we check !access(path, R_OK) to make sure that the file exists and is readable. If it's not, then we silently ignore it. git became noisy: $ git fetch --all warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory Fetching origin warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory Fetching charon warning: unable to access '/home/jrn/.config/git/config': Not a directory [...] On this machine, ~/.config/git has been a regular file for a while, with ~/.gitconfig a symlink to it. Probably ENOTDIR should be ignored just like ENOENT is. Except for the noise, the behavior is fine, but something still feels wrong. When ~/.gitconfig is unreadable (EPERM), the messages are a symptom of an older issue: the config file is being ignored. Shouldn't git error out instead so the permissions can be fixed? E.g., if the sysadmin has set [branch] autoSetupRebase to true in /etc/gitconfig and I have set it to false in my own ~/.gitconfig, I'd rather see git error out because ~/.gitconfig has become unreadable in a chmod gone wrong than have a branch set up with the wrong settings and have to learn to fix it up myself. In other words, how about something like this? Jonathan Nieder (2): config, gitignore: failure to access with ENOTDIR is ok config: treat user and xdg config permission problems as errors config.c | 4 ++-- git-compat-util.h | 6 +- wrapper.c | 10 +- 3 files changed, 16 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] config, gitignore: failure to access with ENOTDIR is ok
The access_or_warn() function is used to check for optional
configuration files like .gitconfig and .gitignore and warn when they
are not accessible due to a configuration issue (e.g., bad
permissions). It is not supposed to complain when a file is simply
missing.
Noticed on a system where ~/.config/git was a file --- when the new
XDG_CONFIG_HOME support looks for ~/.config/git/config it should
ignore ~/.config/git instead of printing irritating warnings:
$ git status -s
warning: unable to access '/home/jrn/.config/git/config': Not a directory
warning: unable to access '/home/jrn/.config/git/config': Not a directory
warning: unable to access '/home/jrn/.config/git/config': Not a directory
warning: unable to access '/home/jrn/.config/git/config': Not a directory
Compare v1.7.12.1~2^2 (attr:failure to open a .gitattributes file
is OK with ENOTDIR, 2012-09-13).
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
git-compat-util.h | 5 -
wrapper.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 2fbf1fd8..f567767f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -635,7 +635,10 @@ int rmdir_or_warn(const char *path);
*/
int remove_or_warn(unsigned int mode, const char *path);
-/* Call access(2), but warn for any error besides ENOENT. */
+/*
+ * Call access(2), but warn for any error except missing file
+ * (ENOENT or ENOTDIR).
+ */
int access_or_warn(const char *path, int mode);
/* Warn on an inaccessible file that ought to be accessible */
diff --git a/wrapper.c b/wrapper.c
index 68739aaa..c1b919f3 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -411,7 +411,7 @@ void warn_on_inaccessible(const char *path)
int access_or_warn(const char *path, int mode)
{
int ret = access(path, mode);
- if (ret errno != ENOENT)
+ if (ret errno != ENOENT errno != ENOTDIR)
warn_on_inaccessible(path);
return ret;
}
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] config: treat user and xdg config permission problems as errors
Git reads multiple configuration files: settings come first from the
system config file (typically /etc/gitconfig), then the xdg config
file (typically ~/.config/git/config), then the user's dotfile
(~/.gitconfig), then the repository configuration (.git/config).
Git has always used access(2) to decide whether to use each file; as
an unfortunate side effect, that means that if one of these files is
unreadable (e.g., EPERM or EIO), git skips it. So if I use
~/.gitconfig to override some settings but make a mistake and give it
the wrong permissions then I am subject to the settings the sysadmin
chose for /etc/gitconfig.
Better to error out and ask the user to correct the problem.
This only affects the user and xdg config files, since the user
presumably has enough access to fix their permissions. If the system
config file is unreadable, the best we can do is to warn about it so
the user knows to notify someone and get on with work in the meantime.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
config.c | 4 ++--
git-compat-util.h | 1 +
wrapper.c | 8
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 08e47e2e..e8875b8a 100644
--- a/config.c
+++ b/config.c
@@ -945,12 +945,12 @@ int git_config_early(config_fn_t fn, void *data, const
char *repo_config)
found += 1;
}
- if (xdg_config !access_or_warn(xdg_config, R_OK)) {
+ if (xdg_config !access_or_die(xdg_config, R_OK)) {
ret += git_config_from_file(fn, xdg_config, data);
found += 1;
}
- if (user_config !access_or_warn(user_config, R_OK)) {
+ if (user_config !access_or_die(user_config, R_OK)) {
ret += git_config_from_file(fn, user_config, data);
found += 1;
}
diff --git a/git-compat-util.h b/git-compat-util.h
index f567767f..dfe2e318 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -640,6 +640,7 @@ int remove_or_warn(unsigned int mode, const char *path);
* (ENOENT or ENOTDIR).
*/
int access_or_warn(const char *path, int mode);
+int access_or_die(const char *path, int mode);
/* Warn on an inaccessible file that ought to be accessible */
void warn_on_inaccessible(const char *path);
diff --git a/wrapper.c b/wrapper.c
index c1b919f3..7cbe96a0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -416,6 +416,14 @@ int access_or_warn(const char *path, int mode)
return ret;
}
+int access_or_die(const char *path, int mode)
+{
+ int ret = access(path, mode);
+ if (ret errno != ENOENT errno != ENOTDIR)
+ die_errno(_(unable to access '%s'), path);
+ return ret;
+}
+
struct passwd *xgetpwuid_self(void)
{
struct passwd *pw;
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors
Jeff King wrote: For example, servers may depend on /etc/gitconfig to enforce security policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to work around a broken machine. Very good point. How about these patches on top? Jonathan Nieder (2): config doc: advertise GIT_CONFIG_NOSYSTEM config: exit on error accessing any config file Documentation/git-config.txt | 8 config.c | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/2] config doc: advertise GIT_CONFIG_NOSYSTEM
When a syntax error or other problem renders /etc/gitconfig buggy on a multiuser system where mortals do not have write access to /etc, the GIT_CONFIG_NOSYSTEM variable is the best tool we have to keep getting work done until the sysadmin sorts the problem out. Noticed while experimenting with teaching git to error out when /etc/gitconfig is unreadable. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Documentation/git-config.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index eaea0791..907a1fd5 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -240,6 +240,14 @@ GIT_CONFIG:: Using the --global option forces this to ~/.gitconfig. Using the --system option forces this to $(prefix)/etc/gitconfig. +GIT_CONFIG_NOSYSTEM:: + Whether to skip reading settings from the system-wide + $(prefix)/etc/gitconfig file. This environment variable can + be used along with HOME and XDG_CONFIG_HOME to create a + predictable environment for a picky script, or you can set it + temporarily to avoid using a buggy /etc/gitconfig file while + waiting for someone with sufficient permissions to fix it. + See also FILES. -- 1.8.0.rc2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/2] config: exit on error accessing any config file
There is convenience in warning and moving on when somebody has a
bogus permissions on /etc/gitconfig and cannot do anything about it.
But the cost in predictability and security is too high --- when
unreadable config files are skipped, it means an I/O error or
permissions problem causes important configuration to be bypassed.
For example, servers may depend on /etc/gitconfig to enforce security
policy (setting transfer.fsckObjects or receive.deny*). Best to
always error out when encountering trouble accessing a config file.
This may add inconvenience in some cases:
1. You are inspecting somebody else's repo, and you do not have
access to their .git/config file. Git typically dies in this
case already since we cannot read core.repositoryFormatVersion,
so the change should not be too noticeable.
2. You have used sudo -u or a similar tool to switch uid, and your
environment still points Git at your original user's global
config, which is not readable. In this case people really would
be inconvenienced (they would rather see the harmless warning and
continue the operation) but they can work around it by setting
HOME appropriately after switching uids.
3. You do not have access to /etc/gitconfig due to a broken setup.
In this case, erroring out is a good way to put pressure on the
sysadmin to fix the setup. While they wait for a reply, users
can set GIT_CONFIG_NOSYSTEM to true to keep Git working without
complaint.
After this patch, errors accessing the repository-local and systemwide
config files and files requested in include directives cause Git to
exit, just like errors accessing ~/.gitconfig.
Explained-by: Jeff King p...@peff.net
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
config.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index e8875b8a..a4d153f6 100644
--- a/config.c
+++ b/config.c
@@ -60,7 +60,7 @@ static int handle_path_include(const char *path, struct
config_include_data *inc
path = buf.buf;
}
- if (!access_or_warn(path, R_OK)) {
+ if (!access_or_die(path, R_OK)) {
if (++inc-depth MAX_INCLUDE_DEPTH)
die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
cf cf-name ? cf-name : the command line);
@@ -939,7 +939,7 @@ int git_config_early(config_fn_t fn, void *data, const char
*repo_config)
home_config_paths(user_config, xdg_config, config);
- if (git_config_system() !access_or_warn(git_etc_gitconfig(), R_OK)) {
+ if (git_config_system() !access_or_die(git_etc_gitconfig(), R_OK)) {
ret += git_config_from_file(fn, git_etc_gitconfig(),
data);
found += 1;
@@ -955,7 +955,7 @@ int git_config_early(config_fn_t fn, void *data, const char
*repo_config)
found += 1;
}
- if (repo_config !access_or_warn(repo_config, R_OK)) {
+ if (repo_config !access_or_die(repo_config, R_OK)) {
ret += git_config_from_file(fn, repo_config, data);
found += 1;
}
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/2] doc: advertise GIT_CONFIG_NOSYSTEM
On a multiuser system where mortals do not have write access to /etc, the GIT_CONFIG_NOSYSTEM variable is the best tool we have to keep getting work done when a syntax error or other problem renders /etc/gitconfig buggy, until the sysadmin sorts the problem out. Noticed while experimenting with teaching git to error out when /etc/gitconfig is unreadable. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jonathan Nieder wrote: --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -240,6 +240,14 @@ GIT_CONFIG:: Using the --global option forces this to ~/.gitconfig. Using the --system option forces this to $(prefix)/etc/gitconfig. +GIT_CONFIG_NOSYSTEM:: Hm, unlike GIT_CONFIG this applies to all git commands (not just git config), so it is misleading to document them in the same place. Here's a better patch. Documentation/git-config.txt | 4 Documentation/git.txt| 8 2 files changed, 12 insertions(+) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index eaea0791..9ae2508f 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -240,6 +240,10 @@ GIT_CONFIG:: Using the --global option forces this to ~/.gitconfig. Using the --system option forces this to $(prefix)/etc/gitconfig. +GIT_CONFIG_NOSYSTEM:: + Whether to skip reading settings from the system-wide + $(prefix)/etc/gitconfig file. See linkgit:git[1] for details. + See also FILES. diff --git a/Documentation/git.txt b/Documentation/git.txt index d1d227a3..ae1f833a 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -757,6 +757,14 @@ for further details. and read the password from its STDOUT. See also the 'core.askpass' option in linkgit:git-config[1]. +'GIT_CONFIG_NOSYSTEM':: + Whether to skip reading settings from the system-wide + `$(prefix)/etc/gitconfig` file. This environment variable can + be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a + predictable environment for a picky script, or you can set it + temporarily to avoid using a buggy `/etc/gitconfig` file while + waiting for someone with sufficient permissions to fix it. + 'GIT_FLUSH':: If this environment variable is set to 1, then commands such as 'git blame' (in incremental mode), 'git rev-list', 'git log', -- 1.8.0.rc2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files
Junio C Hamano wrote:
If the config side can
be switched to unconditionally attempt to fopen and then deal with
an error when it happens, we can get rid of access_or_{warn,die}
and replace them with fopen_or_{warn,die} and use them from the two
places (attr.c:read_attr_from_file() and the configuration stuff).
I haven't looked to see if that a too intrusive refactoring to be
worth it, though.
That sounds reasonable, but I'm punting on it. The first step would
be tweaking the git_config_from_file() calling convention to convey
missing file errors specially, perhaps by making sure errno is
meaningful when the return value is -1, and that already sounds like
work. ;-)
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/2] Re: [PATCH 2/7] Change canonicalize_url() to use the SVN 1.7 API when available.
Hi Eric,
Michael G Schwern wrote:
On 2012.7.28 6:50 AM, Jonathan Nieder wrote:
Michael G Schwern wrote:
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
[...]
@@ -100,6 +102,20 @@ API as a URL.
=cut
sub canonicalize_url {
+ my $url = shift;
+
+ # The 1.7 way to do it
+ if ( defined SVN::_Core::svn_uri_canonicalize ) {
+ return SVN::_Core::svn_uri_canonicalize($url);
+ }
+ # There wasn't a 1.6 way to do it, so we do it ourself.
+ else {
+ return _canonicalize_url_ourselves($url);
[...]
Leaves me a bit nervous.
As it should, SVN dumped a mess on us.
Here's a pair of patches that address some of the bugs I was alluding
to. Patch 1 makes _canonicalize_url_ourselves() match Subversion's
own canonicalization behavior more closely, though even with that
patch it still does not meet Subversion's requirements perfectly
(e.g., %ab is not canonicalized to %AB). Patch 2 makes that not
matter by using svn_path_canonicalize() when possible, which is the
standard way to do this kind of thing.
Sorry for the lack of clarity before.
Jonathan Nieder (2):
git svn: do not overescape URLs (fallback case)
Git::SVN::Utils::canonicalize_url: use svn_path_canonicalize when
available
perl/Git/SVN/Utils.pm | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git svn: do not overescape URLs (fallback case)
Subversion's canonical URLs are intended to make URL comparison easy
and therefore have strict rules about what characters are special
enough to urlencode and what characters should be left alone.
When in the fallback codepath because unable to use libsvn's own
canonicalization function for some reason, escape special characters
in URIs according to the svn_uri__char_validity[] table in
subversion/libsvn_subr/path.c (r935829). The libsvn versions that
trigger this code path are not likely to be strict enough to care, but
it's nicer to be consistent.
Noticed by using SVN 1.6.17 perl bindings, which do not provide
SVN::_Core::svn_uri_canonicalize (triggering the fallback code),
with libsvn 1.7.5, whose do_switch is fussy enough to care:
Committing to file:///home/jrn/src/git/t/trash%20directory.\
t9118-git-svn-funky-branch-names/svnrepo/pr%20ject/branches\
/more%20fun%20plugin%21 ...
svn: E235000: In file '[...]/subversion/libsvn_subr/dirent_uri.c' \
line 2291: assertion failed (svn_uri_is_canonical(url, pool))
error: git-svn died of signal 6
not ok - 3 test dcommit to funky branch
After this change, the '!' in 'more%20fun%20plugin!' is not urlencoded
and t9118 passes again.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
perl/Git/SVN/Utils.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 8b8cf375..3d1a0933 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -155,7 +155,7 @@ sub _canonicalize_url_path {
my @parts;
foreach my $part (split m{/+}, $uri_path) {
- $part =~
s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(%%%02X,ord($1))/eg;
+ $part =~
s/([^!\$%'()*+,.\/\w:=\@_`~-]|%(?![a-fA-F0-9]{2}))/sprintf(%%%02X,ord($1))/eg;
push @parts, $part;
}
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] git svn: canonicalize_url(): use svn_path_canonicalize when available
Until Subversion 1.7 (more precisely r873487), the standard way to
canonicalize a URI was to call svn_path_canonicalize(). Use it.
This saves git svn from having to rely on our imperfect
reimplementation of the same. If the function doesn't exist or
returns undef, though, it can use the fallback code, which we keep to
be conservative. Since svn_path_canonicalize() was added before
Subversion 1.1, hopefully that doesn't happen often.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
perl/Git/SVN/Utils.pm | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 3d1a0933..40f7c799 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -138,15 +138,19 @@ API as a URL.
sub canonicalize_url {
my $url = shift;
+ my $rv;
# The 1.7 way to do it
if ( defined SVN::_Core::svn_uri_canonicalize ) {
- return SVN::_Core::svn_uri_canonicalize($url);
+ $rv = SVN::_Core::svn_uri_canonicalize($url);
}
- # There wasn't a 1.6 way to do it, so we do it ourself.
- else {
- return _canonicalize_url_ourselves($url);
+ # The 1.6 way to do it
+ elsif ( defined SVN::_Core::svn_path_canonicalize ) {
+ $rv = SVN::_Core::svn_path_canonicalize($url);
}
+ return $rv if defined $rv;
+
+ return _canonicalize_url_ourselves($url);
}
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/hooks: avoid requiring root access in usage instructions
Junio C Hamano wrote: We already encourage casting-in-stone a particular version of the sample hook when a new repository is created by copying them from the template directory. This prevents from surprising users when an updated version of Git changes the behaviour of these samples. Even if you think bugs in older ones may be corrected in newer ones, silently updating the hook the user chose to use by inspecting one particular version is not something we would want to do lightly. For context, the sample hooks you are talking about are the *.sample files from the templates/ directory. Except for post-update.sample, most are not very useful out of the box, and they are very much intended as examples to start people's thinking, as opposed to something one-size-fits-all. By contrast, the post-receive-email script from contrib is a complete program with a well-defined behavior and configuration that have stayed consistent while the details of its implementation improved. It can be used by symlinking into place, but maybe a better set of instructions would say # This script takes no arguments and expects the same input format # as git's post-receive hook, so if this script is at # /usr/share/git-core/contrib/hooks/post-receive-email (as it is # on Debian and Fedora), you can do # # cd /path/to/your/repository.git # echo '#!/bin/sh' hooks/post-receive # echo 'exec /usr/share/git-core/contrib/hooks/post-receive-email' \ # hooks/post-receive # chmod +x hooks/post-receive That would make it more obvious what I can do next if my post-receive input should be passed both to post-receive-email and some other tool (perhaps in a pipeline using tee). Hmm? Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Hi Felipe, Felipe Contreras wrote: This test is completely wrong. 1) Where are the marks file? 2) master..master shouldn't export anything Why shouldn't master..master export anything? It means update the master ref; we already have all commits up to and including master^0. The underlying problem is that fast-export takes rev-list arguments as parameters, which is unfortunately only an approximation to what is really intended. Ideally it would separately take a list of refs to import and rev-list arguments representing the commits we already have. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Felipe Contreras wrote: Does it mean that? I don't think so, but let's assume that's the case. We don't have all those commits; without the marks we have nothing. Or what exactly do you mean by 'we'? Not everyone uses marks. Ciao, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Felipe Contreras wrote: Again, if you don't have marks, I don't see what you expect to be exported with 'master..master', even with marks, I don't see what you expect. And that's fine. Unless you were trying to do some work and this lack of understanding got in the way. In that case, with a calmer and more humble approach you might find people willing to help you. Maybe they will learn something from you, too. Ciao, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Felipe Contreras wrote: I don't need help, I am helping you, I was asked to take a look at this patch series. If you don't want my help, then by all means, keep this series rotting, it has being doing so for the past year without anybody complaining. Ah, so _that_ (namely getting Sverre's remote helper to work) is the work you were trying to do. Thanks for explaining. If I understand correctly, it is possible to get Sverre's remote helper to work without affecting this particular testcase. From that point of view I think you were on the right track. The testcase is imho correct and does not need changing. So yes, I don't want your help changing it. I don't suspect you will be using git fast-export $(git rev-parse master)..master. It is safe and good to add additional testcases documenting the syntax that you do use, as an independent topic. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Felipe Contreras wrote: All right, so I run this and get this: % git fast-export master..master reset refs/heads/master from 8c7a786b6c8eae8eac91083cdc9a6e337bc133b0 As an user of fast-export, what do I do with that now? You passed master.. on the command line, indicating that your repository already has commit 8c7a786b6c8eae8eac91083cdc9a6e337bc133b0. Now you can update the master branch to point to that commit, as the fast-export output indicates. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Felipe Contreras wrote: Show me a single remote helper that manually stores SHA-1's and I might believe you, but I doubt that, marks are too convenient. Oh dear lord. Why are you arguing? Explain how coming to a consensus on this will help accomplish something useful, and then I can explain my point of view. In the meantime, this seems like a waste of time. Let's agree to disagree. Regards, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Sverre Rabbelier wrote:
That's weird, we have this bit:
+ if (elem-whence != REV_CMD_REV elem-whence !=
REV_CMD_RIGHT)
+ continue;
If I understand correctly that should cause it to only output revs
(e.g. 'foo1') and the rhs side of a have..want spec.
If I remember right, '^foo1' is (whence == REV_CMD_REV) with (flags ==
UNINTERESTING). That's why sequencer.c checks for unadorned revs like
this:
if (opts-revs-cmdline.nr == 1
opts-revs-cmdline.rev-whence == REV_CMD_REV
opts-revs-no_walk
!opts-revs-cmdline.rev-flags) {
Maybe
if (elem-flags UNINTERESTING)
continue;
if (elem-whence == REV_CMD_PARENTS_ONLY) /* foo^@ */
continue;
would work well here? That would handle bizarre cases like --not
next..master (and ordinary cases like master...next) better, by
focusing on the semantics instead of syntax.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] __git_ps1: migrate out of contrib/completion
Different installers put the git-prompt.sh shell library at different
places on the installed system, so there is no shared location users
can count on:
Fedora - /etc/profile.d/git-prompt.sh
Gentoo - /usr/share/bash-completion/git-prompt
Arch - /usr/share/git/git-prompt.sh
The __git_ps1 helper doesn't have anything to do with bash completion
in principle, but because it was written in the context of that
project, its sources are kept in contrib/completion/git-prompt.sh.
Let's make it a first-class shell library in the toplevel and install
it to $(gitexecdir) alongside git-sh-setup and git-sh-i18n, where it
can be easily found.
Keep a symlink in contrib/completion/ to avoid breaking setups where
this library is used directly from the source tree.
Now you can put the following in your ~/.bashrc:
if test ${BASH+set} test ${PS1+set} # interactive!
then
gitexecdir=$(git --exec-path)
if test -r $gitexecdir/git-sh-prompt)
then
. $gitexecdir/git-sh-prompt
fi
if type -t __git_ps1 /dev/null
then
PS1='\w$(__git_ps1)\$ '
fi
fi
and the shell prompt will show the current branch name in git
repositories when on a machine with a new enough version of git.
Reported-by: Danny Yates mail4da...@googlemail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Danny Yates wrote:
Would that not give the impression of git sh-prompt being a core
command? If so, that would be poor, IMHO.
Not any more than $(git --exec-path)/git-sh-setup and git-sh-i18n. :)
When I was investigating this last night, I expected to find it
(git-prompt.sh) in contrib, although that doesn't make an enormous
amount of sense. Ideally, the full path to wherever it's installed
should be mentioned in the bash completion file (which is where I
went to look when __git_ps1 stopped working),
Yes, certainly.
but that would mean
modifying a file from upstream and I'm not sure if that's easy/the
done thing.
We're talking on the upstream list now, so that's not an issue.
Thanks,
Jonathan
Documentation/git-sh-prompt.txt| 79 ++
Makefile | 1 +
contrib/completion/git-completion.bash | 2 +-
contrib/completion/git-prompt.sh | 291 +
.../completion/git-prompt.sh = git-sh-prompt.sh | 0
5 files changed, 82 insertions(+), 291 deletions(-)
create mode 100644 Documentation/git-sh-prompt.txt
rewrite contrib/completion/git-prompt.sh (100%)
mode change 100644 = 12
rename contrib/completion/git-prompt.sh = git-sh-prompt.sh (100%)
diff --git a/Documentation/git-sh-prompt.txt b/Documentation/git-sh-prompt.txt
new file mode 100644
index ..2c705fef
--- /dev/null
+++ b/Documentation/git-sh-prompt.txt
@@ -0,0 +1,79 @@
+git-sh-prompt(1)
+
+
+NAME
+
+git-sh-prompt - Functions to describe repository in bash or zsh prompt
+
+SYNOPSIS
+
+[verse]
+'. $(git --exec-path)/git-sh-prompt'
+
+DESCRIPTION
+---
+This script allows you to see the current branch in your bash prompt.
+
+To enable:
+
+1. Add the following line to your .bashrc and .zshrc:
+
+ . $(git --exec-path)/git-sh-prompt
+
+2. Change your PS1 to also show the current branch:
+
+ Bash: PS1='[\u@\h \W$(__git_ps1 (%s))]\$ '
+ Zsh: PS1='[%n@%m %c$(__git_ps1 (%s))]\$ '
+
+The argument to __git_ps1 will only be displayed if you are currently
+in a git repository. The %s token is replaced by the name of the current
+branch.
+
+In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty value,
+unstaged (`*`) and staged (`+`) changes are indicated next to the branch
+name. You can configure this per repository with the `bash.showDirtyState`
+variable, which defaults to `true` once GIT_PS1_SHOWDIRTYSTATE is enabled.
+
+You can also see if something is currently stashed, by setting
+GIT_PS1_SHOWSTASHSTATE to a nonempty value. If something is stashed,
+then a '$' is shown next to the branch name.
+
+If you would like to see if there are untracked files, set
+GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there are untracked
+files, a '%' is shown next to the branch name.
+
+If you would like to see the difference between HEAD and its upstream,
+set GIT_PS1_SHOWUPSTREAM=auto. A '' indicates the current branch is
+behind, '' indicates it is ahead, '' indicates it has diverged, and
+'=' indicates no difference. You can further control behavior by
+setting GIT_PS1_SHOWUPSTREAM to a space-separated list of values:
+
+verbose::
+ show number of commits ahead of/behind (+/-) upstream
+legacy::
+ don't use the '--count' option available in recent versions
+ of git-rev-list
+git::
+ always compare HEAD to `@{upstream}`
+svn::
+ always compare HEAD
Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
Sverre Rabbelier wrote: I know there was a reason why using UNINTERESTING didn't work (otherwise we could've used that to start with, instead of needing Junio's whence solution). I think all refs ended up being marked as UNINTERESTING or somesuch. True. Is it be possible to check UNINTERESTING in revs-cmdline before the walk? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Where should git-prompt.sh be installed?
SZEDER Gábor wrote: On Wed, Oct 24, 2012 at 05:51:06PM -0700, Jonathan Nieder wrote: Proposal: 1) /usr/lib/git-core/git-sh-prompt 2) git-sh-prompt(1) Not sure about the sh part. The prompt function is very Bash-specific, it won't work under a plain POSIX shell. That's an interesting point. Here's my logic: The prompt function was originally Bash-specific, but over time it gained support for Zsh as well. If we're lucky, some day it might gain support for mksh. Meanwhile something simple like git-prompt would sound too much like a normal git command, I fear. Shell language extensions used: * future standard: local variables * cosmetic: [[ and (( syntax in place of test and $( * cosmetic: for ((n=1; n = n_stop; n++)) instead of a more explicit while loop * cosmetic: var+=value syntax * optimization: syntax to iterate over lines in a variable instead of, for example, sed + eval * arrays: svn_remote, svn_upstream --- using plain variables would require some escaping. So nothing fundamental, but since this is only useful in shells like Bash that support command substitution in $PS1, it seems reasonable to keep using the extensions until someone wants to use it with a more limited shell. In other words, the interface is still this is a scriptlet you might source in your Bourne-style shell to get a __git_ps1 function to use in your prompt. The implementation language just happens not to be POSIX shell today. Perhaps it should return early in shells other than bash and zsh to help people keep their .shrc simple. What do you think? Do other VCSes have similar prompt scripts? Where do they install theirs? Mercurial has an hg-prompt extension that can be used by running hg prompt. http://mercurial.selenic.com/wiki/PromptExtension I haven't found one for Subversion. Bazaar has contrib/bash/bzrbashprompt.sh. It doesn't get installed. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/2] doc: command line interface (cli) dot-repository dwimmery
Hi, Philip Oakley wrote: The Git cli will generally accept dot '.' (period) as equivalent to the current repository when appropriate. Tell the reader of this 'do what I mean' (dwim)mery action. [...] --- a/Documentation/gitcli.txt +++ b/Documentation/gitcli.txt @@ -59,6 +59,10 @@ working tree. After running `git add hello.c; rm hello.c`, you will _not_ see `hello.c` in your working tree with the former, but with the latter you will. +Just as, by convention, the filesystem '.' refers to the current directory, +using a '.' (period) as a repository name in Git (a dot-repository) refers +to your local repository. Good idea, but I fear that no one would find it there. Would it make sense to put this in Documentation/urls.txt (aka the GIT URLS section of git-fetch(1) and git-clone(1)), where other URL schemes are documented? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] config doc: add dot-repository note
Philip Oakley wrote: --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -734,6 +734,8 @@ branch.name.remote:: overridden by `branch.name.pushremote`. If no remote is configured, or if you are not on any branch, it defaults to `origin` for fetching and `remote.pushdefault` for pushing. + Additionally, a `.` (period) means the current local repository + (a dot-repository), see `branch.name.merge`'s final note below. Does this accept an arbitrary git URL, or only remote names? The current documentation makes it sound like the latter, which makes this exception seem very weird. Is it actually the former? I think a cross-reference to the git urls or git remotes documentation would be a better way to go, instead of having to repeat the rules of URL syntax everywhere they are used. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] patch-ids: check modified paths before calculating diff
John Keeping wrote:
In this case, it is likely that only a small number of paths are touched
by the commits on the smaller side of the range and by storing these we
can ignore many commits on the other side before unpacking blobs and
diffing them.
I like this idea a lot.
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -1,5 +1,6 @@
#include cache.h
#include diff.h
+#include diffcore.h
#include commit.h
#include sha1-lookup.h
#include patch-ids.h
@@ -16,6 +17,137 @@ static int commit_patch_id(struct commit *commit, struct
diff_options *options,
return diff_flush_patch_id(options, sha1);
}
+struct collect_paths_info {
+ struct string_list *paths;
+ int searching;
+};
+
+static int collect_paths_recursive(int n, struct tree_desc *t,
+ const char *base, struct pathspec *pathspec,
+ struct collect_paths_info *data);
Can you say a little about how this function works (e.g., in a
comment)? What are the preconditions and postconditions? How does
the state evolve (e.g, when is searching set)?
+
+static int same_entry(struct name_entry *a, struct name_entry *b)
+{
+ if (!a-sha1 || !b-sha1)
+ return a-sha1 == b-sha1;
+ return !hashcmp(a-sha1, b-sha1)
+ a-mode == b-mode;
Style: unusual whitespace (the tab after return). I'd just do
if (!a-sha1 || ...)
return ...
return !hashcmp(a-sha1, b-sha1) a-mode == b-mode;
since it is not too long.
[...]
+static char *traverse_path(const struct traverse_info *info,
+ const struct name_entry *n)
+{
+ char *path = xmalloc(traverse_path_len(info, n) + 1);
+ return make_traverse_path(path, info, n);
+}
This function seems to be the same as one of the same name in
builtin/merge-tree.c. Should it be put somewhere more public, for
example as part of the tree-walk API? Who is responsible for freeing
path? Would it make sense to use a strbuf here?
strbuf_setlen(buf, traverse_path_len(info, n));
make_traverse_path(buf.buf, info, n);
+
+static int add_touched_path(struct collect_paths_info *info, const char
*path)
+{
+ if (info-searching) {
+ if (!string_list_has_string(info-paths, path))
+ return -1;
+ }
+ string_list_insert(info-paths, path);
+ return 0;
+}
Same question about internal API: when I see
add_touched_path(info, path)
what should I expect it to do?
In the info-searching case, string_list_insert(info-paths, path)
will always be a no-op, right? What does it mean that this is adding
a touched path in that case?
[...]
+static int collect_touched_paths_cb(int n, unsigned long mask,
+ unsigned long dirmask, struct name_entry *entry,
+ struct traverse_info *info)
+{
Same question about contracts. Ideally a reader in a rush should be
able to read an opening comment about what the function does and
then return to the caller without delving into the details of how
it does its work.
+ struct collect_paths_info *collect_info = info-data;
+ if (n == 1) {
+ /* We're handling a root commit - add all the paths. */
[...]
+ if ((entry[0].sha1 S_ISDIR(entry[0].mode)) ||
+ (entry[1].sha1 S_ISDIR(entry[1].mode))) {
At this point I'm not sure what two trees are being traversed in
parallel, so it's not obvious to me what the code's about. Are
they the before and after trees for commits being rebased?
[...]
+static int collect_touched_paths(struct commit *commit, struct patch_ids
*ids,
+ int searching)
+{
+ struct tree_desc trees[2];
+ struct collect_paths_info info = { ids-touched_paths, searching };
+ void *commitbuf;
+ int result;
+
+ commitbuf = fill_tree_descriptor(trees + 1, commit-object.sha1);
+ if (commit-parents) {
+ void *parentbuf = fill_tree_descriptor(trees + 0,
+ commit-parents-item-object.sha1);
Looks like yes.
What should happen for commits with more than 1 parent? If they're
supposed to not enter this codepath because of a previous check, a
die(BUG: ...) could be a good way to make reading easier.
[...]
@@ -40,6 +172,7 @@ int init_patch_ids(struct patch_ids *ids)
diff_setup(ids-diffopts);
DIFF_OPT_SET(ids-diffopts, RECURSIVE);
diff_setup_done(ids-diffopts);
+ ids-touched_paths.strdup_strings = 1;
Good.
[...]
@@ -64,6 +199,13 @@ static struct patch_id *add_commit(struct commit *commit,
unsigned char sha1[20];
int pos;
+ if (no_add) {
+ if (collect_touched_paths(commit, ids, 1) 0)
+ return NULL;
Ah, so this is what the searching does.
The existing no_add argument is very confusing (what does it mean to
add a commit without adding?), but at least the confusion is
self-contained in a small, simple set of functions:
static struct patch_id *add_commit(struct commit
Re: git-diff-index man page
Junio C Hamano wrote: --- a/Documentation/git-diff-index.txt +++ b/Documentation/git-diff-index.txt @@ -3,7 +3,7 @@ git-diff-index(1) NAME -git-diff-index - Compares content and mode of blobs between the index and repository +git-diff-index - Compare a tree and the working tree or the index Looks good. I think it scans better with the second the left out: git diff-index - Compare a tree to the working tree or index [...] @@ -13,11 +13,12 @@ SYNOPSIS DESCRIPTION --- -Compares the content and mode of the blobs found via a tree +Compare the content and mode of the blobs found in a tree object I think this s/Compares/Compare/ is a regression: traditionally the DESCRIPTION sections of manpages describe what a command does in the present indicative (When paths are specified, compares only those named paths), freeing up the imperative for cases where it is useful to instruct the user about what to do. The s/via/in/ looks fine. -object with the content of the current index and, optionally +with the corresponding tracked file in the working tree, or with the Since they are being compared to blobs, file should be files here. -ignoring the stat state of the file on disk. When paths are -specified, compares only those named paths. Otherwise all -entries in the index are compared. +corresponding paths in the index. When paths are specified, +compares only those named paths. Otherwise all entries in the index +are compared. The path parameters to git diff-index are pathspecs and not literal paths. This should say somewhere that this only looks at paths in the index and that untracked files are ignored. Perhaps something as simple as s/entries in the index/tracked files/ would do the trick. Thanks and hope that helps, Jonathan diff --git i/Documentation/git-diff-index.txt w/Documentation/git-diff-index.txt index 58308e15..a86cf62e 100644 --- i/Documentation/git-diff-index.txt +++ w/Documentation/git-diff-index.txt @@ -3,7 +3,7 @@ git-diff-index(1) NAME -git-diff-index - Compare a tree and the working tree or the index +git-diff-index - Compare a tree to the working tree or index SYNOPSIS @@ -13,12 +13,11 @@ SYNOPSIS DESCRIPTION --- - -Compare the content and mode of the blobs found in a tree object -with the corresponding tracked file in the working tree, or with the -corresponding paths in the index. When paths are specified, -compares only those named paths. Otherwise all entries in the index -are compared. +Compares the content and mode of the blobs found in a tree object +with the corresponding tracked files in the working tree, or with the +corresponding paths in the index. When path arguments are present, +compares only paths matching those patterns. Otherwise all tracked +files are compared. OPTIONS --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Outdated and broken online versions of user-manual.html
Philip Oakley wrote: From: Thomas Ackermann th.ac...@arcor.de (5) Large overlapping with the tutorials. IMHO all of the tutorials should be blended into user-manual [...] I would be a little cautious of your point 5 if it squoze everything into one overlong document at the expense of losing the shorter documents - one can't eat a whole melon in one bite ;-) Yes. Once there was a lovely file at Documentation/howto/isolate-bugs-with-bisect.txt explaining how to use git bisect to find which commit caused a kernel regression. That it was a small independent file that didn't assume the reader had read much before was very helpful, since it meant people could easily point novices to that page and say It's easy! when asking them to track down a bug. Nowadays that content is gracefully included in the user-manual under the heading [[using-bisect]]. But I never point people to it any more; I just write out the steps by hand, to avoid intimidating them. Ideally this content would be in an EXAMPLE or TUTORIAL section of the git-bisect(1) manpage for easier reference. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/3] Begin replacing OpenSSL with CommonCrypto
Torsten Bögershausen wrote: One minor nit, or 2: imap-send.c: In function ‘cram’: imap-send.c:913: warning: statement with no effect This fixes it: diff --git a/imap-send.c b/imap-send.c index 8ea180f..11577c9 100644 --- a/imap-send.c +++ b/imap-send.c @@ -35,7 +35,7 @@ typedef void *SSL; #define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len) #define HMAC_Update CCHmacUpdate #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash) -#define HMAC_CTX_cleanup +#define HMAC_CTX_cleanup(c) #define EVP_md5() kCCHmacAlgMD5 #else #include openssl/evp.h Good catch. Thanks. (And I think there are more minor nits: #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash) could be written as #define HMAC_Final(hmac, hash, ptr) CCHmacFinal((hmac), (hash)) (Use paranthese around each parameter) Similar change for HMAC_Init() Not needed --- the comma operator has lower precedence than any other, and any expression containing commas would have to already be surrounded by parentheses to be an argument to this function-like macro. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Reading commit objects
Chico Sokol wrote: We're trying to build a improved java library focused in our needs (jgit has a really confusing api focused in solving egit needs). JGit is also open to contributions, including contributions that add less confusing API calls. :) See http://wiki.eclipse.org/JGit/User_Guide http://wiki.eclipse.org/EGit/Contributor_Guide#JGit http://wiki.eclipse.org/EGit/Contributor_Guide#Using_Gerrit_at_https:.2F.2Fgit.eclipse.org.2Fr https://dev.eclipse.org/mailman/listinfo/jgit-dev Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git clone depth of 0 not possible.
Matthijs Kooijman wrote: In other words: we won't break existing clients if we suddenly send back one less commit than before, since the client just sends over what it wants and then assumes that whatever it gets back is really what it wanted? Yes, depending on your definition of break. An advantage of that approach is that old clients would get the new, intuitive behavior without upgrading. A disadvantage is that it is a confusing world where the same command produces different effects when contacting different servers. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] test: improve rebase -q test
Junio C Hamano wrote:
A more preferrable alternative may be adding something like this to
test-lib.sh and call it from here and elsewhere (there are about 50
places that do test ! -s filename), perhaps?
test_must_be_an_empty_file () {
if test -s $1
then
cat $1
false
fi
}
I generally just use the two-liner
empty
test_cmp empty output
directly in cases like this.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] Highlevel (but simple to implement) commands provided by default for git
Ramkumar Ramachandra wrote: Bráulio Bhavamitra wrote: Agree, these aliased should work as a fallback or as an automatic short version Making builtins override'able is also a terrible idea. It opens doors to potential bugs we don't want to deal with. Simple example: am = log -1 log = am -3 That's detectable and could be made to error out, so it's not too bad. A bigger problem (in my opinion) with allowing arbitrary changes to the meaning of existing commands is that scripts, whether placed in .sh files or given as commands to run over IRC, stop working altogether. It's nice to have commands like git log and git am mean the same thing no matter what machine I am on. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] Highlevel (but simple to implement) commands provided by default for git
Felipe Contreras wrote: On Wed, May 29, 2013 at 6:43 PM, Jonathan Nieder jrnie...@gmail.com wrote: Ramkumar Ramachandra wrote: Making builtins override'able is also a terrible idea. It opens doors to potential bugs we don't want to deal with. Simple example: am = log -1 log = am -3 That's detectable and could be made to error out, so it's not too bad. A bigger problem (in my opinion) with allowing arbitrary changes to the meaning of existing commands is that scripts, whether placed in .sh files or given as commands to run over IRC, stop working altogether. It's nice to have commands like git log and git am mean the same thing no matter what machine I am on. Except that's not true: It's not true that my opinion is that a bigger problem than the non-problem Ram mentioned with allowing arbitrary changes to the meaning of existing commands is that scripts stop working reliably? This combative style of communication is toxic. It kills the chance of a calm, pleasant discussion, even with patient people who don't even fundamentally disagree. Please stop it. Perhaps you meant Commands like 'git log' and 'git am' actually don't mean the same thing on all machines. The default format of 'git log' is configurable. But that is neither here nor there, which would have been a pleasant (if irrelevant) response instead of an obnoxious one. Regards, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] Highlevel (but simple to implement) commands provided by default for git
Felipe Contreras wrote: On Thu, May 30, 2013 at 12:23 AM, Jonathan Nieder jrnie...@gmail.com wrote: Felipe Contreras wrote: On Wed, May 29, 2013 at 6:43 PM, Jonathan Nieder jrnie...@gmail.com wrote: A bigger problem (in my opinion) with allowing arbitrary changes to the meaning of existing commands is that scripts, whether placed in .sh files or given as commands to run over IRC, stop working altogether. It's nice to have commands like git log and git am mean the same thing no matter what machine I am on. Except that's not true: It's not true that my opinion is that a bigger problem than the non-problem Ram mentioned with allowing arbitrary changes to the meaning of existing commands is that scripts stop working reliably? It's not true what you said: commands like git log and git am mean the same thing no matter what machine I am on. It's not true that it's nice when they do? This combative style of communication is toxic. It kills the chance of a calm, pleasant discussion, even with patient people who don't even fundamentally disagree. Please stop it. Stop assuming bad faith[1]. Perhaps you mean I'm trying, but I'm human --- please bear with me while I work on improving. Know that my intentions are good. Unfortunately, good intentions are not enough. Communicating in a way that clearly conveys what you mean instead of something else that derails the conversation is a real skill and, as I said, it's an important one that is basic to being able to have a productive conversation. If you are working on it and are not there yet, my best advice would be to lurk more and speak less, to learn from the example of others, and to start by working on how to receive criticism and to clear up misunderstandings, which can at least mitigate the damage when things go wrong. You have accused others of assuming bad faith in the past, which only escalates things. It's not a good way to move forward. It's possible that the best way to move forward involves some way (e.g., mail client configuration that holds messages in drafts for a little while before sending them out) of being able to cool down when discussions get hot. Sincerely, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-daemon: needs /root/.config/git/config?
Johannes Sixt wrote: Am 04.06.2013 18:08, schrieb Jeff King: However, since changing user id and leaving $HOME is so common, there is a patch under consideration to loosen the check only for the case of EACCES on files in $HOME. That commit is 4698c8f (config: allow inaccessible configuration under $HOME, 2013-04-12); it's not yet in any released version of git, though. [...] I've a PHP script in ~/public_html that runs git. Without the mentioned patch, the script bails out due to this error. This time it's Apache that gets me into trouble because at the time the PHP script and git run, $HOME is still /root, but the user identity is not root anymore. The patch is direly needed; without it, I need to use 'env HOME=/home/j6t /usr/local/bin/git' in my script. I could be remembering wrong, but I thought it was not so much under consideration as accepted for 1.8.4. I haven't heard any compelling reasons not to apply it. Would it would make sense against earlier releases as well? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dependencies and packaging (Re: [Administrivia] On ruby and contrib/)
Ramkumar Ramachandra wrote: Git is probably the _last_ thing to be complaining about when it comes to packaging. It would be nice if contrib/ files supported the usual make; make install; make clean targets. That's missing functionality that does matter to at least one packager. It would be nice if the dependencies associated to each piece of functionality or makefile flag were documented more clearly. Currently when e.g. features of gitweb gain dependencies I don't notice until the testsuite fails. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: `gitsubmodule` does not list modules with unicode characters
Fredrik Gustafsson wrote: I've looked into this a bit. Thanks for investigating. [...] Why don't we always print names quoted? IMHO the choose of line termination should not do anything else than alter the line termination. However, an other solution would be to use git ls-files -z in git-submodule.sh and then rewrite the perl-code to handle \0 instead of \n. The whole point of -z is that by using a terminator that is guaranteed not to appear in filenames, it avoids the need to quote filenames. Otherwise at least \n would need to be quoted. How about something like this patch? -- 8 -- Subject: ls-files doc: clarify purpose of -z option The purpose of the -z option is to avoid quoting issues by using a delimiter that implies a binary-clean parser and cannot appear in filenames, and in that spirit, -z turns off C-style quoting. But without looking carefully through the entire manpage, it is too easy to miss that. Reported-by: Fredrik Gustafsson iv...@iveqy.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Documentation/git-ls-files.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index c0856a6e..753c223f 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -75,7 +75,9 @@ OPTIONS succeed. -z:: - \0 line termination on output. + Terminate lines with NUL instead of LF. + This avoids the need to quote filenames; see the Output section + below for details. -x pattern:: --exclude=pattern:: -- 1.8.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
