[PATCH] remote-hg: fix commit messages
git fast-import expects an extra newline after the commit message data, but we are adding it only on hg-git compat mode, which is why the bidirectionality tests pass. We should add it unconditionally. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/remote-helpers/git-remote-hg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index a5f0013..5481331 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -362,6 +362,8 @@ def export_ref(repo, name, kind, head): else: modified, removed = get_filechanges(repo, c, parents[0]) +desc += '\n' + if mode == 'hg': extra_msg = '' @@ -385,7 +387,6 @@ def export_ref(repo, name, kind, head): else: extra_msg += extra : %s : %s\n % (key, urllib.quote(value)) -desc += '\n' if extra_msg: desc += '\n--HG--\n' + extra_msg -- 1.8.2.1.679.g509521a -- 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] remote-hg: fix commit messages
On Thu, Apr 18, 2013 at 1:06 AM, Felipe Contreras felipe.contre...@gmail.com wrote: git fast-import expects an extra newline after the commit message data, but we are adding it only on hg-git compat mode, which is why the bidirectionality tests pass. We should add it unconditionally. This doesn't depend on any other patches, and I think it might be worth to put it in 'maint', it's not like a *huge* deal, but it would be nice if a mercurial repo cloned with v1.8.2.2 and one cloned with v1.8.3 end up with the same commit ids. It would have been nicer if v1.8.1 had this already, but hey, it's new, and it's in contrib. Cheers. -- Felipe Contreras -- 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: put THEIR commits AFTER my commits with a single rebase command
Am 4/18/2013 7:18, schrieb Ilya Basin: desired result: A---B---C origin/master / D---E---F---G---A'---B'---C' *master Variant 1: git branch -f tmp git reset --hard origin/master git rebase tmp Variant 1a: git reset --hard origin/master git rebase @{1} This variant is bad, because 'git reset --hard' checks out some files and 'git rebase' rewrites them again before applying commits. It's a redundant job. Variant 2: git branch -f tmp origin/master git rebase --onto master master tmp git branch -f master git checkout master Too many commands. I want to do this with just one command. And I want to stay be on branch master in case of rebase conflicts. Perhaps this one: git merge origin/master git rebase ORIG_HEAD -- Hannes -- 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: put THEIR commits AFTER my commits with a single rebase command
On Thu, Apr 18, 2013 at 7:18 AM, Ilya Basin basini...@gmail.com wrote: I asked this on stackoverflow, but no reply. http://stackoverflow.com/questions/15971244/git-put-their-commits-after-my-commits-with-a-single-rebase-command Suppose master and origin/master diverged. I'm on master and I want to put the commits from origin/master after my commits and then push --force. A---B---C origin/master / D---E---F---G *master desired result: A---B---C origin/master / D---E---F---G---A'---B'---C' *master Note that if other people are working on top of origin/master, then what you are proposing is quite rude to them, since they must now manually rebase their own work on top of your rebased history. Rewriting public history is generally considered evil. Variant 1: git branch -f tmp git reset --hard origin/master git rebase tmp This variant is bad, because 'git reset --hard' checks out some files and 'git rebase' rewrites them again before applying commits. It's a redundant job. Variant 2: git branch -f tmp origin/master git rebase --onto master master tmp git branch -f master git checkout master Too many commands. I want to do this with just one command. And I want to stay be on branch master in case of rebase conflicts. git cherry-pick master..origin/master ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- 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
t6200: avoid path mangling issue on Windows
From: Johannes Sixt j...@kdbg.org MSYS bash interprets the slash in the argument core.commentchar=/ as root directory and mangles it into a Windows style path. Use a different core.commentchar to dodge the issue. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t6200-fmt-merge-msg.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index e7e945d..54b5744 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -179,8 +179,8 @@ test_expect_success '--log=5 with custom comment character' ' cat expected -EOF Merge branch ${apos}left${apos} - / By Another Author (3) and A U Thor (2) - / Via Another Committer + x By Another Author (3) and A U Thor (2) + x Via Another Committer * left: Left #5 Left #4 @@ -189,7 +189,7 @@ test_expect_success '--log=5 with custom comment character' ' Common #1 EOF - git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD actual + git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD actual test_cmp expected actual ' -- 1.8.2.1.1678.gf713add -- 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[2]: put THEIR commits AFTER my commits with a single rebase command
JH git cherry-pick master..origin/master Thanks Johan. -- 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/6] transport-helper: clarify *:* refspec
Felipe Contreras felipe.contre...@gmail.com writes: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). [...] -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' So what's wrong with the tests? Do they fail to test what they claim (how?), test something that wasn't reasonable to begin with, or something entirely different? -- Thomas Rast trast@{inf,student}.ethz.ch -- 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 rev-list --pretty=format: issue
Junio C Hamano gits...@pobox.com writes: Forrest Galloway f.gallo...@gmail.com writes: git 1.8.2.1 on OSX(Mountain Lion) installed with Homebrew I am seeing an issue when trying to format the output from rev-list command. git log --pretty=format:%H - %an, %ar : %s When I attempt the above string, instead of printing to the shell, LESS is opened and the output is displayed there. [...] Actually, less is running in both cases. [...] If you do not want pager, run it with no-pager, like this: git --no-pager log ...your other parameters... Also note that the pager is automatically disabled if you redirect stdout, so --no-pager is only required in some fringe cases, notably when attempting to run git under GDB. -- Thomas Rast trast@{inf,student}.ethz.ch -- 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] stash: tighten IS_STASH_LIKE logic
Currently, 'git stash show' and 'git stash apply' can show/apply any merge commit, as the test for setting IS_STASH_LIKE simply asserts if the commit is a merge. Improve the situation by asserting if the index_commit and the worktree_commit are based off the same commit, by checking that $REV^1 is equal to $REV^2^1. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- git-stash.sh | 3 ++- t/t3903-stash.sh | 11 +++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/git-stash.sh b/git-stash.sh index bbefdf6..d0428a8 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -366,13 +366,14 @@ parse_flags_and_rev() } i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null) - set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null) + set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: $REV^2^1 2/dev/null) s=$1 w_commit=$1 b_commit=$2 w_tree=$3 b_tree=$4 i_tree=$5 + test $b_commit = $6 IS_STASH_LIKE=t test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*}) IS_STASH_REF=t diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 5dfbda7..11bcd72 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -637,4 +637,15 @@ test_expect_success 'stash where working directory contains HEAD file' ' test_cmp output expect ' +test_expect_success 'show refuses to show any random merge commit' ' + git stash clear + git reset --hard + git checkout -b quux + test_commit bar + git checkout - + test_commit foo + git merge quux + test_must_fail git stash show HEAD +' + test_done -- 1.8.2.1.423.g4fb5c0a.dirty -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Felipe Contreras felipe.contre...@gmail.com writes: * How many times have you tracked regressions in transport helper's import/export functionality? Hint: zero. The real question to make the situation non-hypothetical would actually be how many times did you track a regression that bisected down to *this particular commit*. Any regression that ends up on another commit is irrelevant. I guess you realize how stupid my argument is. But how is yours different? You do realize that your claim that nobody is ever going to bisect down to your commit is as hypothetical as other people's claim (if you think it is not, then try to point us a proof that nobody is ever going to need a good message in the future to understand what I mean). We're trying to make all the code and all the commits clean. It seems to be a consensus here that review is good. I see no reason to purposely make some commits less good than others based on the fact that they may not be used in the future. Search your favorite search engine for broken window principle to get more arguments in this direction. * How many times has *anybody* done so? Hint: other than me, quite possibly zero. If you want to be the only developer, and avoid being disturbed by others, then why are you pushing your changes to git.git? Why are you even discussing on this list? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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/2] Add documentation for new expiry option values
The first patch changes the manpages for git gc and git reflog to document the new expiry options made possible by Junio's date.c: add parse_expiry_date(). Feel free to squash this patch onto yours. The second patch changes the parse_options() API documentation to mention that a no- prefix can also be used for non-boolean options. It is related only by the fact that I was confused by this omission when I was trying to understand your patch. Michael Haggerty (2): git-gc.txt, git-reflog.txt: document new expiry options api-parse-options.txt: document no- for non-boolean options Documentation/git-gc.txt | 5 +++-- Documentation/git-reflog.txt | 9 +++-- Documentation/technical/api-parse-options.txt | 2 ++ 3 files changed, 12 insertions(+), 4 deletions(-) -- 1.8.2.1 -- 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-gc.txt, git-reflog.txt: document new expiry options
Document the new values that can be used for expiry values where their use makes sense: * git reflog expire --expire=[all|never] * git reflog expire --expire-unreachable=[all|never] * git gc --prune=all Other combinations aren't useful and therefore no documentation is added (even though they are allowed): * git gc --prune=never is redundant with git gc --no-prune * git prune --expire=all is equivalent to providing no --expire option * git prune --expire=never is a NOP Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/git-gc.txt | 5 +++-- Documentation/git-reflog.txt | 9 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt index b370b02..2402ed6 100644 --- a/Documentation/git-gc.txt +++ b/Documentation/git-gc.txt @@ -62,8 +62,9 @@ automatic consolidation of packs. --prune=date:: Prune loose objects older than date (default is 2 weeks ago, - overridable by the config variable `gc.pruneExpire`). This - option is on by default. + overridable by the config variable `gc.pruneExpire`). + --prune=all prunes loose objects regardless of their age. + --prune is on by default. --no-prune:: Do not prune any loose objects. diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index fb8697e..70791b9 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -67,14 +67,19 @@ them. --expire=time:: Entries older than this time are pruned. Without the option it is taken from configuration `gc.reflogExpire`, - which in turn defaults to 90 days. + which in turn defaults to 90 days. --expire=all prunes + entries regardless of their age; --expire=never turns off + pruning of reachable entries (but see --expire-unreachable). --expire-unreachable=time:: Entries older than this time and not reachable from the current tip of the branch are pruned. Without the option it is taken from configuration `gc.reflogExpireUnreachable`, which in turn defaults to - 30 days. + 30 days. --expire-unreachable=all prunes unreachable + entries regardless of their age; --expire-unreachable=never + turns off early pruning of unreachable entries (but see + --expire). --all:: Instead of listing refs explicitly, prune all refs. -- 1.8.2.1 -- 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/7] Rework git core for native submodules
Philip Oakley wrote: Would it be possible to summarise the key points and proposals of where the subject is now? Sure. If you want an update from the current approach, wait for a v2; I'm cooking it for some time, and getting some resulting ideas merged into upstream early (look for clone.submoduleGitDir on the list, for instance). When upstream is in better shape to ease in a better fundamental design, I'll post my v2 to the list. I'll refrain from posting any updates now, because I don't think the resulting discussion will generate any value. If you want to know what this thread was about, I think [1] and [2] summarize my arguments quite well. [1]: http://thread.gmane.org/gmane.comp.version-control.git/220047/focus=220436 [2]: http://thread.gmane.org/gmane.comp.version-control.git/220047/focus=220495 -- 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] api-parse-options.txt: document no- for non-boolean options
Document that the no- prefix can also be used for non-boolean options. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-parse-options.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 32ddc1c..2ff368e 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -41,6 +41,8 @@ The parse-options API allows: * Boolean long options can be 'negated' (or 'unset') by prepending `no-`, e.g. `--no-abbrev` instead of `--abbrev`. Conversely, options that begin with `no-` can be 'negated' by removing it. + Other long options can be unset (e.g., set string to NULL, set + integer to 0) by prepending `no-`. * Options and non-option arguments can clearly be separated using the `--` option, e.g. `-a -b --option -- --this-is-a-file` indicates that -- 1.8.2.1 -- 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/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). [...] -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' So what's wrong with the tests? Do they fail to test what they claim (how?), test something that wasn't reasonable to begin with, or something entirely different? Look at the code comment, and look at the now updated documentation that assumes that *:* was reasonable. Given the available information, it would be reasonable to assume that *:* did work, but it didn't work, and it's not really possible to fix it, even if we wanted to, it would be a hack. It's better to accept that fact and stop worrying too much about what would be the best way to do the wrong thing. -- Felipe Contreras -- 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 1/6] transport-helper: clarify *:* refspec
On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). In what way doesn't it work? If I specify that refspec then I do get output that appears sensible. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 15 --- transport-helper.c | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031..0c91aba 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. This is wrong. As your later patch makes clearer, there is no implied refspec for push - it only works for fetch. I found the wording you've reverted to extremely misleading. How about something like this: For historical reasons, 'import' treats the absence of a 'refspec' line as equivalent to `refspec *:*`; remote helpers should always specify an explicit refspec. ? 'bidi-import':: This modifies the 'import' capability. diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027..cd1873c 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' - test_expect_success 'pulling without marks' ' (cd local2 GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) diff --git a/transport-helper.c b/transport-helper.c index dcd8d97..cea787c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, * were fetching. * * (If no refspec capability was specified, for historical - * reasons we default to *:*.) + * reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as git fetch can use the value to write feedback to the -- 1.8.2.1.679.g509521a -- 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[2]: put THEIR commits AFTER my commits with a single rebase command
JS Perhaps this one: JSgit merge origin/master JSgit rebase ORIG_HEAD JS -- Hannes Wouldn't I have to resolve conflicts twice? BTW, during the rebase, can I tell git to rewrite a different branch upon rebase success or abort? git branch -f tmp origin/master git rebase --onto master master tmp if [ $? -ne 0 ]; then # modify some file in .git/ ? else git branch -f master git checkout master fi -- -- 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: What's cooking between #05 and #06
On Wed, Apr 17, 2013 at 11:25:07PM +0200, Jens Lehmann wrote: I like it, as it gets rid of the top-level requirement. But from my testing it looks like we're not quite there yet. 'summary' and 'status' behave as if they were run in the toplevel directory, while a git status shows all filenames relative to the current directory. Me thinks 'summary' and 'status' (and all other submodule commands) should behave like status and print relative paths too. I'm not really sure yet how $sm_path should behave for 'foreach', but I suspect having it relative to the current directory would be the way to go (which it currently isn't). When submodule add is run with a relative path it is relative to the top-level directory, which I find confusing (and won't play well with shell completion). This confused me for a bit because I was sure I handled this, but I see I missed relative submodules URLs. So the path at which to put the submodule is correct, but the path from which to clone is not. 'deinit .' doesn't deinit submodules above the current directory (but prints the path relative to top-level) while 'init' will initialize all submodules known to the superproject. I can't see how this happens. 'init' uses module_list which has been updated to handle relative paths. So I expect 'git submodule init .' to work correctly here. I would expect either of them to act on all submodules when given no extra arguments. So this is a good start, but it looks like there is some work left to do before this can hit master. Thanks for the feedback. -- 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] t3400 (rebase): add failing test for a peculiar rev spec
'git rebase' does not recognize revisions specified as :/text. This is because the attempts to rev-parse ${REV}^0, which fails in this case. Add a test to document this failure. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Okay, so I'm not sure what the right fix for this is. - The :/text syntax seems to be broken in the first place, as it can't be combined with ^ or @. I'd like to be able to say {:/bardery}^1, or even {:/foomery}^{/communist mule}. Why shouldn't I be allowed to do this? - The failure occurs in git-rebase.sh:403. Is it using the ^0 only to make sure that the revision specified is a commit? Surely, there'a a better way to do this? Can someone point me in the right direction? t/t3400-rebase.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index f6cc102..42ca1e0 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -88,6 +88,14 @@ test_expect_success 'rebase fast-forward to master' ' test_i18ngrep Fast-forwarded HEAD to my-topic-branch out ' +test_expect_failure 'rebase against revision specified as :/text' ' + git checkout my-topic-branch^ + sha1=$(git rev-parse :/Add B) + git rebase $sha1 + git checkout my-topic-branch^ + git rebase :/Add B +' + test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' -- 1.8.2.1.423.g4fb5c0a.dirty -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 2:44 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: * How many times have you tracked regressions in transport helper's import/export functionality? Hint: zero. The real question to make the situation non-hypothetical would actually be how many times did you track a regression that bisected down to *this particular commit*. Any regression that ends up on another commit is irrelevant. I guess you realize how stupid my argument is. But how is yours different? I did not make any argument (stupid or otherwise), I made I claim; I won't waste my time with hypotheticals. You do realize that your claim that nobody is ever going to bisect down to your commit is as hypothetical as other people's claim (if you think it is not, then try to point us a proof that nobody is ever going to need a good message in the future to understand what I mean). Yeah, they are both hypotheticals, the only difference is that your claim is very easy to prove; all you need is *ONE* example. But I'm very happy to withdraw my claim, as long as you withdraw your claim as well, and we go back to the default position: we don't know if anybody will every look at these commit messages again. We're trying to make all the code and all the commits clean. It seems to be a consensus here that review is good. I see no reason to purposely make some commits less good than others based on the fact that they may not be used in the future. You have to prove first that they are less good, and the best way to do that is provide commit messages of your own, if you do that, they can be used instead, but if you don't, what do you propose to do? Drop the patches? Search your favorite search engine for broken window principle to get more arguments in this direction. More like broken windows hypothesis, which is not without its critics. * How many times has *anybody* done so? Hint: other than me, quite possibly zero. If you want to be the only developer, and avoid being disturbed by others, then why are you pushing your changes to git.git? Why are you even discussing on this list? Doesn't matter, it's still *HYPOTHETICAL* that anybody will every hit this in a bisect. Now, if you agree it's all hypothetical, the next rational thing to do is risk analysis: how likely is it to happen, and what would be the impact if it does? The answer to both questions is: close to *ZERO*. So, considering the nature of these patches (a remote-helper in the contrib area that is relatively new), and the active developers (me), I'd say it's much more important to get the fixes in, than to document every little quirk, detail and reasoning behind them. It's the balance I think it's best at this point, and it is my time, and it is my decision what I do with it. It might also help to compare oranges with oranges, and with regards to remote-hg transport helpers, I do believe the one in contrib/remote-helpers has the best commit messages: msysgit's remote-hg: --- commit 6bbd5365988d63780acc2ab407878eef8c19b47c Author: Sverre Rabbelier srabbel...@gmail.com Date: Sun Aug 22 01:22:14 2010 -0500 git_remote_helpers: add fastimport library git_remote_helpers/fastimport/__init__.py | 0 git_remote_helpers/fastimport/commands.py | 469 + git_remote_helpers/fastimport/dates.py| 79 +++ git_remote_helpers/fastimport/errors.py | 182 + git_remote_helpers/fastimport/head_tracker.py | 47 +++ git_remote_helpers/fastimport/helpers.py | 88 + git_remote_helpers/fastimport/idmapfile.py| 65 + git_remote_helpers/fastimport/parser.py | 621 ++ git_remote_helpers/fastimport/processor.py| 222 +++ git_remote_helpers/setup.py | 3 +- 10 files changed, 1775 insertions(+), 1 deletion(-) --- gitifyhg: -- commit 4b364563cd705dc5e69082e6b80d304fe50b9c9c Author: Alex Sydell a...@dropbox.com Date: Sat Mar 23 23:46:33 2013 -0700 Report correct (instead of unknown) hashes when importing refs into git gitifyhg/gitifyhg.py | 27 +-- gitifyhg/hgimporter.py | 20 ++-- gitifyhg/util.py | 44 test/test_push.py | 31 +++ 4 files changed, 94 insertions(+), 28 deletions(-) --- And of course, the best place to discuss the lack of good commit messages, is in the patches themselves, which are after all, sent to the mailing list for everyone to review. Cheers. -- Felipe Contreras -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Felipe Contreras wrote: I think the commit message is fine, you don't. So YOU go ahead and write the proper one. If you don't, all you are doing is being an impediment to progress. Hey Felipe. Let's get a few things straightened out first: - We all act in our selfish interests, and write code to scratch our personal itches. I don't write code or commit messages for anyone else, and neither should you. - However, we're not working in isolation. We have this giant mailing list where we all post our patches. It's like a bazaar where we compete against other patches for developer attention and potential reviewers. In other words, it's a free market, and we're selling our product: if it fails to sell, will you blame the market or your product? I write clear code and beautiful commit messages exactly for this reason: I'm fighting for attention! - We have to learn to interoperate with others' code and conventions, if we want to be part of the community. That doesn't mean that we drown out our individuality, but it means that a our patch series has to conform to some minimal, loose, and evolving standard. Now, you can argue that many of the existing conventions are outdated (I do it all the time), but it cannot change overnight. Your influence on the community will show up over an extended period of time. - We are not an old enterprise who blame breakages on a few individuals, and fire them. We're a community where all of us are equally responsible for all parts of the code. I am as responsible for the remote-hg code in master as you are, as I had every opportunity to review it when the patch series came up on the list. I might have chosen not to, but that doesn't relieve me of responsibility. - We don't practice division of labour. There are no managers, testing people, documentation people, code-writing people, commit-message writing people etc. Everyone has to do some portion of all these tasks, although we try to keep the boring work/ technical debt to a minimum. Don't ask other people to write commit messages for your code. -- 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 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 3:24 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). In what way doesn't it work? If I specify that refspec then I do get output that appears sensible. % git checkout origin/master % make -C t t5801-remote-helpers.sh not ok 15 - pushing with straight refspec # TODO known breakage It fails for me here. Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 15 --- transport-helper.c | 2 +- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031..0c91aba 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. This is wrong. It has been like that since v1.7.0. As your later patch makes clearer, there is no implied refspec for push - it only works for fetch. I found the wording you've reverted to extremely misleading. How about something like this: For historical reasons, 'import' treats the absence of a 'refspec' line as equivalent to `refspec *:*`; remote helpers should always specify an explicit refspec. Maybe my version was misleading because it didn't mention it was for historical reasons, but yours is plain wrong; remote helpers might not be using 'import' or 'export' at all, so not all remote helpers should always specify an explicit refspec. And if the previous text It is recommended that all importers providing the 'import' capability use this. It's mandatory for 'export'. does not convey the idea these remote helpers should always specify an explicit refspec, I don't know what it does. -- Felipe Contreras -- 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: State of CVS-to-git conversion tools (Was: Re: cvsps: bad usage: invalid argument --norc)
Ilya Basin basini...@gmail.com: Hi Eric. I tried --fast-export. It's 2 times faster. The first thing that differs: in cvsps2 commits with adjacent timestamps were joined into one (see the attached files). Do you know the reason? The cvsps guy included code to do that. Keith Packard didn't. Sorry I can't be more helpful, but that's about all I know. I didn't write either analysis stage; I understand cvsps's, somewhat, because I had to fix several nasty bugs in it. I *don't* understand cvs-fast-export's analysis stage very well yet, because it has no obvious bugs that have required me to dive in. (Keith's notes document one major bug, which may be inherent to the mismatch between file- and changest-orientation and not fixable in the general case, though I will try.) Does this --fast-export thing support what John mentioned, the incremental import support? Does 'git fast-import' has it? cvs-fast-export does not have incremental-import support. Whether git-cvs-import has it depend on which version you have and what backend it it is using. I don't maintain that wrapper. I need it, because full import takes too long. The central repo of my employer is CVS, other people commit to it and I use git internally to be able to tidy my commit history before exporting to CVS. You are out of luck. That feature was dependent on a very fragile coupling between the old output format and a bunch of unmaintainably horrible Perl in the git-cvs-import wrapper script. It didn't work very well; frankly, I'm amazed it worked at all. The things I had to do to fix the serious bugs in cvsps2 and make it output a fast-import stream had the side effect of breaking that coupling. cvsps3 won't give you that feature. Dropping back to cvsps2 to keep that feature will expose you to the cvsps2 bugs. I'm sorry these tools are such a mess. I'm trying to fix that, but it's hard, slow work. The problems are deeply ugly and the edge cases have poisoned spikes. -- a href=http://www.catb.org/~esr/;Eric S. Raymond/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 v2 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 04:27:57AM -0500, Felipe Contreras wrote: On Thu, Apr 18, 2013 at 3:24 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote: diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index f506031..0c91aba 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -174,8 +174,8 @@ ref. This capability can be advertised multiple times. The first applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. This is wrong. It has been like that since v1.7.0. That doesn't mean it's right. I suspect that it was originally correct, but then 'export' was added at which point this became stale. As your later patch makes clearer, there is no implied refspec for push - it only works for fetch. I found the wording you've reverted to extremely misleading. How about something like this: For historical reasons, 'import' treats the absence of a 'refspec' line as equivalent to `refspec *:*`; remote helpers should always specify an explicit refspec. Maybe my version was misleading because it didn't mention it was for historical reasons, but yours is plain wrong; remote helpers might not be using 'import' or 'export' at all, so not all remote helpers should always specify an explicit refspec. And if the previous text It is recommended that all importers providing the 'import' capability use this. It's mandatory for 'export'. does not convey the idea these remote helpers should always specify an explicit refspec, I don't know what it does. I didn't say mine was correct, but there was a reason that I wanted to change the existing text. Just going back to what was there before is not a good way to make things better. In my copy of pu I don't see the text It's mandatory for 'export', it just stops after It is recommended that all importers providing the 'import' capability use this. That paragraph also starts with This modifies the 'import' capability making no mention of export. Perhaps we need a slightly more extensive re-write of the documentation for refspec to make it very clear where it applies and when it is needed. -- 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
Pushing/fetching from/into a shallow-cloned repository
The git-clone manual page, both [1] and my local copy coming with Git for Windows 1.8.1, say about the --depth command-line option: --depth depth Create a shallow clone with a history truncated to the specified number of revisions. A shallow repository has a number of limitations (you cannot clone or fetch from it, nor push from nor into it), but is adequate if you are only interested in the recent history of a large project with a long history, and would want to send in fixes as patches. But having done a shallow clone (--depth=1) of one of my repositories, I was able to record a new commit, push it out to a reference bare repository and then fetch back to another clone of the same repository just fine. I have then killed my test commit doing a forced push from another clone and subsequently was able to do `git fetch` in my shallow clone just fine. So I observe pushing/fetching works OK at least for a simple case like this one. Hence I'd like to ask: if the manual page is wrong or I'm observing some corner case? -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 4:19 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: I think the commit message is fine, you don't. So YOU go ahead and write the proper one. If you don't, all you are doing is being an impediment to progress. Hey Felipe. Let's get a few things straightened out first: - We all act in our selfish interests, and write code to scratch our personal itches. I don't write code or commit messages for anyone else, and neither should you. - However, we're not working in isolation. We have this giant mailing list where we all post our patches. It's like a bazaar where we compete against other patches for developer attention and potential reviewers. In other words, it's a free market, and we're selling our product: if it fails to sell, will you blame the market or your product? I write clear code and beautiful commit messages exactly for this reason: I'm fighting for attention! Except the customers are not git developers, it's git users. Git developers rejecting patches because of the commit message is akin to distributors rejecting products because they don't like the transportation packages; they are only hurting themselves, by hurting their customers. - We have to learn to interoperate with others' code and conventions, if we want to be part of the community. That doesn't mean that we drown out our individuality, but it means that a our patch series has to conform to some minimal, loose, and evolving standard. Now, you can argue that many of the existing conventions are outdated (I do it all the time), but it cannot change overnight. Your influence on the community will show up over an extended period of time. And the only way it can change is by discussing. The only one that gets bitten by fixes not getting merged are git users, not me. So if a discussion of a commit message impedes the merging of the commit, I don't get affected, but when we have agreed to disagree on what constitutes a good message, and the patch is still on hold, then there's a problem. - We are not an old enterprise who blame breakages on a few individuals, and fire them. We're a community where all of us are equally responsible for all parts of the code. I am as responsible for the remote-hg code in master as you are, as I had every opportunity to review it when the patch series came up on the list. I might have chosen not to, but that doesn't relieve me of responsibility. I don't think so. Unless you added your Signed-off-by, you are not. - We don't practice division of labour. There are no managers, testing people, documentation people, code-writing people, commit-message writing people etc. Everyone has to do some portion of all these tasks, although we try to keep the boring work/ technical debt to a minimum. Don't ask other people to write commit messages for your code. I am not. Neither should they ask me to write the commit messages they want. They can make *suggestions*, and I can reject them. When two persons have different ideas, often times both are wrong, and the middle-ground is best, but sometimes a person reaches the middle-ground, and sometimes one person was right from the start. But when everyone shares the *assumption* that there is never a commit message that is too long, you know the wrestling mat of ideas is rigged. I wonder if I should write a commit message as long as a book chapter for a one-liner, only to prove a point, but I'm honestly afraid that it would be committed as is. And remember what started the conversation; do you think a patch with a possibly incomplete commit message should not be merged to pu (proposed updates), shouldn't even be mentioned in the what's cooking mail, and thus shouldn't even be considered cooking? Cheers. -- Felipe Contreras -- 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: Pushing/fetching from/into a shallow-cloned repository
On Thu, 18 Apr 2013 13:52:33 +0400 Konstantin Khomoutov kostix+...@007spb.ru wrote: The git-clone manual page, both [1] and my local copy coming with Git for Windows 1.8.1, say about the --depth command-line option: [...] Oops, by [1] I wanted to refer to this version: https://www.kernel.org/pub/software/scm/git/docs/git-clone.html -- 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 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 4:45 AM, John Keeping j...@keeping.me.uk wrote: On Thu, Apr 18, 2013 at 04:27:57AM -0500, Felipe Contreras wrote: Maybe my version was misleading because it didn't mention it was for historical reasons, but yours is plain wrong; remote helpers might not be using 'import' or 'export' at all, so not all remote helpers should always specify an explicit refspec. And if the previous text It is recommended that all importers providing the 'import' capability use this. It's mandatory for 'export'. does not convey the idea these remote helpers should always specify an explicit refspec, I don't know what it does. I didn't say mine was correct, but there was a reason that I wanted to change the existing text. Just going back to what was there before is not a good way to make things better. And just because it was that way before doesn't mean it was worst. Your patch essentially switched from it is implied, to it should be explicit, which is wrong. Going back to the previous text is the simplest change that restores a reasonable explanation. The next patches in this series deal with the rest of the issues in this text. In my copy of pu I don't see the text It's mandatory for 'export', it just stops after It is recommended that all importers providing the 'import' capability use this. That paragraph also starts with This modifies the 'import' capability making no mention of export. This is patch 1 of 6, keep going. -- Felipe Contreras -- 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
ไฟใต้ กับพวก สวมรอย
ไฟใต้ ไม่จัดการพวก 'สวมรอย' ก่อน เห็นทีจะยากที่ใต้จะกลับมาร่มเย็นอีกครั้ง... ไทยรัฐออนไลน์พยายามค้นหาข้อมูลจากคนใน หน่วยงานระดับปฏิบัติการในพื้นที่หลายต่อหลายคน ถึงสภาพจริงที่เกิดขึ้น โดยเฉพาะเรื่องของการข่าวว่ามันเหมือน หรือแตกต่างไปจากที่รับทราบกันแค่ไหนอย่างไร ก็ได้คำตอบมาว่า จริงอยู่ที่จุดเริ่มต้นของความรุนแรงในพื้นที่ 3 จ.ชายแดนใต้นั้น อาจเริ่มมาจากการกระทำภายใต้อุดมการณ์อย่างแท้จริง แต่เมื่อเวลาผ่านไปนานร่วมสิบปี กลับพบว่ามีกลุ่มคนบางกลุ่ม ฉกฉวยโอกาสในการเกิดความไม่สงบเหล่านี้ เป็นช่องทางในการกระทำสิ่งเพื่อประโยชน์ของตนเองจนสถานการณ์ขยายตัว ซึ่งล้วนแต่ก็เป็นเรื่องที่ผิดกฎหมายแทบทั้งนั้น ไม่ว่าจะเป็นเรื่องของการค้าของผิดกฎหมายเช่น น้ำมันเถื่อน ยาเสพติด โดยเฉพาะยาเสพติด เจ้าหน้าที่ที่คลุกคลีในพื้นที่จริงๆ ต่างพูดเหมือนกันว่า กลายเป็นสิ่งหนึ่งที่ ระบาด ในพื้นที่ และมีการใช้ยาเสพติดเหล่านี้ เป็นสิ่งจูงใจในการก่อเหตุต่างๆ ไปแล้วบอกเล่าเหล่านี้ ถูกสำทับ ให้หนักแน่นด้วยการเล่าต่อว่า ขณะลงพื้นที่ไม่ยากเลยที่จะพบเห็นถุงยา ร่วงหล่น ในสวนยาง... เมื่อไทยรัฐออนไลน์จี้หนักเข้าไปถึงมุมมองถึงเหตุแห่งความรุนแรงที่เกิด กลับถูกขยายความต่อว่า หลังๆ หลายเหตุการณ์เหมือนถูก สวมรอย อันเนื่องมาจากเหตุการณ์ความรุนแรงที่เกิดขึ้นในพื้นที่ บางครั้งตอบไม่ได้ว่าทำไปเพื่ออะไร จะแค่หวังเพียงเพื่อป่วนเมือง บางครั้งยังมองไม่ได้ขนาดนั้นเลย แหล่งที่มา...http://www.thairath.co.th/content/pol/337575
Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 05:02:11AM -0500, Felipe Contreras wrote: On Thu, Apr 18, 2013 at 4:45 AM, John Keeping j...@keeping.me.uk wrote: In my copy of pu I don't see the text It's mandatory for 'export', it just stops after It is recommended that all importers providing the 'import' capability use this. That paragraph also starts with This modifies the 'import' capability making no mention of export. This is patch 1 of 6, keep going. Ah, I see now. I had read all of the patches initially, but then focused on this one for discussion. The end result after this series does look good, but I find it slightly strange to take a step backwards on the way. -- 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 3/6] transport-helper: clarify pushing without refspecs
On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote: This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Let's at least tell the user what's going on. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 6 +++--- transport-helper.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ba7240c..4d26e37 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -162,8 +162,8 @@ Miscellaneous capabilities For remote helpers that implement 'import' or 'export', this capability allows the refs to be constrained to a private namespace, instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' or 'export' - capabilities use this. + It is recommended that all importers providing the 'import' + capability use this. It's mandatory for 'export'. s/It's/It is/ + A helper advertising the capability `refspec refs/heads/*:refs/svn/origin/branches/*` diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index cd1873c..3eeb309 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_failure 'pushing without refspecs' ' +test_expect_success 'pushing without refspecs' ' test_when_finished (cd local2 git reset --hard origin) (cd local2 echo content file git commit -a -m ten - GIT_REMOTE_TESTGIT_REFSPEC= git push) - compare_refs local2 HEAD server HEAD + GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error) + grep remote-helper doesn.t support push; refspec needed error ' test_expect_success 'pulling without marks' ' diff --git a/transport-helper.c b/transport-helper.c index cea787c..4d98567 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data-refspecs) + die(remote-helper doesn't support push; refspec needed); I think the refspec needed text is likely to be confusing if an end-user ever sees this message. I'm not sure how we can provide useful feedback for both remote helper authors and end-users though. + helper = get_helper(transport); write_constant(helper-in, export\n); @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport *transport, char *private; unsigned char sha1[20]; - if (!data-refspecs) - continue; private = apply_refspecs(data-refspecs, data-refspec_nr, ref-name); if (private !get_sha1(private, sha1)) { strbuf_addf(buf, ^%s, private); -- 1.8.2.1.679.g509521a -- 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 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 5:11 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote: This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Let's at least tell the user what's going on. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 6 +++--- transport-helper.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ba7240c..4d26e37 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -162,8 +162,8 @@ Miscellaneous capabilities For remote helpers that implement 'import' or 'export', this capability allows the refs to be constrained to a private namespace, instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' or 'export' - capabilities use this. + It is recommended that all importers providing the 'import' + capability use this. It's mandatory for 'export'. s/It's/It is/ What's the difference? --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data-refspecs) + die(remote-helper doesn't support push; refspec needed); I think the refspec needed text is likely to be confusing if an end-user ever sees this message. I'm not sure how we can provide useful feedback for both remote helper authors and end-users though. It doesn't have to be. They'll google it, or they'll post a bug report with this warning verbatim, or they'll just ignore it. I don't think there's much more we can do to help in either of these cases. -- Felipe Contreras -- 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
Bug with rev-list --reverse?
Hi, If I do these: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? Cheers. -- Felipe Contreras -- 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 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 05:14:18AM -0500, Felipe Contreras wrote: On Thu, Apr 18, 2013 at 5:11 AM, John Keeping j...@keeping.me.uk wrote: On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote: This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Let's at least tell the user what's going on. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 6 +++--- transport-helper.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ba7240c..4d26e37 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -162,8 +162,8 @@ Miscellaneous capabilities For remote helpers that implement 'import' or 'export', this capability allows the refs to be constrained to a private namespace, instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' or 'export' - capabilities use this. + It is recommended that all importers providing the 'import' + capability use this. It's mandatory for 'export'. s/It's/It is/ What's the difference? It's is considered informal, while we do adopt that tone on the mailing list and sometimes in the documentation, in general I think we should avoid contractions in the formal documentation. In this case it is particularly jarring because it immediately follows a sentence where we use the full It is form. -- 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 with rev-list --reverse?
On Thu, Apr 18, 2013 at 05:17:14AM -0500, Felipe Contreras wrote: If I do these: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? I expect to get the same output. This is probably because I consider --reverse to be an output filter. So I expect to show the commits v1.8.1.5^..v1.8.1.6 -1 which selects a single commit and then show that in reverse order. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Felipe Contreras wrote: Except the customers are not git developers, it's git users. Git developers rejecting patches because of the commit message is akin to distributors rejecting products because they don't like the transportation packages; they are only hurting themselves, by hurting their customers. Huh? I certainly don't develop for some git users I don't even know or care about. In this order of precedence, my customers are: 1. Me. 2. People who develop git.git, whom I have to cooperate with. 3. People who exercise git heavily like the linux.git community, as opposed to some little projects that operate using pull requests on GitHub. ... 137. People who incidentally choose to use git. 138. People who incidentally choose to use git, but aren't on Linux. I don't know if Junio or the others share this view, but this is how I personally operate and I'm very happy. And nobody is hurting anyone else. Someone wrote some code, and failed to sell it to the community. That's all happened. The only one that gets bitten by fixes not getting merged are git users, not me. So if a discussion of a commit message impedes the merging of the commit, I don't get affected, but when we have agreed to disagree on what constitutes a good message, and the patch is still on hold, then there's a problem. I think the whole issue of whether your commit message conforms to some transcendental standard is orthogonal to the issue. Is your patch getting attention? Has it attracted reviewers, and turned up on the latest What's cooking? I don't think so. Unless you added your Signed-off-by, you are not. Okay, so your view differs. I am not. Neither should they ask me to write the commit messages they want. They can make *suggestions*, and I can reject them. Ofcourse you have a right to reject suggestions. The question at the end of the day doesn't change: did you manage to get people to read your patch? When two persons have different ideas, often times both are wrong, and the middle-ground is best, but sometimes a person reaches the middle-ground, and sometimes one person was right from the start. https://yourlogicalfallacyis.com/middle-ground :) But when everyone shares the *assumption* that there is never a commit message that is too long, you know the wrestling mat of ideas is rigged. I wonder if I should write a commit message as long as a book chapter for a one-liner, only to prove a point, but I'm honestly afraid that it would be committed as is. I'm with you, and don't share that assumption. I'm not accusing you of writing commit messages that don't conform to some transcendental standard either: I didn't look at your patches in the first place, because the simple Signed-off-by: one-liner in the body didn't really make me want to read it. And remember what started the conversation; do you think a patch with a possibly incomplete commit message should not be merged to pu (proposed updates), shouldn't even be mentioned in the what's cooking mail, and thus shouldn't even be considered cooking? It's irrelevant what I think or others think. The point is that it wasn't mentioned. Now, why wasn't it mentioned? Is it because Junio and the community hate you, and are conspiring against getting your code merged? Or is it because it didn't catch anyone's eye, and Junio was waiting for it to happen (as always)? TL;DR version: Your goal in submitting a patch is to sell it to other people in the community. If enough people like your patch, it gets merged (but that is only the second step). Your goal is not to fix problems for some unknown users, or argue about some transcendental standard. Ofcourse the community shares some view about what a patch should look like, but you can mould those expectations gradually. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 5:27 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Except the customers are not git developers, it's git users. Git developers rejecting patches because of the commit message is akin to distributors rejecting products because they don't like the transportation packages; they are only hurting themselves, by hurting their customers. Huh? I certainly don't develop for some git users I don't even know or care about. In this order of precedence, my customers are: 1. Me. 2. People who develop git.git, whom I have to cooperate with. 3. People who exercise git heavily like the linux.git community, as opposed to some little projects that operate using pull requests on GitHub. ... 137. People who incidentally choose to use git. 138. People who incidentally choose to use git, but aren't on Linux. As they are for most open source developers on the planet, not me. I believe a project is nothing without its users. And nobody is hurting anyone else. Someone wrote some code, and failed to sell it to the community. That's all happened. If remote-hg wasn't available for users, they would be hurt; if stash wasn't available, if rebase --interactive didn't exist, if there was no msysgit, if it wasn't so fast, if the object model wasn't so simple and extensible; users would be hurt. And if users didn't have all these, there would be less users, and if there were less users, there would be less developers, and mercurial might have been more popular, and most repositories you have to work on would be in mercurial, and you might be developing mercurial right now. But I won't bother trying to convince you that no project is more important than its users (in the words of Linus Torvalds), because most people don't see the big picture. I don't think so. Unless you added your Signed-off-by, you are not. Okay, so your view differs. I am not. Neither should they ask me to write the commit messages they want. They can make *suggestions*, and I can reject them. Ofcourse you have a right to reject suggestions. The question at the end of the day doesn't change: did you manage to get people to read your patch? No, at the end of the day what matters is: did the users benefit from this? The answer for this particular patch is no, and it's not my fault. When two persons have different ideas, often times both are wrong, and the middle-ground is best, but sometimes a person reaches the middle-ground, and sometimes one person was right from the start. https://yourlogicalfallacyis.com/middle-ground :) Yeah, but I didn't claim that, I said sometimes one person was right from the start, no middle-ground. But when everyone shares the *assumption* that there is never a commit message that is too long, you know the wrestling mat of ideas is rigged. I wonder if I should write a commit message as long as a book chapter for a one-liner, only to prove a point, but I'm honestly afraid that it would be committed as is. I'm with you, and don't share that assumption. s/everyone/almost everyone/ I'm not accusing you of writing commit messages that don't conform to some transcendental standard either: I didn't look at your patches in the first place, because the simple Signed-off-by: one-liner in the body didn't really make me want to read it. And remember what started the conversation; do you think a patch with a possibly incomplete commit message should not be merged to pu (proposed updates), shouldn't even be mentioned in the what's cooking mail, and thus shouldn't even be considered cooking? It's irrelevant what I think or others think. The point is that it wasn't mentioned. Now, why wasn't it mentioned? Is it because Junio and the community hate you, and are conspiring against getting your code merged? Or is it because it didn't catch anyone's eye, and Junio was waiting for it to happen (as always)? My abridged version of the story is: because Jeff King pointed to an area of improvement he wasn't even strongly attached to, I agreed to resubmit, Junio saw that, then I changed my mind, Junio probably didn't see that, and then he forgot about it. Then, since it's taboo to suggest that a concise commit message is fine, a discussion sprung. TL;DR version: Your goal in submitting a patch is to sell it to other people in the community. I disagree. If enough people like your patch, it gets merged (but that is only the second step). Your goal is not to fix problems for some unknown users, or argue about some transcendental standard. I disagree. Ofcourse the community shares some view about what a patch should look like, but you can mould those expectations gradually. If experience is any guide, doesn't look like that. But I've noticed that after many months that a patch has been sent people realize that it's more important to get the damn issue fixed than to have a hugely verbose commit message... Cheers. -- Felipe
Re: Bug with rev-list --reverse?
Felipe Contreras: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? Quoting the manual page: Commit Limiting Besides specifying a range of commits that should be listed using the special notations explained in the description, additional commit limiting may be applied. Note that they are applied before commit ordering and formatting options, such as --reverse. Given that, I would expect the output to be the same. -- \\// Peter - http://www.softwolves.pp.se/ -- 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: put THEIR commits AFTER my commits with a single rebase command
Am 4/18/2013 10:33, schrieb Ilya Basin: JS Perhaps this one: JSgit merge origin/master JSgit rebase ORIG_HEAD JS -- Hannes Wouldn't I have to resolve conflicts twice? Yes. But you did run 'git config rerere.enabled true' when you started with git, didn't you? ;-) Anyway, Johan's idea to use git cherry-pick is much better. BTW, during the rebase, can I tell git to rewrite a different branch upon rebase success or abort? git branch -f tmp origin/master git rebase --onto master master tmp if [ $? -ne 0 ]; then # modify some file in .git/ ? What do you expect here? Failure of git rebase means that it is not complete, yet. So far, nothing has been rewritten. So what? Perhaps you mean: # never mind git rebase --abort else git branch -f master git checkout master fi -- Hannes -- 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 with rev-list --reverse?
On Thu, Apr 18, 2013 at 5:26 AM, John Keeping j...@keeping.me.uk wrote: On Thu, Apr 18, 2013 at 05:17:14AM -0500, Felipe Contreras wrote: If I do these: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? I expect to get the same output. This is probably because I consider --reverse to be an output filter. So I expect to show the commits v1.8.1.5^..v1.8.1.6 -1 which selects a single commit and then show that in reverse order. How about this: % git log --oneline --reverse --max-count=1 v1.8.1.5^..v1.8.1.6 In this case --max-count is acting as start from the first commit before the tip, not as output a maximum of one commit. Given that the name is max-count, I expect it to be the later. And if max-count doesn't select a maximum of n commits, then what does? -- Felipe Contreras -- 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 with rev-list --reverse?
On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting pe...@softwolves.pp.se wrote: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? Quoting the manual page: Commit Limiting Besides specifying a range of commits that should be listed using the special notations explained in the description, additional commit limiting may be applied. Note that they are applied before commit ordering and formatting options, such as --reverse. Given that, I would expect the output to be the same. If expectations were based on documentation, all one has to do is document bugs, and there would be no bugs anymore :) Code can be changed to fit more appropriately user expectations (which are independent of documentation), and the documentation updated accordingly. -- Felipe Contreras -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Since you disagreed with the rest, I'll only respond to this part: Felipe Contreras wrote: But I won't bother trying to convince you that no project is more important than its users (in the words of Linus Torvalds), because most people don't see the big picture. I didn't say otherwise. What I'm saying is: my personal incentive to write code does not prioritize the supposed benefit of some unknown user somewhere on the planet above everything else. My personal incentive prioritizes me, and my immediate circle (ie. the git community). The benefit propagates outwards to extended circles until it reaches the people I care least about: incidental end-users. That's how people are connected: how can I care about distant unknown people I'm not connected to? The people in the outermost circles benefit the least, because they didn't get a say in the development. All they can do is write a rant about it on their blog, and hope that it gets fixed someday. You just ditched us, the inner circle of people who care about your work the most, and are instead trying to convince us that we're hurting some unknown hypothetical users by not merging your code immediately. If you think these users are more important to you than we are, then why are you posting your code on this mailing list? Start your own project that's focused on satisfying these users. It doesn't even need to be open source or have a community of reviewers, because all you care about are users. -- 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 crash in Ubuntu 12.04
Hi, Probably not because there are no debugging symbols. Not sure how ubuntu packages these symbols.. Would recompiling the source packages and debugging would give different results? Any chance you could publish the repository that causes the crash? -- Duy I don't think I can publish the repo online. But I am willing to try any steps on the repo to get more info. Most likely I would restore the latest repo to the stanby server I tested which works with any crash. Let me know what I can do to get more information. ./Siva. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Okay, one more segment needs to be responded to. Felipe Contreras wrote: If remote-hg wasn't available for users, they would be hurt; if stash wasn't available, if rebase --interactive didn't exist, if there was no msysgit, if it wasn't so fast, if the object model wasn't so simple and extensible; users would be hurt. And if users didn't have all these, there would be less users, and if there were less users, there would be less developers, and mercurial might have been more popular, and most repositories you have to work on would be in mercurial, and you might be developing mercurial right now. Flawed logic. A large number of users doesn't automatically imply good software with lots of features, or even a great development community. A great development community leads to great software. And great software leads to lots of users. Sure, there's a feedback loop pushing users to become developers; but it doesn't start with users of vaporware, leading to more developers joining the effort to turn that vaporware into a great product. Life doesn't begin with users. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 6:31 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Since you disagreed with the rest, I'll only respond to this part: Felipe Contreras wrote: But I won't bother trying to convince you that no project is more important than its users (in the words of Linus Torvalds), because most people don't see the big picture. I didn't say otherwise. What I'm saying is: my personal incentive to write code does not prioritize the supposed benefit of some unknown user somewhere on the planet above everything else. My personal incentive prioritizes me, and my immediate circle (ie. the git community). The benefit propagates outwards to extended circles until it reaches the people I care least about: incidental end-users. If the people that matter most are given the worst prioritization, it means the prioritization is wrong. That's how people are connected: how can I care about distant unknown people I'm not connected to? It's called empathy. The people in the outermost circles benefit the least, because they didn't get a say in the development. All they can do is write a rant about it on their blog, and hope that it gets fixed someday. To the detriment of the project. You just ditched us, the inner circle of people who care about your work the most, and are instead trying to convince us that we're hurting some unknown hypothetical users by not merging your code immediately. The users are real, the developers that will look retroatcively to the commit message of this patch are not. If you think these users are more important to you than we are, then why are you posting your code on this mailing list? What other way is there for this code to reach the users? Start your own project that's focused on satisfying these users. Start a new project so I can include a patch that hasn't made it yet into the what's cooking in one week? That's ridiculous. It doesn't even need to be open source or have a community of reviewers, because all you care about are users. Who said *all* that matters are the users? And even if somebody did, ultimately a closed source proprietary software doesn't benefit the users, so either way it has to be open and active to benefit the users. -- Felipe Contreras -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 6:46 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Okay, one more segment needs to be responded to. Felipe Contreras wrote: If remote-hg wasn't available for users, they would be hurt; if stash wasn't available, if rebase --interactive didn't exist, if there was no msysgit, if it wasn't so fast, if the object model wasn't so simple and extensible; users would be hurt. And if users didn't have all these, there would be less users, and if there were less users, there would be less developers, and mercurial might have been more popular, and most repositories you have to work on would be in mercurial, and you might be developing mercurial right now. Flawed logic. A large number of users doesn't automatically imply good software with lots of features, or even a great development community. A great development community leads to great software. And great software leads to lots of users. Sure, there's a feedback loop pushing users to become developers; but it doesn't start with users of vaporware, leading to more developers joining the effort to turn that vaporware into a great product. Life doesn't begin with users. Nobody knows how life began, and it doesn't matter now, what matters is how life evolves. It doesn't matter if the chicken was first, or the egg, what matters is that if all the chickens and eggs are gone, there won't be more. Plenty of projects have died because they stopped caring about their users, and without users there's no new developers, and the old developers eventually move on, and all the literary quality of commit messages have no eyes to see it. I repeat: no project is more important than its users. -- Felipe Contreras -- 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/6] transport-helper: clarify *:* refspec
Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). [...] -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' So what's wrong with the tests? Do they fail to test what they claim (how?), test something that wasn't reasonable to begin with, or something entirely different? Look at the code comment, and look at the now updated documentation that assumes that *:* was reasonable. Given the available information, it would be reasonable to assume that *:* did work, but it didn't work, and it's not really possible to fix it, even if we wanted to, it would be a hack. It's better to accept that fact and stop worrying too much about what would be the best way to do the wrong thing. Ok, you say that the *failing* test set an expectation that is unrealistic, so let's drop it. But then what about the successful test? Does it actually work (and by removing the test, you are saying that we don't care if we subsequently break that (mis)feature)? Or did it test the wrong thing? -- Thomas Rast trast@{inf,student}.ethz.ch -- 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/6] transport-helper: clarify *:* refspec
On Thu, Apr 18, 2013 at 7:39 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote: Felipe Contreras felipe.contre...@gmail.com writes: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). [...] -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' - -test_expect_failure 'pushing with straight refspec' ' - test_when_finished (cd local2 git reset --hard origin) - (cd local2 - echo content file - git commit -a -m eleven - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) - compare_refs local2 HEAD server HEAD -' So what's wrong with the tests? Do they fail to test what they claim (how?), test something that wasn't reasonable to begin with, or something entirely different? Look at the code comment, and look at the now updated documentation that assumes that *:* was reasonable. Given the available information, it would be reasonable to assume that *:* did work, but it didn't work, and it's not really possible to fix it, even if we wanted to, it would be a hack. It's better to accept that fact and stop worrying too much about what would be the best way to do the wrong thing. Ok, you say that the *failing* test set an expectation that is unrealistic, so let's drop it. But then what about the successful test? Does it actually work (and by removing the test, you are saying that we don't care if we subsequently break that (mis)feature)? Or did it test the wrong thing? Yeah, it works, in the sense that peeing in a bottle is a solution; it might work, but it's not recommendable. So, if suddenly working, frankly I don't care. I added those tests, and I don't think they are needed. In a not too distant future it should not be permitted to work; we don't want developers to shoot themselves in the foot, and heir users too. -- Felipe Contreras -- 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 with rev-list --reverse?
Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting pe...@softwolves.pp.se wrote: % git log --oneline -1 v1.8.1.5^..v1.8.1.6 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6 I expect to get a different output, and not both showing v1.8.1.6. Wouldn't you agree? Quoting the manual page: Commit Limiting Besides specifying a range of commits that should be listed using the special notations explained in the description, additional commit limiting may be applied. Note that they are applied before commit ordering and formatting options, such as --reverse. Given that, I would expect the output to be the same. If expectations were based on documentation, all one has to do is document bugs, and there would be no bugs anymore :) Code can be changed to fit more appropriately user expectations (which are independent of documentation), and the documentation updated accordingly. It's been this way forever, and applies to rev-list where we can't just break how options work (for fear of breaking scripts). You could come up with a patch series that first starts emitting warnings whenever the user asks for behavior that will change, and later flips the default and removes the warning (the latter would be merged for 2.0 or so). -- Thomas Rast trast@{inf,student}.ethz.ch -- 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 1/2] rev-parse: add --filename-prefix option
John Keeping wrote: This adds a prefix string to any filename arguments encountered after it has been specified. Very nice. I thought we'd have to resort to path mangling in shell to fix git-submodule.sh. Glad to see that we can go with something cleaner. Perhaps pull some bits from your nice Documentation into the commit message? diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index f267a1d..de894c7 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr) show(buffer); } -static int show_file(const char *arg) +static int show_file(const char *arg, int output_prefix) Okay, so you've essentially patched show_file() to accept an additional argument, and modified callers to call with this additional argument. I suppose show_(rev|reference|default|flag|rev|with_type|datestring|abbrev) don't need to be patched, as they are path-independent. { show_default(); if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) { - show(arg); + if (output_prefix) { + const char *prefix = startup_info-prefix; + show(prefix_filename(prefix, +prefix ? strlen(prefix) : 0, +arg)); + } else + show(arg); Uh, why do you need output_prefix? If startup_info-prefix is set, use it. Is startup_info-prefix set by anyone by cmd_rev_parse()? @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n int cmd_rev_parse(int argc, const char **argv, const char *prefix) @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) i++; continue; } + if (!strcmp(arg, --prefix)) { + prefix = argv[i+1]; + startup_info-prefix = prefix; + output_prefix = 1; + i++; + continue; + } Wait, why isn't prefix filled in when run_builtin() calls this? Oh, right: because we didn't mark this builtin with RUN_SETUP or RUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it would be a major problem (all our scripts would break) if rev-parse did cd-to-toplevel. Why are you setting prefix to argv[i+1], and then setting startup_info-prefix to that? Is anyone else in cmd_rev_parse() going to use it? +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) I'm wondering if you need such a convoluted usage though. Will you ever need to specify a prefix by hand that is different from what git rev-parse --show-toplevel returns? If not, why don't you just rev-parse --emulate-toplevel, and get rid of specifying prefix by hand altogether? Then again, this is a plumbing command, so the simplicity is probably more valuable. diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh new file mode 100755 index 000..5ef48d2 --- /dev/null +++ b/t/t1513-rev-parse-prefix.sh +test_expect_success 'empty prefix -- file' ' + git rev-parse --prefix -- top sub1/file1 actual + cat -EOF expected Nit: when you're not putting in variables, you can cat -\EOF. +test_expect_success 'empty prefix HEAD:./path' ' + git rev-parse --prefix HEAD:./top actual + git rev-parse HEAD:top expected Nit: why did you change ./top to top? Your --prefix option doesn't require you to change your arguments accordingly, does it? -- 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 1/2] rev-parse: add --filename-prefix option
On Thu, Apr 18, 2013 at 07:58:25PM +0530, Ramkumar Ramachandra wrote: John Keeping wrote: This adds a prefix string to any filename arguments encountered after it has been specified. Very nice. I thought we'd have to resort to path mangling in shell to fix git-submodule.sh. Glad to see that we can go with something cleaner. Perhaps pull some bits from your nice Documentation into the commit message? Good idea. I intended to re-write the commit message for v2 since the patch was completely re-written but forgot by the time I'd sorted out patch 2 as well. I will do for v3. diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index f267a1d..de894c7 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr) show(buffer); } -static int show_file(const char *arg) +static int show_file(const char *arg, int output_prefix) Okay, so you've essentially patched show_file() to accept an additional argument, and modified callers to call with this additional argument. I suppose show_(rev|reference|default|flag|rev|with_type|datestring|abbrev) don't need to be patched, as they are path-independent. { show_default(); if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) { - show(arg); + if (output_prefix) { + const char *prefix = startup_info-prefix; + show(prefix_filename(prefix, +prefix ? strlen(prefix) : 0, +arg)); + } else + show(arg); Uh, why do you need output_prefix? If startup_info-prefix is set, use it. Is startup_info-prefix set by anyone by cmd_rev_parse()? output_prefix is a flag to say do we want to show the prefix. We need it because show_file is used for the -- argument separator as well as file paths. Without a separate flag we end up prefixing -- with the prefix path. @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n int cmd_rev_parse(int argc, const char **argv, const char *prefix) @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) i++; continue; } + if (!strcmp(arg, --prefix)) { + prefix = argv[i+1]; + startup_info-prefix = prefix; + output_prefix = 1; + i++; + continue; + } Wait, why isn't prefix filled in when run_builtin() calls this? Oh, right: because we didn't mark this builtin with RUN_SETUP or RUN_SETUP_GENTLY. Okay, now why didn't we change that? Because it would be a major problem (all our scripts would break) if rev-parse did cd-to-toplevel. prefix is already set, by setup_git_git_directory. The point is that we just change the values set in setup_git_directory so that the command behaves as if it were run from a subdirectory. Why are you setting prefix to argv[i+1], and then setting startup_info-prefix to that? Is anyone else in cmd_rev_parse() going to use it? +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) I'm wondering if you need such a convoluted usage though. Will you ever need to specify a prefix by hand that is different from what git rev-parse --show-toplevel returns? If not, why don't you just rev-parse --emulate-toplevel, and get rid of specifying prefix by hand altogether? Then again, this is a plumbing command, so the simplicity is probably more valuable. How does that work? When we run rev-parse with the --prefix argument we're no longer in the subdirectory. While this may look convoluted here, I don't think it is in normal usage inside a script. If you look at the way it's used in patch 2 we're careful not to just remap all the arguments but to extract the flags before remapping file paths when we know that everything we have is a file path. diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh new file mode 100755 index 000..5ef48d2 --- /dev/null +++ b/t/t1513-rev-parse-prefix.sh +test_expect_success 'empty prefix -- file' ' + git rev-parse --prefix -- top sub1/file1 actual + cat -EOF expected Nit: when you're not putting in variables, you can cat -\EOF. +test_expect_success 'empty prefix HEAD:./path' ' + git rev-parse --prefix HEAD:./top actual + git rev-parse HEAD:top expected Nit: why did you change ./top to top? Your --prefix option doesn't require you to change your arguments accordingly, does it? The
Re: [PATCH v2 2/2] submodule: drop the top-level requirement
On Thu, Apr 18, 2013 at 08:16:42PM +0530, Ramkumar Ramachandra wrote: John Keeping wrote: Use the new rev-parse --prefix option to process all paths given to the submodule command, dropping the requirement that it be run from the top-level of the repository. Yay! diff --git a/git-submodule.sh b/git-submodule.sh index 79bfaac..bbf7983 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -112,6 +115,7 @@ resolve_relative_url () # module_list() { + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) Nit: Why not use -- to disambiguate between options and arguments, like you showed in your intended usage? Erm... it does. What's before $@? Ah, you mean after set. In this case, rev-parse will keep the -- we supply to it, so we get that from there and do not want to give set an extra one. So, this essentially converts all the paths specified in $@ to toplevel-relative paths. Beautiful, as this is practically the only change you require in each function. + sm_path=$wt_prefix$sm_path Wait, isn't sm_path the value of submodule.name.path in .gitmodules? Why does it need to be prefixed? I think you've lost too much context here. This is in cmd_add and at this point sm_path holds the argument supplied by the user, which we must adjust as it will be relative to the current directory. We should probably be testing for an absolute path here though. @@ -942,6 +948,7 @@ cmd_summary() { fi cd_to_toplevel + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) Um, what about other functions? Yeah, it looks like all of them except this one call module_list(). Exactly. Apart from this and cmd_add everything uses module_list. diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index ff26535..7795f21 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' ' test_cmp empty untracked ' +test_expect_success 'submodule add in subdir' ' + echo refs/heads/master expect + empty Nit: Use : empty. It's clearer. + ( + mkdir addtest/sub Why is the mkdir inside the subshell? The next statement (cd) onwards should be in the subshell, to save you cd'ing back. + cd addtest/sub + git submodule add $submodurl ../realsubmod3 + git submodule init + ) + + rm -f heads head untracked Huh? What is this in the middle? The next statement (call to inspect) write-truncates them anyway, so this is unnecessary. I just followed the surrounding tests here. I think it's better for follow the pattern of the surrounding code here than get this one test perfect. A cleanup can follow on top if someone wants to do it. -- 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 p4 submit failing
The issue is caused by the line endings. I retested the problem with a different file and in this case, the error is caused by the line endings of the file checked out in the perforce workspace being win-style crlf, and the line endings of the file in the git repo being unix style lf. (In this scenario, I took out the .gitattributes, core.autocrlf was set to false and LineEnd was set to share) In this case, I checked out the file in perforce, ran dos2unix against it, and submitted that, then ran git p4 submit and it worked. I noticed that the error is caused by the git apply failing in the def applyCommit(self, id) function at lines 1296-1305. diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id) patchcmd = diffcmd + | git apply tryPatchCmd = patchcmd + --check - applyPatchCmd = patchcmd + --check --apply - patch_succeeded = True if os.system(tryPatchCmd) != 0: fixed_rcs_keywords = False patch_succeeded = False print Unfortunately applying the change failed! So most likely in git apply command, it can't find the changes because of the line endings being different between them. I couldn't find a parameter that would magically make it work. When I added --verbose to git apply the output only says: error: while searching for: and then the first lines of the first diff Hello Simon, I have CCed you to alert you to the possible bug. Any assistance would be appreciated. On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon christopher.yee...@gmail.com wrote: Yes this is the case. Many of the files have crlf endings. core.autocrlf was recently set to input. I can't remember the timeline exactly though, but in addition to this, I have a .gitattributes file with the default set to text=auto (* text=auto) and the php files set to text eol=lf (*.php text eol=lf) Also my perforce workspace's LineEnd setting is set to Share. I've experienced the behavior in both .php and .xml files though Before all of this started I had core.autocrlf set to false, and no .gitattributes file and perforce workspace's LineEnd was set to the default, but I got a conflict where the only difference was the line endings, so I changed things to the way they are now. Any recommendations? Should I change everything back the way it was? On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff p...@padd.com wrote: l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100: Just a thought, but check the files that are failing to see if they've got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all sorts of nasty problems. That's assuming it's definitely not a CRLF line ending problem on Windows. I had recently debugged a similar-looking problem where core.autocrlf was set to input. Christopher, if you have this set and/or the .xml files have ^M (CRLF) line endings, please let us know. -- Pete On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon christopher.yee...@gmail.com wrote: I tried running git p4 submit on a repo that I've been running as an interim bridge between git and perforce. Multiple people are using the repo as a remote and its being periodically submitted back to perforce. It's been working mostly fine. Then one day out of the blue I get this error. I can no longer push any git commits to perforce. (This is from the remote repo which I am pushing back to perforce) user@hostname:~/Source/code$ git p4 submit -M --export-labels Perforce checkout for depot path //depot/perforce/workspace/ located at /home/user/Source/git-p4-area/perforce/workspace/ Synchronizing p4 checkout... ... - file(s) up-to-date. Applying ffa390f comments in config xml files //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for edit error: patch failed: sub/folder/structure/first.xml:1 error: sub/folder/structure/first.xml: patch does not apply error: patch failed: sub/folder/structure/second.xml:1 error: sub/folder/structure/second.xml: patch does not apply error: patch failed: sub/folder/structure/third.xml:1 error: sub/folder/structure/third.xml: patch does not apply error: patch failed: sub/folder/structure/forth.xml:1 error: sub/folder/structure/forth.xml: patch does not apply error: patch failed: sub/folder/structure/fifth.xml:1 error: sub/folder/structure/fifth.xml: patch does not apply Unfortunately applying the change failed! //depot/perforce/workspace/sub/folder/structure/first.xml#1 - was edit, reverted //depot/perforce/workspace/sub/folder/structure/second.xml#3 - was edit, reverted
Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec
Ramkumar Ramachandra artag...@gmail.com writes: 'git rebase' does not recognize revisions specified as :/text. This is because the attempts to rev-parse ${REV}^0, which fails in this case. Add a test to document this failure. - The failure occurs in git-rebase.sh:403. Is it using the ^0 only to make sure that the revision specified is a commit? Surely, there'a a better way to do this? Can someone point me in the right direction? How about ${REV}^0 into nREV=$(git rev-parse ${REV})^0 and use it where you need an object name that needs to be parsed by get_sha1(), e.g. git checkout -q $nREV^0 I would suggest a helper function in git-sh-setup, something like: peel_committish () { case $1 in :/*) peeltmp=$(git rev-parse --verify $1) git rev-parse --verify $peeltmp^0 ;; *) git rev-parse --verify $1^0 ;; esac } -- 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] blame: handle broken commit headers gracefully
On Wed, Apr 17, 2013 at 02:55:29PM -0700, Junio C Hamano wrote: Or you can imagine nastier input strings, like Name -email@host 123456789 - Name ema-il@host 123456789 - Name email@host~ 123456789 - I am afraid that at some point we should salvage as much as we can, which is a worthy goal, becomes a losing proposition. Good point. In the worst cases, even if you cleaned things up, you might even need to allocate a new string (like your middle one), which would make calling split_ident_line a lot more annoying. Probably not worth the effort. -Peff -- 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: t6200: avoid path mangling issue on Windows
Johannes Sixt j.s...@viscovery.net writes: From: Johannes Sixt j...@kdbg.org MSYS bash interprets the slash in the argument core.commentchar=/ as root directory and mangles it into a Windows style path. Use a different core.commentchar to dodge the issue. Signed-off-by: Johannes Sixt j...@kdbg.org ... - git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD actual + git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD actual Sigh... Again? Are folks working on Msys bash aware that sometimes the users may want to say key=value on their command line without the value getting molested in any way and giving them some escape hatch would help them? Perhaps they have already decided that it is not feasible after thinking about the issue, in which case I do not have new ideas to offer. I'll apply the patch as-is, but this feels really painful to the users. Thanks. -- 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
[PATCHv2] l10n: de.po: translate 39 new messages
Translate 39 new messages came from git.pot update in c138af5 (l10n: git.pot: v1.8.3 round 1 (54 new, 15 removed)). While at there, fix some small issues. Signed-off-by: Ralf Thielow ralf.thie...@gmail.com Acked-by: Thomas Rast tr...@inf.ethz.ch --- po/de.po | 206 +-- 1 file changed, 108 insertions(+), 98 deletions(-) diff --git a/po/de.po b/po/de.po index 2cd3fa9..88d7919 100644 --- a/po/de.po +++ b/po/de.po @@ -136,12 +136,13 @@ msgstr #: branch.c:201 #, c-format msgid Cannot setup tracking information; starting point '%s' is not a branch. -msgstr +msgstr Kann Informationen zum Übernahmezweig nicht einrichten; +Startpunkt '%s' ist kein Zweig. #: branch.c:203 -#, fuzzy, c-format +#, c-format msgid the requested upstream branch '%s' does not exist -msgstr Zweig '%s' existiert nicht +msgstr der angeforderte externe Übernahmezweig '%s' existiert nicht #: branch.c:205 msgid @@ -154,6 +155,15 @@ msgid will track its remote counterpart, you may want to use\n \git push -u\ to set the upstream config as you push. msgstr +\n +Falls Sie vorhaben, Ihre Arbeit auf einem bereits existierenden,\n +externen Übernahmezweig aufzubauen, sollten Sie \git fetch\\n +ausführen, um diesen abzurufen.\n +\n +Falls Sie vorhaben, einen neuen lokalen Zweig zu versenden\n +der seinem externen Gegenstück folgen soll, können Sie\n +\git push -u\ verwenden, um den externen Übernahmezweig\n +beim Versand zu konfigurieren. #: bundle.c:36 #, c-format @@ -181,22 +191,22 @@ msgid revision walk setup failed msgstr Einrichtung des Revisionsgangs fehlgeschlagen #: bundle.c:186 -#, fuzzy, c-format +#, c-format msgid The bundle contains this ref: msgid_plural The bundle contains these %d refs: -msgstr[0] Das Paket enthält %d Referenz -msgstr[1] Das Paket enthält %d Referenzen +msgstr[0] Das Paket enthält diese Referenz: +msgstr[1] Das Paket enthält diese %d Referenzen: #: bundle.c:193 msgid The bundle records a complete history. msgstr Das Paket speichert eine komplette Historie. #: bundle.c:195 -#, fuzzy, c-format +#, c-format msgid The bundle requires this ref: msgid_plural The bundle requires these %d refs: -msgstr[0] Das Paket benötigt diese Referenz -msgstr[1] Das Paket benötigt diese %d Referenzen +msgstr[0] Das Paket benötigt diese Referenz: +msgstr[1] Das Paket benötigt diese %d Referenzen: #: bundle.c:294 msgid rev-list died @@ -731,7 +741,7 @@ msgid Unable to write index. msgstr Konnte Bereitstellung nicht schreiben. #: object.c:195 -#, fuzzy, c-format +#, c-format msgid unable to parse object: %s msgstr Konnte Objekt '%s' nicht parsen. @@ -933,7 +943,7 @@ msgstr Kann keine Versionsbeschreibung für %s bekommen #: sequencer.c:621 #, c-format msgid could not revert %s... %s -msgstr Konnte %s nicht zurücksetzen... %s +msgstr Konnte %s nicht zurücknehmen... %s #: sequencer.c:622 #, c-format @@ -1056,11 +1066,11 @@ msgstr Konnte %s nicht formatieren. #: sequencer.c:1101 msgid Can't revert as initial commit -msgstr Kann nicht zu initialer Version zurücksetzen. +msgstr Rücknahme-Version kann nicht initial sein. #: sequencer.c:1102 msgid Can't cherry-pick into empty head -msgstr Kann \cherry-pick\ nicht in einem leerem Kopf ausführen. +msgstr Kann \cherry-pick\ nicht in einem leeren Zweig ausführen. #: sha1_name.c:1036 msgid HEAD does not point to a branch @@ -1376,33 +1386,29 @@ msgid (all conflicts fixed: run \git commit\) msgstr (alle Konflikte behoben: führen Sie \git commit\ aus) #: wt-status.c:972 -#, fuzzy, c-format +#, c-format msgid You are currently reverting commit %s. -msgstr Sie sind gerade bei einer binären Suche in Zweig '%s'. +msgstr Sie nehmen gerade Version '%s' zurück. #: wt-status.c:977 -#, fuzzy msgid (fix conflicts and run \git revert --continue\) msgstr - (beheben Sie die Konflikte und führen Sie dann \git rebase --continue\ + (beheben Sie die Konflikte und führen Sie dann \git revert --continue\ aus) #: wt-status.c:980 -#, fuzzy msgid (all conflicts fixed: run \git revert --continue\) -msgstr (alle Konflikte behoben: führen Sie \git rebase --continue\ aus) +msgstr (alle Konflikte behoben: führen Sie \git revert --continue\ aus) #: wt-status.c:982 -#, fuzzy msgid (use \git revert --abort\ to cancel the revert operation) msgstr - (benutzen Sie \git rebase --abort\ um den ursprünglichen Zweig -auszuchecken) + (benutzen Sie \git revert --abort\ um die Umkehroperation abzubrechen) #: wt-status.c:993 -#, fuzzy, c-format +#, c-format msgid You are currently bisecting, started from branch '%s'. -msgstr Sie sind gerade bei einer binären Suche in Zweig '%s'. +msgstr Sie sind gerade bei einer binären Suche, gestartet von Zweig '%s'. #: wt-status.c:997 msgid You are currently bisecting. @@ -1419,13 +1425,12 @@ msgid On branch msgstr Auf Zweig #: wt-status.c:1186 -#, fuzzy msgid HEAD detached at -msgstr losgelöste Zweigspitze (HEAD) +msgstr
Re: Bug with rev-list --reverse?
Thomas Rast tr...@inf.ethz.ch writes: You could come up with a patch series that first starts emitting warnings whenever the user asks for behavior that will change, and later flips the default and removes the warning (the latter would be merged for 2.0 or so). Please don't. The fact that reverse then count mode may be useful does not mean count then show in reverse mode does not have any use. git log --oneline --reverse -20 is a very valid way to ask what did I do recently and get you did this, that and then... in that order. Adding a new option can be done anytime without any complex transition plan. You may want to introduce a --show-in-reverse synonym to the current --reverse when you add the new mode of reversing (--reverse-before-count?) so that eventually we won't have to ask which kind of reverse an unadorned --reverse option mean? by deprecating a plain --reverse, though. -- 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/13] utf8.c: add reencode_string_len() that can handle NULs in string
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: diff --git a/utf8.h b/utf8.h index d3da96f..a43ef9a 100644 --- a/utf8.h +++ b/utf8.h @@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len, int indent, int indent2, int width); #ifndef NO_ICONV -char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv); -char *reencode_string(const char *in, const char *out_encoding, const char *in_encoding); +char *reencode_string_iconv(const char *in, size_t insz, + iconv_t conv, int *outsz); +char *reencode_string_len(const char *in, int insz, + const char *out_encoding, + const char *in_encoding, + int *outsz); #else -#define reencode_string(a,b,c) NULL +#define reencode_string_len(a,b,c,d,e) NULL #endif +static inline char *reencode_string(const char *in, + const char *out_encoding, + const char *in_encoding) +{ + return reencode_string_len(in, strlen(in), +out_encoding, in_encoding, +NULL); +} + int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding); #endif Nicely done. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 06:39:06PM -0700, Junio C Hamano wrote: Subject: [PATCH] git add: rework the logic to warn git add pathspec... default change [...] Rework the logic to detect the case where the behaviour will be different in Git 2.0, and issue a warning only when it matters. Even with the code before this warning, git add subdir will have to traverse the directory in order to find _new_ files the index does not know about _anyway_, so we can do this check without adding an extra pass to find if pathspec matches any removed file. Thanks, I think this is looking much better. A few minor nits on the message itself: +static void warn_add_would_remove(const char *path) +{ + warning(_(In Git 2.0, 'git add pathspec...' will also update the\n + index for paths removed from the working tree that match\n + the given pathspec. If you want to 'add' only changed\n + or newly created paths, say 'git add --no-all pathspec...' +instead.\n\n This wrapping looks funny in the actual output, due to the extra warning: and the lack of newline before instead: warning: In Git 2.0, 'git add pathspec...' will also update the index for paths removed from the working tree that match the given pathspec. If you want to 'add' only changed or newly created paths, say 'git add --no-all pathspec...' instead. Wrapping it like this ends up with a much more natural-looking paragraph: warning: In Git 2.0, 'git add pathspec...' will also update the index for paths removed from the working tree that match the given pathspec. If you want to 'add' only changed or newly created paths, say 'git add --no-all pathspec...' instead. + '%s' would be removed from the index without --no-all.), + path); I think mentioning the filename is a good thing; the original message left me scratching my head and wondering so, did you add it or not?. I still think your would be is unnecessarily confusing, though. It is would be in Git 2.0 without --no-all, but we did not now. Which makes sense when you think about it, but it took me a moment to parse. Perhaps we can be more direct with something like: warning: did not stage removal of 'foo' or perhaps the present tense not staging removal of... would be better. I also think it makes sense to show every path that is affected, not just the first one, to be clear about what was done (and what _would_ have been done in Git 2.0). A patch with all of the suggestions together is below. I still think the multi-line warning block looks ugly. I kind of like the way advise() puts hint: on each line. I wonder if we should do the same here. diff --git a/builtin/add.c b/builtin/add.c index 4242bce..aae550a 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -52,15 +52,19 @@ static void warn_add_would_remove(const char *path) return DIFF_STATUS_MODIFIED; } +static const char *add_would_remove_warning = N_( +/* indent for warning: */ + In Git 2.0, 'git add pathspec...' will also update the\n +index for paths removed from the working tree that match the given\n +pathspec. If you want to 'add' only changed or newly created paths,\n +say 'git add --no-all pathspec...' instead.\n); + static void warn_add_would_remove(const char *path) { - warning(_(In Git 2.0, 'git add pathspec...' will also update the\n - index for paths removed from the working tree that match\n - the given pathspec. If you want to 'add' only changed\n - or newly created paths, say 'git add --no-all pathspec...' - instead.\n\n - '%s' would be removed from the index without --no-all.), - path); + static int warned_once; + if (!warned_once++) + warning(_(add_would_remove_warning)); + warning(did not stage removal of '%s', path); } static void update_callback(struct diff_queue_struct *q, @@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q, } break; case DIFF_STATUS_DELETED: - if (data-warn_add_would_remove) { + if (data-warn_add_would_remove) warn_add_would_remove(path); - data-warn_add_would_remove = 0; - } if (data-flags ADD_CACHE_IGNORE_REMOVAL) break; if (!(data-flags ADD_CACHE_PRETEND)) -- 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 1/6] transport-helper: clarify *:* refspec
Felipe Contreras felipe.contre...@gmail.com writes: The *:* refspec doesn't work, and never has, clarify the code and documentation to reflect that. This in effect reverts commit 9e7673e (gitremote-helpers(1): clarify refspec behaviour). ... applicable refspec takes precedence. The left-hand of refspecs advertised with this capability must cover all refs reported by -the list command. If a helper does not need a specific 'refspec' -capability then it should advertise `refspec *:*`. +the list command. If no 'refspec' capability is advertised, +there is an implied `refspec *:*`. I presume s/.$/, but `*:*` does not work so do not use it./ is implied? diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index f387027..cd1873c 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_success 'pulling with straight refspec' ' - (cd local2 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) - compare_refs local2 HEAD server HEAD -' This one made me raise my eyebrows, first. The only reason this test passes is because this particular pull, due to what local2 and server happens to have before this test runs, is an Already up-to-date and a no-op. You are removing this because it is not really testing anything useful, and if you change it to test something real, e.g. by rewinding local2, it will reveal the breakage. Am I reading you correctly? diff --git a/transport-helper.c b/transport-helper.c index dcd8d97..cea787c 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport, * were fetching. * * (If no refspec capability was specified, for historical - * reasons we default to *:*.) + * reasons we default to the equivalent of *:*.) * * Store the result in to_fetch[i].old_sha1. Callers such * as git fetch can use the value to write feedback to the -- 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 3/6] transport-helper: clarify pushing without refspecs
Felipe Contreras felipe.contre...@gmail.com writes: This has never worked, since it's inception the code simply skips all the refs, essentially telling fast-export to do nothing. Good description. Let's at least tell the user what's going on. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Documentation/gitremote-helpers.txt | 4 ++-- t/t5801-remote-helpers.sh | 6 +++--- transport-helper.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt index ba7240c..4d26e37 100644 --- a/Documentation/gitremote-helpers.txt +++ b/Documentation/gitremote-helpers.txt @@ -162,8 +162,8 @@ Miscellaneous capabilities For remote helpers that implement 'import' or 'export', this capability allows the refs to be constrained to a private namespace, instead of writing to refs/heads or refs/remotes directly. - It is recommended that all importers providing the 'import' or 'export' - capabilities use this. + It is recommended that all importers providing the 'import' + capability use this. It's mandatory for 'export'. As [1/6] said *:* does not work and has never worked, and especially on the push side the patch below makes it clear it was a glorified no-op, it may be better to say a bit more than use this. It's mandatory. It is not like any value makes sense, right? Perhaps the documentation will be further updated in a later step in the series to clarify it. Let's read on til the end of the series... diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index cd1873c..3eeb309 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' ' compare_refs local2 HEAD server HEAD ' -test_expect_failure 'pushing without refspecs' ' +test_expect_success 'pushing without refspecs' ' test_when_finished (cd local2 git reset --hard origin) (cd local2 echo content file git commit -a -m ten - GIT_REMOTE_TESTGIT_REFSPEC= git push) - compare_refs local2 HEAD server HEAD + GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error) + grep remote-helper doesn.t support push; refspec needed error ' I somehow like this change that turns a _failure into a _success by expecting test_must_fail, especially because it is clear why the tested push should fail when you look at the rest of the patch ;-) Fun. diff --git a/transport-helper.c b/transport-helper.c index cea787c..4d98567 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data-refspecs) + die(remote-helper doesn't support push; refspec needed); + helper = get_helper(transport); write_constant(helper-in, export\n); @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport *transport, char *private; unsigned char sha1[20]; - if (!data-refspecs) - continue; private = apply_refspecs(data-refspecs, data-refspec_nr, ref-name); if (private !get_sha1(private, sha1)) { strbuf_addf(buf, ^%s, private); -- 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 3/6] transport-helper: clarify pushing without refspecs
John Keeping j...@keeping.me.uk writes: +It is recommended that all importers providing the 'import' +capability use this. It's mandatory for 'export'. s/It's/It is/ I personally do not care _too_ deeply either way, but I agree our documentation tends to use the latter more and being consistent would be good. diff --git a/transport-helper.c b/transport-helper.c index cea787c..4d98567 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; +if (!data-refspecs) +die(remote-helper doesn't support push; refspec needed); I think the refspec needed text is likely to be confusing if an end-user ever sees this message. I'm not sure how we can provide useful feedback for both remote helper authors and end-users though. This refspecs only come via the helper and not directly from the end user, no? If that is the case, I do not think confusing is much of an issue. Not _(localizing) is also the right thing to do. We may want to say BUG: at front to clarify it is not the end-user's fault, but a problem they may want to report. If we at this point know what helper attempted export without giving refspecs, it may help to show it here, so that developers will know with what helper the user had problems with. -- 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 4/6] transport-helper: warn when refspec is not used
Felipe Contreras felipe.contre...@gmail.com writes: For the modes that need it. In the future we should probably error out, instead of providing half-assed support. The reason we want to do this is because if it's not present, the remote helper might be updating refs/heads/*, or refs/remotes/origin/*, directly, and in the process fetch will get confused trying to update refs that are already updated, or older than what they should be. We shouldn't be messing with the rest of git. So that answers my question in the response to an earlier one in this series. We expect the ref updates to be done by the fetch or push that drives the helper, and do not want the helper to interfere with its ref updates. So it is not just 'refspec' _allows_ the refs to be constrained to a private namespace, like the earlier updates made the documentation say; it _is_ mandatory to use refspecs to constrain them to avoid touching refs/heads and refs/remotes namespace. Am I reading you correctly? Assuming I am, the patch in this message looks reasonable. It makes the documentation updates a few patches ago look a bit wanting, though. Thanks. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- t/t5801-remote-helpers.sh | 6 -- transport-helper.c| 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh index 3eeb309..1bb7529 100755 --- a/t/t5801-remote-helpers.sh +++ b/t/t5801-remote-helpers.sh @@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new refspec' ' test_expect_success 'cloning without refspec' ' GIT_REMOTE_TESTGIT_REFSPEC= \ - git clone testgit::${PWD}/server local2 + git clone testgit::${PWD}/server local2 2 error + grep This remote helper should implement refspec capability error compare_refs local2 HEAD server HEAD ' test_expect_success 'pulling without refspecs' ' (cd local2 git reset --hard - GIT_REMOTE_TESTGIT_REFSPEC= git pull) + GIT_REMOTE_TESTGIT_REFSPEC= git pull 2 ../error) + grep This remote helper should implement refspec capability error compare_refs local2 HEAD server HEAD ' diff --git a/transport-helper.c b/transport-helper.c index 4d98567..573eaf7 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport *transport) free((char *)refspecs[i]); } free(refspecs); + } else if (data-import || data-bidi_import || data-export) { + warning(This remote helper should implement refspec capability.); } strbuf_release(buf); if (debug) -- 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] remote-hg: fix commit messages
Felipe Contreras felipe.contre...@gmail.com writes: git fast-import expects an extra newline after the commit message data, but we are adding it only on hg-git compat mode, which is why the bidirectionality tests pass. We should add it unconditionally. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- Without knowing that hg-git compat mode is what is used in bidi test (the only mode that supports bidi), which is why was ungrokkable. This is a trivial change without downside risk so I do not mind applying it to 'maint', as you say it is an appropriate there. contrib/remote-helpers/git-remote-hg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg index a5f0013..5481331 100755 --- a/contrib/remote-helpers/git-remote-hg +++ b/contrib/remote-helpers/git-remote-hg @@ -362,6 +362,8 @@ def export_ref(repo, name, kind, head): else: modified, removed = get_filechanges(repo, c, parents[0]) +desc += '\n' + if mode == 'hg': extra_msg = '' @@ -385,7 +387,6 @@ def export_ref(repo, name, kind, head): else: extra_msg += extra : %s : %s\n % (key, urllib.quote(value)) -desc += '\n' if extra_msg: desc += '\n--HG--\n' + extra_msg -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King p...@peff.net writes: +static const char *add_would_remove_warning = N_( +/* indent for warning: */ + In Git 2.0, 'git add pathspec...' will also update the\n +index for paths removed from the working tree that match the given\n +pathspec. If you want to 'add' only changed or newly created paths,\n +say 'git add --no-all pathspec...' instead.\n); + static void warn_add_would_remove(const char *path) { - warning(_(In Git 2.0, 'git add pathspec...' will also update the\n - index for paths removed from the working tree that match\n - the given pathspec. If you want to 'add' only changed\n - or newly created paths, say 'git add --no-all pathspec...' -instead.\n\n - '%s' would be removed from the index without --no-all.), - path); + static int warned_once; + if (!warned_once++) + warning(_(add_would_remove_warning)); + warning(did not stage removal of '%s', path); } Would add --dry-run say this, too? static void update_callback(struct diff_queue_struct *q, @@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q, } break; case DIFF_STATUS_DELETED: - if (data-warn_add_would_remove) { + if (data-warn_add_would_remove) warn_add_would_remove(path); - data-warn_add_would_remove = 0; - } if (data-flags ADD_CACHE_IGNORE_REMOVAL) break; if (!(data-flags ADD_CACHE_PRETEND)) -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 10:51:12AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: +static const char *add_would_remove_warning = N_( +/* indent for warning: */ + In Git 2.0, 'git add pathspec...' will also update the\n +index for paths removed from the working tree that match the given\n +pathspec. If you want to 'add' only changed or newly created paths,\n +say 'git add --no-all pathspec...' instead.\n); + static void warn_add_would_remove(const char *path) { - warning(_(In Git 2.0, 'git add pathspec...' will also update the\n - index for paths removed from the working tree that match\n - the given pathspec. If you want to 'add' only changed\n - or newly created paths, say 'git add --no-all pathspec...' - instead.\n\n - '%s' would be removed from the index without --no-all.), - path); + static int warned_once; + if (!warned_once++) + warning(_(add_would_remove_warning)); + warning(did not stage removal of '%s', path); } Would add --dry-run say this, too? It probably makes sense to continue to have the warning in the dry-run case, but it may make sense to tweak it grammatically when we are in dry-run mode. Saying would stage removal is technically correct, but I think it is somewhat ambiguous: would git do it if we were not in a --dry-run, or would git do it if it were Git 2.0? Doing it as: warning: not staging removal of '%s' could work for both cases. Something like not considering (or another synonym for considering) might be even more accurate. It is not just that we did not stage it; it is what we did not even consider it an item for staging under the current rules. Note that the not staging warnings may potentially be interspersed with the normal dry-run output. I think that's OK. But another alternative would be to collect the paths and then print: warning: In Git 2.0, ... The following deleted paths were not considered under the current rule. Use git add -A to stage their removal now. foo bar -Peff -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King p...@peff.net writes: could work for both cases. Something like not considering (or another synonym for considering) might be even more accurate. It is not just that we did not stage it; it is what we did not even consider it an item for staging under the current rules. Yes, not considering is much more sensible, while side-stepping the dryrun issue. Or warning(ignoring removal of '%s') Note that the not staging warnings may potentially be interspersed with the normal dry-run output. I think that's OK. As long as the top-text makes it clear what not considering (or ignoring) in the following text means, I think it is fine. But I think we are doing users a disservice by listing tons of paths. Where the difference of versions matters _most_ is when the user has tons of removed paths in the working tree. Either with one warning per path, or a block of collected paths at the end, we are scrolling the more important part of the message up. That was why I originally showed one path as an example and stopped there. Perhaps it is a better solution to keep that behaviour and rephrase the message to say that we ignored removal of paths like this one '%s' you lost from the working tree but it will change in Git 2.0 and you will better learn to use the --no-all option now. The inter-topic conflicts between stages of three add in Git 2.0 topics is getting cumbersome even with the help from rerere, so I'd like to merge their preparatory steps as I have them now to 'next' and merge them down to 'master' first, and start applying tweaks from there, or something. -- 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 3/6] transport-helper: clarify pushing without refspecs
On Thu, Apr 18, 2013 at 10:29:30AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: diff --git a/transport-helper.c b/transport-helper.c index cea787c..4d98567 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport *transport, struct string_list revlist_args = STRING_LIST_INIT_NODUP; struct strbuf buf = STRBUF_INIT; + if (!data-refspecs) + die(remote-helper doesn't support push; refspec needed); I think the refspec needed text is likely to be confusing if an end-user ever sees this message. I'm not sure how we can provide useful feedback for both remote helper authors and end-users though. This refspecs only come via the helper and not directly from the end user, no? If that is the case, I do not think confusing is much of an issue. Not _(localizing) is also the right thing to do. We may want to say BUG: at front to clarify it is not the end-user's fault, but a problem they may want to report. If we at this point know what helper attempted export without giving refspecs, it may help to show it here, so that developers will know with what helper the user had problems with. I like this idea. Perhaps we should say Bug in remote helper; refspec needed with export, so that it clearly indicates to both end-users and remote helper authors that the error is in the remote helper. -- 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/6] transport-helper: clarify pushing without refspecs
On 04/18/2013 02:05 AM, Felipe Contreras wrote: This has never worked, since it's inception the code simply skips all s/it's/its/ (sorry for nitpicking) the refs, essentially telling fast-export to do nothing. Let's at least tell the user what's going on. [SNIP] Regards, Stefano -- 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 4/6] transport-helper: warn when refspec is not used
On Thu, Apr 18, 2013 at 12:30 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: For the modes that need it. In the future we should probably error out, instead of providing half-assed support. The reason we want to do this is because if it's not present, the remote helper might be updating refs/heads/*, or refs/remotes/origin/*, directly, and in the process fetch will get confused trying to update refs that are already updated, or older than what they should be. We shouldn't be messing with the rest of git. So that answers my question in the response to an earlier one in this series. We expect the ref updates to be done by the fetch or push that drives the helper, and do not want the helper to interfere with its ref updates. So it is not just 'refspec' _allows_ the refs to be constrained to a private namespace, like the earlier updates made the documentation say; it _is_ mandatory to use refspecs to constrain them to avoid touching refs/heads and refs/remotes namespace. Yeah, it was not stated that it was mandatory, but I think it's in everybody's best interest that it is. -- Felipe Contreras -- 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
Git over HTTPS with basic authentication
Why is git not working over HTTPS with basic authentication? I can clone just fine, but when I try to push, it tells me error: Cannot access URL https://..., return code 22 fatal: git-http-push failed I have googled for an hour now, all I find is people that have the same problem and the solution is always to use SSH. Sibbo -- 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 0/2] submodule: drop the top-level requirement
Thanks to Ram and Jens for the feedback on v2. I've addressed most of their comments in this version, but I've ignored a couple of Ram's nits about test cases where leaving it how it is makes the added tests consistent with those already in the same file. Since v2, the main improvement is that the output from git-submodule now contains paths relative to the directory in which the command is run, not the top-level of the working tree. John Keeping (2): rev-parse: add --prefix option submodule: drop the top-level requirement Documentation/git-rev-parse.txt | 16 ++ builtin/rev-parse.c | 24 +++-- git-submodule.sh| 113 ++-- t/t1513-rev-parse-prefix.sh | 96 ++ t/t7400-submodule-basic.sh | 27 ++ t/t7401-submodule-summary.sh| 14 + 6 files changed, 258 insertions(+), 32 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh -- 1.8.2.1.424.g1899e5e.dirty -- 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 1/2] rev-parse: add --prefix option
This makes 'git rev-parse' behave as if it were invoked from the specified subdirectory of a repository, with the difference that any file paths which it prints are prefixed with the full path from the top of the working tree. This is useful for shell scripts where we may want to cd to the top of the working tree but need to handle relative paths given by the user on the command line. Signed-off-by: John Keeping j...@keeping.me.uk --- Changes since v2: - Rewrite commit message - Add a new test for 'prefix ignored with HEAD:top' - Use '-\EOF' where appropriate in t1513 Documentation/git-rev-parse.txt | 16 +++ builtin/rev-parse.c | 24 --- t/t1513-rev-parse-prefix.sh | 96 + 3 files changed, 131 insertions(+), 5 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt index 1f9ed6c..0ab2b77 100644 --- a/Documentation/git-rev-parse.txt +++ b/Documentation/git-rev-parse.txt @@ -59,6 +59,22 @@ OPTIONS If there is no parameter given by the user, use `arg` instead. +--prefix arg:: + Behave as if 'git rev-parse' was invoked from the `arg` + subdirectory of the working tree. Any relative filenames are + resolved as if they are prefixed by `arg` and will be printed + in that form. ++ +This can be used to convert arguments to a command run in a subdirectory +so that they can still be used after moving to the top-level of the +repository. For example: ++ + +prefix=$(git rev-parse --show-prefix) +cd $(git rev-parse --show-toplevel) +eval set -- $(git rev-parse --sq --prefix $prefix $@) + + --verify:: Verify that exactly one parameter is provided, and that it can be turned into a raw 20-byte SHA-1 that can be used to diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index f267a1d..de894c7 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char *datestr) show(buffer); } -static int show_file(const char *arg) +static int show_file(const char *arg, int output_prefix) { show_default(); if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) { - show(arg); + if (output_prefix) { + const char *prefix = startup_info-prefix; + show(prefix_filename(prefix, +prefix ? strlen(prefix) : 0, +arg)); + } else + show(arg); return 1; } return 0; @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0; + int output_prefix = 0; unsigned char sha1[20]; const char *name = NULL; @@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) const char *arg = argv[i]; if (as_is) { - if (show_file(arg) as_is 2) + if (show_file(arg, output_prefix) as_is 2) verify_filename(prefix, arg, 0); continue; } @@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) as_is = 2; /* Pass on the -- if we show anything but files.. */ if (filter (DO_FLAGS | DO_REVS)) - show_file(arg); + show_file(arg, 0); continue; } if (!strcmp(arg, --default)) { @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) i++; continue; } + if (!strcmp(arg, --prefix)) { + prefix = argv[i+1]; + startup_info-prefix = prefix; + output_prefix = 1; + i++; + continue; + } if (!strcmp(arg, --revs-only)) { filter = ~DO_NOREV; continue; @@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (verify) die_no_single_rev(quiet); as_is = 1; - if (!show_file(arg)) + if (!show_file(arg, output_prefix)) continue;
[PATCH v3 2/2] submodule: drop the top-level requirement
Use the new rev-parse --prefix option to process all paths given to the submodule command, dropping the requirement that it be run from the top-level of the repository. Signed-off-by: John Keeping j...@keeping.me.uk --- Changes since v2: - Handle relative paths given to submodule add --reference=./... - Check for absolute path before prepending $wt_prefix in cmd_add - Export relative sm_path in cmd_foreach - Change displayed sm_path to be relative - Move a mkdir out of a subshell in t7400 I'm not sure if the relative_path function here is sufficient on Windows. We should be okay with the target argument since that comes from git-ls-files but I think we're in trouble if git rev-parse --show-prefix output the prefix with '\' instead of '/'. git-submodule.sh | 113 --- t/t7400-submodule-basic.sh | 27 +++ t/t7401-submodule-summary.sh | 14 ++ 3 files changed, 127 insertions(+), 27 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 79bfaac..0eee703 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -14,10 +14,13 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference re or: $dashless [--quiet] foreach [--recursive] command or: $dashless [--quiet] sync [--recursive] [--] [path...] OPTIONS_SPEC= +SUBDIRECTORY_OK=Yes . git-sh-setup . git-sh-i18n . git-parse-remote require_work_tree +wt_prefix=$(git rev-parse --show-prefix) +cd_to_toplevel command= branch= @@ -106,12 +109,48 @@ resolve_relative_url () echo ${is_relative:+${up_path}}${remoteurl#./} } +# Resolve a path to be relative to another path. This is intended for +# converting submodule paths when git-submodule is run in a subdirectory +# and only handles paths where the directory separator is '/'. +# +# The output is the first argument as a path relative to the second argument, +# which defaults to $wt_prefix if it is omitted. +relative_path () +{ + local target curdir result + target=$1 + curdir=${2-$wt_prefix} + curdir=${curdir%/} + result= + + while test -n $curdir + do + case $target in + $curdir/*) + target=${target#$curdir/} + break + ;; + esac + + result=${result}../ + if test $curdir = ${curdir%/*} + then + curdir= + else + curdir=${curdir%/*} + fi + done + + echo $result$target +} + # # Get submodule info for registered submodules # $@ = path to limit submodule list # module_list() { + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) ( git ls-files --error-unmatch --stage -- $@ || echo unmatched pathspec exists @@ -282,6 +321,7 @@ isnumber() cmd_add() { # parse $args after submodule ... add. + reference_path= while test $# -ne 0 do case $1 in @@ -298,11 +338,11 @@ cmd_add() ;; --reference) case $2 in '') usage ;; esac - reference=--reference=$2 + reference_path=$2 shift ;; --reference=*) - reference=$1 + reference_path=${1#--reference=} ;; --name) case $2 in '') usage ;; esac @@ -323,6 +363,14 @@ cmd_add() shift done + if test -n $reference_path + then + is_absolute_path $reference_path || + reference_path=$wt_prefix$reference_path + + reference=--reference=$reference_path + fi + repo=$1 sm_path=$2 @@ -335,6 +383,8 @@ cmd_add() usage fi + is_absolute_path $sm_path || sm_path=$wt_prefix$sm_path + # assure repo is absolute or relative to parent case $repo in ./*|../*) @@ -479,6 +529,7 @@ cmd_foreach() # we make $path available to scripts ... path=$sm_path cd $sm_path + sm_path=$(relative_path $sm_path) eval $@ if test -n $recursive then @@ -594,27 +645,29 @@ cmd_deinit() die_if_unmatched $mode name=$(module_name $sm_path) || exit + displaypath=$(relative_path $sm_path) + # Remove the submodule work tree (unless the user already did it) if test -d $sm_path then # Protect submodules containing a .git directory if test
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Apr 16, 2013 at 5:45 PM, Phil Hord phil.h...@gmail.com wrote: On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras If you want to waste your time, by all means, rewrite all my commit messages with essays that nobody will ever read. I'm not going to do that for some hypothetical case that will never happen. I'm not going to waste my time. This is not a hypothetical. Almost every time I bisect a regression in git.git, I find the commit message tells me exactly why the commit did what it did and what the expected result was. I find this to be amazingly useful. Do I need to show you real instances of that happening? No. I promise it did, though. Yes please. Show me one of the instances where you hit a bisect with any of the remote-hg commits mentioned above by Thomas Rast. I made no such claim. In fact, I have never bisected to any remote-hg-related commit. I fail to see the relevance of this qualifier, though. P -- 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: Pushing/fetching from/into a shallow-cloned repository
From: Konstantin Khomoutov kostix+...@007spb.ru Sent: Thursday, April 18, 2013 10:52 AM The git-clone manual page, both [1] and my local copy coming with Git for Windows 1.8.1, say about the --depth command-line option: --depth depth Create a shallow clone with a history truncated to the specified number of revisions. A shallow repository has a number of limitations (you cannot clone or fetch from it, nor push from nor into it), but is adequate if you are only interested in the recent history of a large project with a long history, and would want to send in fixes as patches. But having done a shallow clone (--depth=1) of one of my repositories, I was able to record a new commit, push it out to a reference bare repository and then fetch back to another clone of the same repository just fine. I have then killed my test commit doing a forced push from another clone and subsequently was able to do `git fetch` in my shallow clone just fine. So I observe pushing/fetching works OK at least for a simple case like this one. Hence I'd like to ask: if the manual page is wrong or I'm observing some corner case? -- The manual is deliberately misleading. The problem is that the depth is a movable feast that depends on how far the branches have progressed. The DAG will be missing the historic merge bases, and in some scenarios can form disconnected runs of commits. The ref negotiation can also be a problem. The git\Documentation\technical\shallow.txt has some extra details on issues. Philip -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 11:16:33AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: could work for both cases. Something like not considering (or another synonym for considering) might be even more accurate. It is not just that we did not stage it; it is what we did not even consider it an item for staging under the current rules. Yes, not considering is much more sensible, while side-stepping the dryrun issue. Or warning(ignoring removal of '%s') I like that much better than either of my suggestions. Note that the not staging warnings may potentially be interspersed with the normal dry-run output. I think that's OK. As long as the top-text makes it clear what not considering (or ignoring) in the following text means, I think it is fine. Agreed, and I think the current text is fine for that (though neither of us is the best judge at this point of how a less familiar user would interpret it). But I think we are doing users a disservice by listing tons of paths. Where the difference of versions matters _most_ is when the user has tons of removed paths in the working tree. Either with one warning per path, or a block of collected paths at the end, we are scrolling the more important part of the message up. I'm not sure I agree. Even with a handful, it made me wonder why one was mentioned and not others. That _could_ be cleared up by rewording (i.e., making it clear that this is an example, and there may be more). But somehow listing them is what I would expect. Perhaps because it gives the user a clue about what to do next; they ask themselves did I want those updated or not?. In the orphaned-commit message when leaving a detached HEAD, we collect the answer, say you are leaving N commits, and show the first 5 five of them, with an ellipsis at the end if we didn't show them all. Would it makes sense to do that here? Yet another alternative would be to print a warning for each path, but hold the main warning for the end, so that it is the first thing the user sees. That has the added bonus that regular --dry-run output will not scroll it away, either. -Peff -- 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 over HTTPS with basic authentication
On Thu, Apr 18, 2013 at 09:54:32PM +0200, Matthieu Moy wrote: Sebastian Schmidt isib...@gmail.com writes: Why is git not working over HTTPS with basic authentication? I can clone just fine, but when I try to push, it tells me What are you using on the server? Dumb HTTP works by serving the repository files as static pages, which is fundamentally read-only. The recommended way is to use smart-HTTP (see man git-http-backend, requires Git on the server), and the alternative is to use webdav (much slower). Yeah, this is definitely dumb http (since http-push is involved at all, which the original error message showed). Code 22 is curl's there was an HTTP error code, but http-push annoyingly does not output the actual HTTP error[1]. You can see more by setting GIT_CURL_VERBOSE=1 in the environment. Though if you know you did not set up WebDAV on the server, then we can know that is the problem even without seeing the HTTP code. :) -Peff [1] The dumb-http push code is largely unloved and unmaintained at this point. Yet another reason to move to smart http. -- 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
What's cooking in git.git (Apr 2013, #06; Thu, 18)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * fc/completion (2013-04-14) 8 commits (merged to 'next' on 2013-04-14 at a509746) + completion: small optimization + completion: inline __gitcomp_1 to its sole callsite + completion: get rid of compgen + completion: add __gitcomp_nl tests + completion: add new __gitcompadd helper + completion: get rid of empty COMPREPLY assignments + completion: trivial test improvement + completion: add more cherry-pick options In addition to a user visible change to offer more options to cherry-pick, generally cleans up and simplifies the code. * fc/send-email-annotate (2013-04-14) 7 commits (merged to 'next' on 2013-04-14 at 4af1076) + rebase-am: explicitly disable cover-letter + format-patch: trivial cleanups + format-patch: add format.coverLetter configuration variable + log: update to OPT_BOOL + format-patch: refactor branch name calculation + format-patch: improve head calculation for cover-letter + send-email: make annotate configurable Allows format-patch --cover-letter to be configurable; the most notable is the auto mode to create cover-letter only for multi patch series. * jc/detached-head-doc (2013-04-05) 1 commit (merged to 'next' on 2013-04-14 at 24b9271) + glossary: extend detached HEAD description Describe what happens when a command that operates on the current branch is run on a detached HEAD. * jk/daemon-user-doc (2013-04-12) 1 commit (merged to 'next' on 2013-04-14 at 56c08ff) + doc: clarify that git daemon --user=user option does not export HOME=~user Document where the configuration is read by the git-daemon when its --user option is used. * jk/http-dumb-namespaces (2013-04-09) 1 commit (merged to 'next' on 2013-04-15 at 4bfa834) + http-backend: respect GIT_NAMESPACE with dumb clients Allow smart-capable HTTP servers to be restricted via the GIT_NAMESPACE mechanism when talking with commit-walker clients (they already do so when talking with smart HTTP clients). * jk/http-error-messages (2013-04-16) 1 commit (merged to 'next' on 2013-04-16 at 4a32517) + http: set curl FAILONERROR each time we select a handle A regression fix for the recently graduated topic. * jk/merge-tree-added-identically (2013-04-08) 1 commit (merged to 'next' on 2013-04-15 at 35fd4b9) + merge-tree: don't print entries that match local The resolution of some corner cases by git merge-tree were inconsistent between top-of-the-tree and in a subdirectory. * jk/test-trash (2013-04-14) 2 commits (merged to 'next' on 2013-04-15 at 15a6624) + t/test-lib.sh: drop $test variable + t/test-lib.sh: fix TRASH_DIRECTORY handling Fix longstanding issues with the test harness when used with --root=there option. * kb/co-orphan-suggestion-short-sha1 (2013-04-08) 1 commit (merged to 'next' on 2013-04-14 at 8caf7fd) + checkout: abbreviate hash in suggest_reattach Update the informational message when git checkout leaves the detached head state. * rs/empty-archive (2013-04-10) 1 commit (merged to 'next' on 2013-04-15 at eab39bc) + t5004: fix issue with empty archive test and bsdtar Implementations of tar of BSD descend have found to have trouble with reading an otherwise empty tar archive with pax headers and causes an unnecessary test failure. * th/t9903-symlinked-workdir (2013-04-11) 1 commit (merged to 'next' on 2013-04-15 at f062dc6) + t9903: Don't fail when run from path accessed through symlink * tr/packed-object-info-wo-recursion (2013-03-27) 3 commits (merged to 'next' on 2013-03-29 at b1c3858) + sha1_file: remove recursion in unpack_entry + Refactor parts of in_delta_base_cache/cache_or_unpack_entry + sha1_file: remove recursion in packed_object_info Attempts to reduce the stack footprint of sha1_object_info() and unpack_entry() codepaths. -- [New Topics] * jk/a-thread-only-dies-once (2013-04-16) 2 commits (merged to 'next' on 2013-04-18 at 3208f44) + run-command: use thread-aware die_is_recursing routine + usage: allow pluggable die-recursion checks A regression fix for the logic to detect die() handler triggering itself recursively. Will fast-track to 'master'. * tr/copy-revisions-from-stdin (2013-04-16) 1 commit (merged to 'next' on 2013-04-16 at d882870) + read_revisions_from_stdin: make copies for handle_revision_arg A fix to a long-standing issue in the command line parser for revisions, which was triggered by mv/sequence-pick-error-diag topic (now in 'next'). Will merge to 'master'. * jc/prune-all (2013-04-18) 3 commits - api-parse-options.txt: document no- for
Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King p...@peff.net writes: But I think we are doing users a disservice by listing tons of paths. Where the difference of versions matters _most_ is when the user has tons of removed paths in the working tree. Either with one warning per path, or a block of collected paths at the end, we are scrolling the more important part of the message up. I'm not sure I agree. Even with a handful, it made me wonder why one was mentioned and not others. That _could_ be cleared up by rewording (i.e., making it clear that this is an example, and there may be more). But somehow listing them is what I would expect. Perhaps because it gives the user a clue about what to do next; they ask themselves did I want those updated or not?. In the orphaned-commit message when leaving a detached HEAD, we collect the answer, say you are leaving N commits, and show the first 5 five of them, with an ellipsis at the end if we didn't show them all. Would it makes sense to do that here? Because this is to help people who are _used_ to seeing git add not take the removals into account, I doubt that Did I want those updated or not? Let me see the details of them. will be the question they will be asking [*1*]. I dunno. [Footnote] *1* I know I didn't want to include these removals to the index, but I learned today that in later Git I should make myself more clear if I want to keep doing so; thanks for letting me know., or I've long been assuming that I have to say 'git add' and 'git rm' separately, but I learned today that I can say 'add --all', and in later Git I do not even have to; thanks for letting me know. are the two reactions I expected. -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
On Thu, Apr 18, 2013 at 02:37:53PM -0700, Junio C Hamano wrote: Because this is to help people who are _used_ to seeing git add not take the removals into account, I doubt that Did I want those updated or not? Let me see the details of them. will be the question they will be asking [*1*]. I dunno. [Footnote] *1* I know I didn't want to include these removals to the index, but I learned today that in later Git I should make myself more clear if I want to keep doing so; thanks for letting me know., or I've long been assuming that I have to say 'git add' and 'git rm' separately, but I learned today that I can say 'add --all', and in later Git I do not even have to; thanks for letting me know. are the two reactions I expected. I am expecting a reaction more like Hmm, I never thought about it before. Does that make sense to me or not? Let me think about which paths it pertains to and decide. Which I admit is no more likely than the scenarios you outlined, but it is close to what I thought the first time I saw the warning. -Peff -- 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: What's cooking in git.git (Apr 2013, #05; Mon, 15)
Jeff King p...@peff.net writes: I am expecting a reaction more like Hmm, I never thought about it before. Does that make sense to me or not? Let me think about which paths it pertains to and decide. Let's step back and re-review the main text. It currently says: In Git 2.0, 'git add pathspec...' will also update the index for paths removed from the working tree that match the given pathspec. If you want to 'add' only changed or newly created paths, say 'git add --no-all pathspec...' instead. This was written for the old we may want to warn logic that did not even check if we would be omitting a removal. The new logic will show the text _only_ when the difference matters, we have an opportunity to tighten it a lot, for example: You ran 'git add' with neither '-A (--all)' or '--no-all', whose behaviour will change in Git 2.0 with respect to paths you removed from your working tree. * 'git add --no-all pathspec', which is the current default, ignores paths you removed from your working tree. * 'git add --all pathspec' will let you also record the removals. The removed paths (e.g. '%s') are ignored with this version of Git. Run 'git status' to remind yourself what paths you have removed from your working tree. or something? -- 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] stash: tighten IS_STASH_LIKE logic
Ramkumar Ramachandra artag...@gmail.com writes: Currently, 'git stash show' and 'git stash apply' can show/apply any merge commit, as the test for setting IS_STASH_LIKE simply asserts if the commit is a merge. Improve the situation by asserting if the index_commit and the worktree_commit are based off the same commit, by checking that $REV^1 is equal to $REV^2^1. When was the last time you tried to run git stash apply next? The current code parses the ancestors and trees because the real work needs them, and the 'does this look like a stash?' is a side effect, but reading it^2^1 and comparing with it^1 is a pure overhead for the sake of checking. It looks like a futile mental masturbation to solve theoretical non-issue to me. Is it worth it? -- 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 2/2] submodule: drop the top-level requirement
John Keeping j...@keeping.me.uk writes: +relative_path () +{ + local target curdir result + target=$1 + curdir=${2-$wt_prefix} + curdir=${curdir%/} + result= + + while test -n $curdir + do + case $target in + $curdir/*) + target=${target#$curdir/} + break + ;; + esac Could $curdir have glob wildcard to throw this part of the logic off? It is OK to have limitations like you cannot have a glob characters in your path to submodule working tree (at least until we start rewriting these in C or Perl or Python), but we need to be aware of them. module_list() { + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@) An efficient reuse of -- ;-) +test_expect_success 'run summary from subdir' ' + mkdir sub + ( + cd sub + git submodule summary ../actual + ) + cat expected -EOF +* ../sm1 000...$head1 (2): + Add foo2 + +EOF It somewhat looks strange to start with -EOF and then not to indent the body nor EOF. -- 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: Pushing/fetching from/into a shallow-cloned repository
Philip Oakley philipoak...@iee.org writes: So I observe pushing/fetching works OK at least for a simple case like this one. Hence I'd like to ask: if the manual page is wrong or I'm observing some corner case? -- The manual is deliberately misleading. Yes, it is erring on the safe side. It was not coded with the case where the gap widens (e.g. either side rewinds the history) in mind as you explained; for simple cases the code just happens to work, but the users are encouraged not to rely on it to be safe than sorry. -- 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] help.c: add a compatibility comment to cmd_version()
From: Junio C Hamano gits...@pobox.com Sent: Thursday, April 18, 2013 1:13 AM Philip Oakley philipoak...@iee.org writes: How about * E.g. git gui uses the extended regular expression ^git version [1-9]+(\.[0-9]+)+.* * to check for an acceptable version string. The ERE is from git-gui.sh:L958. That is exactly the kind of guarantee we do _not_ want to give. So you are trying to avoid giving _any_ guarantee about what visible manifestation a user may obtain about a system that passes the test suite we have set out. I was hoping we could give a basic minimum indication of the expected style of the version string for a git which *passes* our tests. But like you say, it doesn't form a real guarantee - caveat emptor still applies. ... Hence my suggestion of the basic test that a passing git would produce a consistent version string. I have been assuming that you are trying to avoid an exchange like this one we had in the past: http://thread.gmane.org/gmane.comp.version-control.git/216923/focus=217007 In a sense yes. As David noted, others do attempt to trust us via our current version string format. I thought it worthwhile (given my earlier 'mistake' in 216923/focus=216925) to suggest such a basic indication of our current string style. I also have been assuming that you are pushing to limit the possible versioning scheme, but I do not know what that extra limitation would achieve in the real world. By sticking to the pattern git gui happens to use, git gui may be able to guess that the next version of Git says it is verison 1.8.3. That is the _only_ thing your test buys. But having parsed the 1.8.3 substring out of it, git gui does not know anything about what 1.8.3 gives it. As far as it is concerned, it is a version whose git version output it has never seen (unless it has been kept up to date with the development of Git). You are focusssing on the wrong side of issue (from my viewpoint). If my earlier patch had gone in, it would have passsed our tests but not played nicely with the community tools. That would have been IMHO a regression, so from my viewpoint I believed it was worth having a test to avoid such a 'regression'. By matching against git version [1-9]+(\.[0-9]+)+, it is accepting that future versions may break assumptions it makes for some known versions (which is warranted) and all future versions (which is unwarranted) of Git. Maybe the version 2.0 of Git adds all changes in the directory d, including removals, when you say git add d, but it may have assumed that with Git version 1.5.0 or newer, saying git add d would result in added and modified inside that directory getting updated in the index, but paths removed from the working tree will stay in the index. If it was a gross incompatibility then (a) we are likley to be signalling it for a while, and (b) other tools would need updating as well, and they would hope that they could 'read' the version in a consistent style. We would also still have the choice of changing our existing string style, which would explicitly signal a change via the test fail. The only thing the scripts that read from the output of git version can reliably tell is if it is interacting with a version of Git it knows about. If it made any unwarranted assumption on the versions it hasn't seen, it has to be fixed in git gui _anyway_. Of course, we would not change the output of git version willy-nilly without good reason, but that is a different topic. Ah. I thought it was the [original] topic. I was calibrating the willy-nillyness ;-) So I do not know what you want to achieve in the real world with that extra limitation on the git version output format. Maybe you are proposing something else. I dunno. I was just using a slightly different philosophical approach. -- Philip -- 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 v4 01/13] pretty: save commit encoding from logmsg_reencode if the caller needs it
The commit encoding is parsed by logmsg_reencode, there's no need for the caller to re-parse it again. The reencoded message now has the new encoding, not the original one. The caller would need to read commit object again before parsing. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/blame.c | 2 +- builtin/commit.c | 2 +- commit.h | 1 + pretty.c | 16 revision.c | 2 +- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 86100e9..104a948 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1425,7 +1425,7 @@ static void get_commit_info(struct commit *commit, commit_info_init(ret); encoding = get_log_output_encoding(); - message = logmsg_reencode(commit, encoding); + message = logmsg_reencode(commit, NULL, encoding); get_ac_line(message, \nauthor , ret-author, ret-author_mail, ret-author_time, ret-author_tz); diff --git a/builtin/commit.c b/builtin/commit.c index 4620437..d2f30d9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -955,7 +955,7 @@ static const char *read_commit_message(const char *name) if (!commit) die(_(could not lookup commit %s), name); out_enc = get_commit_output_encoding(); - return logmsg_reencode(commit, out_enc); + return logmsg_reencode(commit, NULL, out_enc); } static int parse_and_validate_options(int argc, const char *argv[], diff --git a/commit.h b/commit.h index 87b4b6c..ad55213 100644 --- a/commit.h +++ b/commit.h @@ -101,6 +101,7 @@ struct userformat_want { extern int has_non_ascii(const char *text); struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */ extern char *logmsg_reencode(const struct commit *commit, +char **commit_encoding, const char *output_encoding); extern void logmsg_free(char *msg, const struct commit *commit); extern void get_commit_format(const char *arg, struct rev_info *); diff --git a/pretty.c b/pretty.c index d3a82d2..c361b9b 100644 --- a/pretty.c +++ b/pretty.c @@ -594,6 +594,7 @@ static char *replace_encoding_header(char *buf, const char *encoding) } char *logmsg_reencode(const struct commit *commit, + char **commit_encoding, const char *output_encoding) { static const char *utf8 = UTF-8; @@ -615,9 +616,15 @@ char *logmsg_reencode(const struct commit *commit, sha1_to_hex(commit-object.sha1), typename(type)); } - if (!output_encoding || !*output_encoding) + if (!output_encoding || !*output_encoding) { + if (commit_encoding) + *commit_encoding = + get_header(commit, msg, encoding); return msg; + } encoding = get_header(commit, msg, encoding); + if (commit_encoding) + *commit_encoding = encoding; use_encoding = encoding ? encoding : utf8; if (same_encoding(use_encoding, output_encoding)) { /* @@ -658,7 +665,8 @@ char *logmsg_reencode(const struct commit *commit, if (out) out = replace_encoding_header(out, output_encoding); - free(encoding); + if (!commit_encoding) + free(encoding); /* * If the re-encoding failed, out might be NULL here; in that * case we just return the commit message verbatim. @@ -1288,7 +1296,7 @@ void format_commit_message(const struct commit *commit, context.commit = commit; context.pretty_ctx = pretty_ctx; context.wrap_start = sb-len; - context.message = logmsg_reencode(commit, output_enc); + context.message = logmsg_reencode(commit, NULL, output_enc); strbuf_expand(sb, format, format_commit_item, context); rewrap_message_tail(sb, context, 0, 0, 0); @@ -1451,7 +1459,7 @@ void pretty_print_commit(const struct pretty_print_context *pp, } encoding = get_log_output_encoding(); - msg = reencoded = logmsg_reencode(commit, encoding); + msg = reencoded = logmsg_reencode(commit, NULL, encoding); if (pp-fmt == CMIT_FMT_ONELINE || pp-fmt == CMIT_FMT_EMAIL) indent = 0; diff --git a/revision.c b/revision.c index 71e62d8..2e77397 100644 --- a/revision.c +++ b/revision.c @@ -2314,7 +2314,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt) * in it. */ encoding = get_log_output_encoding(); - message = logmsg_reencode(commit, encoding); + message = logmsg_reencode(commit, NULL, encoding); /* Copy the commit to temporary if we are using fake headers */ if (buf.len) -- 1.8.2.82.gc24b958 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More
[PATCH v4 02/13] pretty: get the correct encoding for --pretty:format=%e
parse_commit_header() provides the commit encoding for '%e' and it reads it from the re-encoded message, which contains the new encoding, not the original one in the commit object. This never happens because --pretty=format:xxx never respects i18n.logoutputencoding. But that's a different story. Get the commit encoding from logmsg_reencode() instead. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pretty.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pretty.c b/pretty.c index c361b9b..e59688b 100644 --- a/pretty.c +++ b/pretty.c @@ -776,12 +776,12 @@ struct format_commit_context { unsigned commit_message_parsed:1; struct signature_check signature_check; char *message; + char *commit_encoding; size_t width, indent1, indent2; /* These offsets are relative to the start of the commit message. */ struct chunk author; struct chunk committer; - struct chunk encoding; size_t message_off; size_t subject_off; size_t body_off; @@ -828,9 +828,6 @@ static void parse_commit_header(struct format_commit_context *context) } else if (!prefixcmp(msg + i, committer )) { context-committer.off = i + 10; context-committer.len = eol - i - 10; - } else if (!prefixcmp(msg + i, encoding )) { - context-encoding.off = i + 9; - context-encoding.len = eol - i - 9; } i = eol; } @@ -1185,7 +1182,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder, msg + c-committer.off, c-committer.len, c-pretty_ctx-date_mode); case 'e': /* encoding */ - strbuf_add(sb, msg + c-encoding.off, c-encoding.len); + if (c-commit_encoding) + strbuf_addstr(sb, c-commit_encoding); return 1; case 'B': /* raw body */ /* message_off is always left at the initial newline */ @@ -1296,11 +1294,14 @@ void format_commit_message(const struct commit *commit, context.commit = commit; context.pretty_ctx = pretty_ctx; context.wrap_start = sb-len; - context.message = logmsg_reencode(commit, NULL, output_enc); + context.message = logmsg_reencode(commit, + context.commit_encoding, + output_enc); strbuf_expand(sb, format, format_commit_item, context); rewrap_message_tail(sb, context, 0, 0, 0); + free(context.commit_encoding); logmsg_free(context.message, commit); free(context.signature_check.gpg_output); free(context.signature_check.signer); -- 1.8.2.82.gc24b958 -- 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 v4 03/13] pretty-formats.txt: wrap long lines
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index afac703..6bde67e 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -106,18 +106,22 @@ The placeholders are: - '%P': parent hashes - '%p': abbreviated parent hashes - '%an': author name -- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] + or linkgit:git-blame[1]) - '%ae': author email -- '%aE': author email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%aE': author email (respecting .mailmap, see + linkgit:git-shortlog[1] or linkgit:git-blame[1]) - '%ad': author date (format respects --date= option) - '%aD': author date, RFC2822 style - '%ar': author date, relative - '%at': author date, UNIX timestamp - '%ai': author date, ISO 8601 format - '%cn': committer name -- '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%cN': committer name (respecting .mailmap, see + linkgit:git-shortlog[1] or linkgit:git-blame[1]) - '%ce': committer email -- '%cE': committer email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%cE': committer email (respecting .mailmap, see + linkgit:git-shortlog[1] or linkgit:git-blame[1]) - '%cd': committer date - '%cD': committer date, RFC2822 style - '%cr': committer date, relative @@ -138,9 +142,11 @@ The placeholders are: - '%gD': reflog selector, e.g., `refs/stash@{1}` - '%gd': shortened reflog selector, e.g., `stash@{1}` - '%gn': reflog identity name -- '%gN': reflog identity name (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%gN': reflog identity name (respecting .mailmap, see + linkgit:git-shortlog[1] or linkgit:git-blame[1]) - '%ge': reflog identity email -- '%gE': reflog identity email (respecting .mailmap, see linkgit:git-shortlog[1] or linkgit:git-blame[1]) +- '%gE': reflog identity email (respecting .mailmap, see + linkgit:git-shortlog[1] or linkgit:git-blame[1]) - '%gs': reflog subject - '%Cred': switch color to red - '%Cgreen': switch color to green -- 1.8.2.82.gc24b958 -- 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 v4 04/13] pretty: share code between format_decoration and show_decorations
This also adds color support to format_decorations() Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- log-tree.c | 48 ++-- log-tree.h | 1 + pretty.c | 20 ++--- t/t4207-log-decoration-colors.sh | 8 +++ 4 files changed, 39 insertions(+), 38 deletions(-) diff --git a/log-tree.c b/log-tree.c index 7cc7d59..1946e9c 100644 --- a/log-tree.c +++ b/log-tree.c @@ -175,36 +175,52 @@ static void show_children(struct rev_info *opt, struct commit *commit, int abbre } } -void show_decorations(struct rev_info *opt, struct commit *commit) +/* + * The caller makes sure there is no funny color before + * calling. format_decorations makes sure the same after return. + */ +void format_decorations(struct strbuf *sb, + const struct commit *commit, + int use_color) { const char *prefix; struct name_decoration *decoration; const char *color_commit = - diff_get_color_opt(opt-diffopt, DIFF_COMMIT); + diff_get_color(use_color, DIFF_COMMIT); const char *color_reset = - decorate_get_color_opt(opt-diffopt, DECORATION_NONE); + decorate_get_color(use_color, DECORATION_NONE); - if (opt-show_source commit-util) - printf(\t%s, (char *) commit-util); - if (!opt-show_decorations) - return; decoration = lookup_decoration(name_decoration, commit-object); if (!decoration) return; prefix = (; while (decoration) { - printf(%s, prefix); - fputs(decorate_get_color_opt(opt-diffopt, decoration-type), - stdout); + strbuf_addstr(sb, color_commit); + strbuf_addstr(sb, prefix); + strbuf_addstr(sb, decorate_get_color(use_color, decoration-type)); if (decoration-type == DECORATION_REF_TAG) - fputs(tag: , stdout); - printf(%s, decoration-name); - fputs(color_reset, stdout); - fputs(color_commit, stdout); + strbuf_addstr(sb, tag: ); + strbuf_addstr(sb, decoration-name); + strbuf_addstr(sb, color_reset); prefix = , ; decoration = decoration-next; } - putchar(')'); + strbuf_addstr(sb, color_commit); + strbuf_addch(sb, ')'); + strbuf_addstr(sb, color_reset); +} + +void show_decorations(struct rev_info *opt, struct commit *commit) +{ + struct strbuf sb = STRBUF_INIT; + + if (opt-show_source commit-util) + printf(\t%s, (char *) commit-util); + if (!opt-show_decorations) + return; + format_decorations(sb, commit, opt-diffopt.use_color); + fputs(sb.buf, stdout); + strbuf_release(sb); } static unsigned int digits_in_number(unsigned int number) @@ -540,8 +556,8 @@ void show_log(struct rev_info *opt) printf( (from %s), find_unique_abbrev(parent-object.sha1, abbrev_commit)); + fputs(diff_get_color_opt(opt-diffopt, DIFF_RESET), stdout); show_decorations(opt, commit); - printf(%s, diff_get_color_opt(opt-diffopt, DIFF_RESET)); if (opt-commit_format == CMIT_FMT_ONELINE) { putchar(' '); } else { diff --git a/log-tree.h b/log-tree.h index 9140f48..d6ecd4d 100644 --- a/log-tree.h +++ b/log-tree.h @@ -13,6 +13,7 @@ int log_tree_diff_flush(struct rev_info *); int log_tree_commit(struct rev_info *, struct commit *); int log_tree_opt_parse(struct rev_info *, const char **, int); void show_log(struct rev_info *opt); +void format_decorations(struct strbuf *sb, const struct commit *commit, int use_color); void show_decorations(struct rev_info *opt, struct commit *commit); void log_write_email_headers(struct rev_info *opt, struct commit *commit, const char **subject_p, diff --git a/pretty.c b/pretty.c index e59688b..e0f93ba 100644 --- a/pretty.c +++ b/pretty.c @@ -908,23 +908,6 @@ static void parse_commit_message(struct format_commit_context *c) c-commit_message_parsed = 1; } -static void format_decoration(struct strbuf *sb, const struct commit *commit) -{ - struct name_decoration *d; - const char *prefix = (; - - load_ref_decorations(DECORATE_SHORT_REFS); - d = lookup_decoration(name_decoration, commit-object); - while (d) { - strbuf_addstr(sb, prefix); - prefix = , ; - strbuf_addstr(sb, d-name); - d = d-next; - } - if (prefix[0] == ',') - strbuf_addch(sb, ')'); -} - static void strbuf_wrap(struct strbuf *sb,
[PATCH v4 06/13] utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- utf8.c | 20 ++-- utf8.h | 1 + 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/utf8.c b/utf8.c index 6ed93c3..e7ba33c 100644 --- a/utf8.c +++ b/utf8.c @@ -266,18 +266,26 @@ int utf8_width(const char **start, size_t *remainder_p) * string, assuming that the string is utf8. Returns strlen() instead * if the string does not look like a valid utf8 string. */ -int utf8_strwidth(const char *string) +int utf8_strnwidth(const char *string, int len, int skip_ansi) { int width = 0; const char *orig = string; - while (1) { - if (!string) - return strlen(orig); - if (!*string) - return width; + if (len == -1) + len = strlen(string); + while (string string orig + len) { + int skip; + while (skip_ansi + (skip = display_mode_esc_sequence_len(string)) != 0) + string += skip; width += utf8_width(string, NULL); } + return string ? width : len; +} + +int utf8_strwidth(const char *string) +{ + return utf8_strnwidth(string, -1, 0); } int is_utf8(const char *text) diff --git a/utf8.h b/utf8.h index 1f8ecad..d3da96f 100644 --- a/utf8.h +++ b/utf8.h @@ -4,6 +4,7 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */ int utf8_width(const char **start, size_t *remainder_p); +int utf8_strnwidth(const char *string, int len, int skip_ansi); int utf8_strwidth(const char *string); int is_utf8(const char *text); int is_encoding_utf8(const char *name); -- 1.8.2.82.gc24b958 -- 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 v4 07/13] utf8.c: add reencode_string_len() that can handle NULs in string
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- compat/precompose_utf8.c | 2 +- utf8.c | 10 +++--- utf8.h | 19 --- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 8cf5955..d9203d0 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -78,7 +78,7 @@ void precompose_argv(int argc, const char **argv) size_t namelen; oldarg = argv[i]; if (has_non_ascii(oldarg, (size_t)-1, namelen)) { - newarg = reencode_string_iconv(oldarg, namelen, ic_precompose); + newarg = reencode_string_iconv(oldarg, namelen, ic_precompose, NULL); if (newarg) argv[i] = newarg; } diff --git a/utf8.c b/utf8.c index e7ba33c..7c342ff 100644 --- a/utf8.c +++ b/utf8.c @@ -468,7 +468,7 @@ int utf8_fprintf(FILE *stream, const char *format, ...) #else typedef char * iconv_ibp; #endif -char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv) +char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, int *outsz_p) { size_t outsz, outalloc; char *out, *outpos; @@ -502,13 +502,17 @@ char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv) } else { *outpos = '\0'; + if (outsz_p) + *outsz_p = outpos - out; break; } } return out; } -char *reencode_string(const char *in, const char *out_encoding, const char *in_encoding) +char *reencode_string_len(const char *in, int insz, + const char *out_encoding, const char *in_encoding, + int *outsz) { iconv_t conv; char *out; @@ -534,7 +538,7 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e return NULL; } - out = reencode_string_iconv(in, strlen(in), conv); + out = reencode_string_iconv(in, insz, conv, outsz); iconv_close(conv); return out; } diff --git a/utf8.h b/utf8.h index d3da96f..a43ef9a 100644 --- a/utf8.h +++ b/utf8.h @@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len, int indent, int indent2, int width); #ifndef NO_ICONV -char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv); -char *reencode_string(const char *in, const char *out_encoding, const char *in_encoding); +char *reencode_string_iconv(const char *in, size_t insz, + iconv_t conv, int *outsz); +char *reencode_string_len(const char *in, int insz, + const char *out_encoding, + const char *in_encoding, + int *outsz); #else -#define reencode_string(a,b,c) NULL +#define reencode_string_len(a,b,c,d,e) NULL #endif +static inline char *reencode_string(const char *in, + const char *out_encoding, + const char *in_encoding) +{ + return reencode_string_len(in, strlen(in), + out_encoding, in_encoding, + NULL); +} + int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding); #endif -- 1.8.2.82.gc24b958 -- 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 v4 08/13] pretty: two phase conversion for non utf-8 commits
Always assume format_commit_item() takes an utf-8 string for string handling simplicity (we can handle utf-8 strings, but can't with other encodings). If commit message is in non-utf8, or output encoding is not, then the commit is first converted to utf-8, processed, then output converted to output encoding. This of course only works with encodings that are compatible with Unicode. This also fixes the iso8859-1 test in t6006. It's supposed to create an iso8859-1 commit, but the commit content in t6006 is in UTF-8. t6006 is now converted back in UTF-8 (the downside is we can't put utf-8 strings there anymore). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pretty.c | 24 ++-- t/t6006-rev-list-format.sh | 12 ++-- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/pretty.c b/pretty.c index e0f93ba..5947275 100644 --- a/pretty.c +++ b/pretty.c @@ -954,7 +954,8 @@ static int format_reflog_person(struct strbuf *sb, return format_person_part(sb, part, ident, strlen(ident), dmode); } -static size_t format_commit_one(struct strbuf *sb, const char *placeholder, +static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ + const char *placeholder, void *context) { struct format_commit_context *c = context; @@ -1193,7 +1194,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder, return 0; /* unknown placeholder */ } -static size_t format_commit_item(struct strbuf *sb, const char *placeholder, +static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */ +const char *placeholder, void *context) { int consumed; @@ -1273,6 +1275,7 @@ void format_commit_message(const struct commit *commit, { struct format_commit_context context; const char *output_enc = pretty_ctx-output_encoding; + const char *utf8 = UTF-8; memset(context, 0, sizeof(context)); context.commit = commit; @@ -1285,6 +1288,23 @@ void format_commit_message(const struct commit *commit, strbuf_expand(sb, format, format_commit_item, context); rewrap_message_tail(sb, context, 0, 0, 0); + if (output_enc) { + if (same_encoding(utf8, output_enc)) + output_enc = NULL; + } else { + if (context.commit_encoding + !same_encoding(context.commit_encoding, utf8)) + output_enc = context.commit_encoding; + } + + if (output_enc) { + int outsz; + char *out = reencode_string_len(sb-buf, sb-len, + output_enc, utf8, outsz); + if (out) + strbuf_attach(sb, out, outsz, outsz + 1); + } + free(context.commit_encoding); logmsg_free(context.message, commit); free(context.signature_check.gpg_output); diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh index 3fc3b74..0393c9f 100755 --- a/t/t6006-rev-list-format.sh +++ b/t/t6006-rev-list-format.sh @@ -184,7 +184,7 @@ Test printing of complex bodies This commit message is much longer than the others, and it will be encoded in iso8859-1. We should therefore -include an iso8859 character: ¡bueno! +include an iso8859 character: �bueno! EOF test_expect_success 'setup complex body' ' git config i18n.commitencoding iso8859-1 @@ -192,14 +192,14 @@ git config i18n.commitencoding iso8859-1 ' test_format complex-encoding %e 'EOF' -commit f58db70b055c5718631e5c61528b28b12090cdea +commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 iso8859-1 commit 131a310eb913d107dd3c09a65d1651175898735d commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873 EOF test_format complex-subject %s 'EOF' -commit f58db70b055c5718631e5c61528b28b12090cdea +commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 Test printing of complex bodies commit 131a310eb913d107dd3c09a65d1651175898735d changed foo @@ -208,17 +208,17 @@ added foo EOF test_format complex-body %b 'EOF' -commit f58db70b055c5718631e5c61528b28b12090cdea +commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 This commit message is much longer than the others, and it will be encoded in iso8859-1. We should therefore -include an iso8859 character: ¡bueno! +include an iso8859 character: �bueno! commit 131a310eb913d107dd3c09a65d1651175898735d commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873 EOF test_expect_success '%x00 shows NUL' ' - echo expect commit f58db70b055c5718631e5c61528b28b12090cdea + echo expect commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 echo expect fooQbar git rev-list -1 --format=foo%x00bar HEAD actual.nul nul_to_q actual.nul actual -- 1.8.2.82.gc24b958 -- To unsubscribe from this list: send the line unsubscribe git in the body of a
[PATCH v4 09/13] pretty: split color parsing into a separate function
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- pretty.c | 71 +++- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/pretty.c b/pretty.c index 5947275..e0413e3 100644 --- a/pretty.c +++ b/pretty.c @@ -954,6 +954,44 @@ static int format_reflog_person(struct strbuf *sb, return format_person_part(sb, part, ident, strlen(ident), dmode); } +static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ + const char *placeholder, + struct format_commit_context *c) +{ + if (placeholder[1] == '(') { + const char *begin = placeholder + 2; + const char *end = strchr(begin, ')'); + char color[COLOR_MAXLEN]; + + if (!end) + return 0; + if (!prefixcmp(begin, auto,)) { + if (!want_color(c-pretty_ctx-color)) + return end - placeholder + 1; + begin += 5; + } + color_parse_mem(begin, + end - begin, + --pretty format, color); + strbuf_addstr(sb, color); + return end - placeholder + 1; + } + if (!prefixcmp(placeholder + 1, red)) { + strbuf_addstr(sb, GIT_COLOR_RED); + return 4; + } else if (!prefixcmp(placeholder + 1, green)) { + strbuf_addstr(sb, GIT_COLOR_GREEN); + return 6; + } else if (!prefixcmp(placeholder + 1, blue)) { + strbuf_addstr(sb, GIT_COLOR_BLUE); + return 5; + } else if (!prefixcmp(placeholder + 1, reset)) { + strbuf_addstr(sb, GIT_COLOR_RESET); + return 6; + } else + return 0; +} + static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const char *placeholder, void *context) @@ -967,38 +1005,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ /* these are independent of the commit */ switch (placeholder[0]) { case 'C': - if (placeholder[1] == '(') { - const char *begin = placeholder + 2; - const char *end = strchr(begin, ')'); - char color[COLOR_MAXLEN]; - - if (!end) - return 0; - if (!prefixcmp(begin, auto,)) { - if (!want_color(c-pretty_ctx-color)) - return end - placeholder + 1; - begin += 5; - } - color_parse_mem(begin, - end - begin, - --pretty format, color); - strbuf_addstr(sb, color); - return end - placeholder + 1; - } - if (!prefixcmp(placeholder + 1, red)) { - strbuf_addstr(sb, GIT_COLOR_RED); - return 4; - } else if (!prefixcmp(placeholder + 1, green)) { - strbuf_addstr(sb, GIT_COLOR_GREEN); - return 6; - } else if (!prefixcmp(placeholder + 1, blue)) { - strbuf_addstr(sb, GIT_COLOR_BLUE); - return 5; - } else if (!prefixcmp(placeholder + 1, reset)) { - strbuf_addstr(sb, GIT_COLOR_RESET); - return 6; - } else - return 0; + return parse_color(sb, placeholder, c); case 'n': /* newline */ strbuf_addch(sb, '\n'); return 1; -- 1.8.2.82.gc24b958 -- 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 v4 10/13] pretty: add %C(auto) for auto-coloring
This is not simply convenient over %C(auto,xxx). Some placeholders (actually only one, %d) do multi coloring and we can't emit a multiple colors with %C(auto,xxx). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 3 ++- pretty.c | 26 +++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 6bde67e..bad627a 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -156,7 +156,8 @@ The placeholders are: adding `auto,` at the beginning will emit color only when colors are enabled for log output (by `color.diff`, `color.ui`, or `--color`, and respecting the `auto` settings of the former if we are going to a - terminal) + terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring + on the next placeholders until the color is switched again. - '%m': left, right or boundary mark - '%n': newline - '%%': a raw '%' diff --git a/pretty.c b/pretty.c index e0413e3..3612f82 100644 --- a/pretty.c +++ b/pretty.c @@ -778,6 +778,7 @@ struct format_commit_context { char *message; char *commit_encoding; size_t width, indent1, indent2; + int auto_color; /* These offsets are relative to the start of the commit message. */ struct chunk author; @@ -1005,7 +1006,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ /* these are independent of the commit */ switch (placeholder[0]) { case 'C': - return parse_color(sb, placeholder, c); + if (!prefixcmp(placeholder + 1, (auto))) { + c-auto_color = 1; + return 7; /* consumed 7 bytes, C(auto) */ + } else { + int ret = parse_color(sb, placeholder, c); + if (ret) + c-auto_color = 0; + /* +* Otherwise, we decided to treat %Cunknown +* as a literal string, and the previous +* %C(auto) is still valid. +*/ + return ret; + } case 'n': /* newline */ strbuf_addch(sb, '\n'); return 1; @@ -1051,13 +1065,19 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ switch (placeholder[0]) { case 'H': /* commit hash */ + strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_COMMIT)); strbuf_addstr(sb, sha1_to_hex(commit-object.sha1)); + strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_RESET)); return 1; case 'h': /* abbreviated commit hash */ - if (add_again(sb, c-abbrev_commit_hash)) + strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_COMMIT)); + if (add_again(sb, c-abbrev_commit_hash)) { + strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_RESET)); return 1; + } strbuf_addstr(sb, find_unique_abbrev(commit-object.sha1, c-pretty_ctx-abbrev)); + strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_RESET)); c-abbrev_commit_hash.len = sb-len - c-abbrev_commit_hash.off; return 1; case 'T': /* tree hash */ @@ -1095,7 +1115,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return 1; case 'd': load_ref_decorations(DECORATE_SHORT_REFS); - format_decorations(sb, commit, 0); + format_decorations(sb, commit, c-auto_color); return 1; case 'g': /* reflog info */ switch(placeholder[1]) { -- 1.8.2.82.gc24b958 -- 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 v4 11/13] pretty: support padding placeholders, % % and %
Either %, % or % standing before a placeholder specifies how many columns (at least as the placeholder can exceed it) it takes. Each differs on how spaces are padded: % pads on the right (aka left alignment) % pads on the left (aka right alignment) % pads both ways equally (aka centered) The (N) follows them, e.g. `%(100)', to specify the number of columns the next placeholder takes. However, if '|' stands before (N), e.g. `%|(100)', then the number of columns is calculated so that it reaches the Nth column on screen. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 8 +++ pretty.c | 117 - t/t4205-log-pretty-formats.sh| 122 +++ 3 files changed, 246 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index bad627a..e2345d2 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -164,6 +164,14 @@ The placeholders are: - '%x00': print a byte from a hex code - '%w([w[,i1[,i2]]])': switch line wrapping, like the -w option of linkgit:git-shortlog[1]. +- '%(N)': make the next placeholder take at least N columns, + padding spaces on the right if necessary +- '%|(N)': make the next placeholder take at least until Nth + columns, padding spaces on the right if necessary +- '%(N)', '%|(N)': similar to '%(N)', '%|(N)' + respectively, but padding spaces on the left +- '%(N)', '%|(N)': similar to '%(N)', '%|(N)' + respectively, but padding both sides (i.e. the text is centered) NOTE: Some placeholders may depend on other options given to the revision traversal engine. For example, the `%g*` reflog options will diff --git a/pretty.c b/pretty.c index 3612f82..b50ebf5 100644 --- a/pretty.c +++ b/pretty.c @@ -769,16 +769,25 @@ struct chunk { size_t len; }; +enum flush_type { + no_flush, + flush_right, + flush_left, + flush_both +}; + struct format_commit_context { const struct commit *commit; const struct pretty_print_context *pretty_ctx; unsigned commit_header_parsed:1; unsigned commit_message_parsed:1; struct signature_check signature_check; + enum flush_type flush_type; char *message; char *commit_encoding; size_t width, indent1, indent2; int auto_color; + int padding; /* These offsets are relative to the start of the commit message. */ struct chunk author; @@ -993,6 +1002,52 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ return 0; } +static size_t parse_padding_placeholder(struct strbuf *sb, + const char *placeholder, + struct format_commit_context *c) +{ + const char *ch = placeholder; + enum flush_type flush_type; + int to_column = 0; + + switch (*ch++) { + case '': + flush_type = flush_right; + break; + case '': + if (*ch == '') { + flush_type = flush_both; + ch++; + } else + flush_type = flush_left; + break; + default: + return 0; + } + + /* the next value means wide enough to that column */ + if (*ch == '|') { + to_column = 1; + ch++; + } + + if (*ch == '(') { + const char *start = ch + 1; + const char *end = strchr(start, ')'); + char *next; + int width; + if (!end || end == start) + return 0; + width = strtoul(start, next, 10); + if (next == start || width == 0) + return 0; + c-padding = to_column ? -width : width; + c-flush_type = flush_type; + return end - placeholder + 1; + } + return 0; +} + static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ const char *placeholder, void *context) @@ -1057,6 +1112,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return end - placeholder + 1; } else return 0; + + case '': + case '': + return parse_padding_placeholder(sb, placeholder, c); } /* these depend on the commit */ @@ -1221,6 +1280,59 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ return 0; /* unknown placeholder */ } +static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */ + const char *placeholder, + struct format_commit_context *c) +{ + struct
[PATCH v4 12/13] pretty: support truncating in %, % and %
%(N,trunc) truncates the right part after N columns and replace the last two letters with ... ltrunc does the same on the left. mtrunc cuts the middle out. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 7 -- pretty.c | 51 +--- t/t4205-log-pretty-formats.sh| 39 ++ utf8.c | 46 utf8.h | 2 ++ 5 files changed, 140 insertions(+), 5 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index e2345d2..d3450bf 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -164,8 +164,11 @@ The placeholders are: - '%x00': print a byte from a hex code - '%w([w[,i1[,i2]]])': switch line wrapping, like the -w option of linkgit:git-shortlog[1]. -- '%(N)': make the next placeholder take at least N columns, - padding spaces on the right if necessary +- '%(N[,trunc|ltrunc|mtrunc])': make the next placeholder take at + least N columns, padding spaces on the right if necessary. + Optionally truncate at the beginning (ltrunc), the middle (mtrunc) + or the end (trunc) if the output is longer than N columns. + Note that truncating only works correctly with N = 2. - '%|(N)': make the next placeholder take at least until Nth columns, padding spaces on the right if necessary - '%(N)', '%|(N)': similar to '%(N)', '%|(N)' diff --git a/pretty.c b/pretty.c index b50ebf5..f7c6442 100644 --- a/pretty.c +++ b/pretty.c @@ -776,6 +776,13 @@ enum flush_type { flush_both }; +enum trunc_type { + trunc_none, + trunc_left, + trunc_middle, + trunc_right +}; + struct format_commit_context { const struct commit *commit; const struct pretty_print_context *pretty_ctx; @@ -783,6 +790,7 @@ struct format_commit_context { unsigned commit_message_parsed:1; struct signature_check signature_check; enum flush_type flush_type; + enum trunc_type truncate; char *message; char *commit_encoding; size_t width, indent1, indent2; @@ -1033,7 +1041,7 @@ static size_t parse_padding_placeholder(struct strbuf *sb, if (*ch == '(') { const char *start = ch + 1; - const char *end = strchr(start, ')'); + const char *end = start + strcspn(start, ,)); char *next; int width; if (!end || end == start) @@ -1043,6 +1051,23 @@ static size_t parse_padding_placeholder(struct strbuf *sb, return 0; c-padding = to_column ? -width : width; c-flush_type = flush_type; + + if (*end == ',') { + start = end + 1; + end = strchr(start, ')'); + if (!end || end == start) + return 0; + if (!prefixcmp(start, trunc))) + c-truncate = trunc_right; + else if (!prefixcmp(start, ltrunc))) + c-truncate = trunc_left; + else if (!prefixcmp(start, mtrunc))) + c-truncate = trunc_middle; + else + return 0; + } else + c-truncate = trunc_none; + return end - placeholder + 1; } return 0; @@ -1309,9 +1334,29 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */ total_consumed++; } len = utf8_strnwidth(local_sb.buf, -1, 1); - if (len padding) + if (len padding) { + switch (c-truncate) { + case trunc_left: + strbuf_utf8_replace(local_sb, + 0, len - (padding - 2), + ..); + break; + case trunc_middle: + strbuf_utf8_replace(local_sb, + padding / 2 - 1, + len - (padding - 2), + ..); + break; + case trunc_right: + strbuf_utf8_replace(local_sb, + padding - 2, len - (padding - 2), + ..); + break; + case trunc_none: + break; + } strbuf_addstr(sb, local_sb.buf); - else { + } else { int sb_len = sb-len, offset = 0; if (c-flush_type == flush_left) offset = padding - len; diff --git
[PATCH v4 13/13] pretty: support % that steal trailing spaces
This is pretty useful in `%(100)%s%Cred%(20)% an' where %s does not use up all 100 columns and %an needs more than 20 columns. By replacing %(20) with %(20), %an can steal spaces from %s. % understands escape sequences, so %Cred does not stop it from stealing spaces in %(100). Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 5 - pretty.c | 34 ++ t/t4205-log-pretty-formats.sh| 14 ++ utf8.c | 2 +- utf8.h | 1 + 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index d3450bf..c96ff41 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -173,7 +173,10 @@ The placeholders are: columns, padding spaces on the right if necessary - '%(N)', '%|(N)': similar to '%(N)', '%|(N)' respectively, but padding spaces on the left -- '%(N)', '%|(N)': similar to '%(N)', '%|(N)' +- '%(N)', '%|(N)': similar to '%(N)', '%|(N)' + respectively, except that if the next placeholder takes more spaces + than given and there are spaces on its left, use those spaces +- '%(N)', '%|(N)': similar to '% (N)', '%|(N)' respectively, but padding both sides (i.e. the text is centered) NOTE: Some placeholders may depend on other options given to the diff --git a/pretty.c b/pretty.c index f7c6442..d59579a 100644 --- a/pretty.c +++ b/pretty.c @@ -773,6 +773,7 @@ enum flush_type { no_flush, flush_right, flush_left, + flush_left_and_steal, flush_both }; @@ -1026,6 +1027,9 @@ static size_t parse_padding_placeholder(struct strbuf *sb, if (*ch == '') { flush_type = flush_both; ch++; + } else if (*ch == '') { + flush_type = flush_left_and_steal; + ch++; } else flush_type = flush_left; break; @@ -1334,6 +1338,36 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */ total_consumed++; } len = utf8_strnwidth(local_sb.buf, -1, 1); + + if (c-flush_type == flush_left_and_steal) { + const char *ch = sb-buf + sb-len - 1; + while (len padding ch sb-buf) { + const char *p; + if (*ch == ' ') { + ch--; + padding++; + continue; + } + /* check for trailing ansi sequences */ + if (*ch != 'm') + break; + p = ch - 1; + while (ch - p 10 *p != '\033') + p--; + if (*p != '\033' || + ch + 1 - p != display_mode_esc_sequence_len(p)) + break; + /* +* got a good ansi sequence, put it back to +* local_sb as we're cutting sb +*/ + strbuf_insert(local_sb, 0, p, ch + 1 - p); + ch = p - 1; + } + strbuf_setlen(sb, ch + 1 - sb-buf); + c-flush_type = flush_left; + } + if (len padding) { switch (c-truncate) { case trunc_left: diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index b8685b9..26fbfde 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -260,4 +260,18 @@ EOF test_cmp expected actual ' +test_expect_success 'left/right alignment formatting with stealing' ' + git commit --amend -m short --author long long long l...@me.com + git log --pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual + # complete the incomplete line at the end + echo actual + cat \EOF expected +short long long long +message .. A U Thor +add bar A U Thor +initial A U Thor +EOF + test_cmp expected actual +' + test_done diff --git a/utf8.c b/utf8.c index 197414a..b1e1303 100644 --- a/utf8.c +++ b/utf8.c @@ -9,7 +9,7 @@ struct interval { int last; }; -static size_t display_mode_esc_sequence_len(const char *s) +size_t display_mode_esc_sequence_len(const char *s) { const char *p = s; if (*p++ != '\033') diff --git a/utf8.h b/utf8.h index edde8ee..32a7bfb 100644 --- a/utf8.h +++ b/utf8.h @@ -3,6 +3,7 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */ +size_t display_mode_esc_sequence_len(const char *s); int utf8_width(const char **start, size_t *remainder_p); int utf8_strnwidth(const char *string, int len, int skip_ansi); int utf8_strwidth(const char
Re: git p4 submit failing
christopher.yee...@gmail.com wrote on Thu, 18 Apr 2013 11:24 -0500: The issue is caused by the line endings. I retested the problem with a different file and in this case, the error is caused by the line endings of the file checked out in the perforce workspace being win-style crlf, and the line endings of the file in the git repo being unix style lf. (In this scenario, I took out the .gitattributes, core.autocrlf was set to false and LineEnd was set to share) In this case, I checked out the file in perforce, ran dos2unix against it, and submitted that, then ran git p4 submit and it worked. I noticed that the error is caused by the git apply failing in the def applyCommit(self, id) function at lines 1296-1305. diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id) patchcmd = diffcmd + | git apply tryPatchCmd = patchcmd + --check - applyPatchCmd = patchcmd + --check --apply - patch_succeeded = True if os.system(tryPatchCmd) != 0: fixed_rcs_keywords = False patch_succeeded = False print Unfortunately applying the change failed! So most likely in git apply command, it can't find the changes because of the line endings being different between them. I couldn't find a parameter that would magically make it work. When I added --verbose to git apply the output only says: error: while searching for: and then the first lines of the first diff That seems like exactly the correct diagnosis of the problem. What to do about it, I'm not so sure. We could suggest that people use the same line-ending conventions in both git and p4 land. This is easy if they are both lf. But, if crlf is preferred, do you know how to configure git to use crlf line endings? Does that fix it? There's also the config setting apply.ignorewhitespace; not sure if that would allow the apply step to apply an lf-ending patch to the crlf-ending p4 workspace. -- Pete Hello Simon, I have CCed you to alert you to the possible bug. Any assistance would be appreciated. On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon christopher.yee...@gmail.com wrote: Yes this is the case. Many of the files have crlf endings. core.autocrlf was recently set to input. I can't remember the timeline exactly though, but in addition to this, I have a .gitattributes file with the default set to text=auto (* text=auto) and the php files set to text eol=lf (*.php text eol=lf) Also my perforce workspace's LineEnd setting is set to Share. I've experienced the behavior in both .php and .xml files though Before all of this started I had core.autocrlf set to false, and no .gitattributes file and perforce workspace's LineEnd was set to the default, but I got a conflict where the only difference was the line endings, so I changed things to the way they are now. Any recommendations? Should I change everything back the way it was? On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff p...@padd.com wrote: l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100: Just a thought, but check the files that are failing to see if they've got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all sorts of nasty problems. That's assuming it's definitely not a CRLF line ending problem on Windows. I had recently debugged a similar-looking problem where core.autocrlf was set to input. Christopher, if you have this set and/or the .xml files have ^M (CRLF) line endings, please let us know. -- Pete On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon christopher.yee...@gmail.com wrote: I tried running git p4 submit on a repo that I've been running as an interim bridge between git and perforce. Multiple people are using the repo as a remote and its being periodically submitted back to perforce. It's been working mostly fine. Then one day out of the blue I get this error. I can no longer push any git commits to perforce. (This is from the remote repo which I am pushing back to perforce) user@hostname:~/Source/code$ git p4 submit -M --export-labels Perforce checkout for depot path //depot/perforce/workspace/ located at /home/user/Source/git-p4-area/perforce/workspace/ Synchronizing p4 checkout... ... - file(s) up-to-date. Applying ffa390f comments in config xml files //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for edit //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for edit error: patch failed: sub/folder/structure/first.xml:1 error: sub/folder/structure/first.xml: patch does not apply