Re: [PATCH v5 5/7] add tests for rebasing merged history
Am 6/4/2013 19:18, schrieb Junio C Hamano: Martin von Zweigbergk martinv...@gmail.com writes: --- +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i I do not quite follow this TODO. While I think it would be nice to update rebase so that all variants consider replaying the commits in the same order, in this history you have: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z as long as o comes after n and e is shown before n or after o, which all three expected results satisify, it is in --topo-order, isn't it? The comment is really just about the inconsistency, not about a request to have a guaranteed order among the parents of a merge commit. Having said that, wouldn't it be useful (generally, not just in this context) to have a guarantee in which order --topo-order lists parents of a merge? +test_expect_success rebase -p re-creates merge from side branch + reset_rebase + git rebase -p z w + test_cmp_rev z HEAD^ + test_cmp_rev w^2 HEAD^2 + Hmm, turning the left one to the right one? +# d---e d---e +#\ \ \ \ +# n---o---w === n---o \ +# \ \ \ +# z z---W If w were a merge of o into e (i.e. w^1 were e not o), what should happen? Would we get the same topology? 'git rebase -p z w' is a nonsense request in this situation. (I.e., there is no requirement on the result.) At best, we could detect it and bail out or warn. -- 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: SNI (SSL virtual hosts)
On Tue, 4 Jun 2013, Janusz Harkot wrote: valid point, but from what you can find on the web, the only solution provided everywhere was to disable certificate checking… so maybe that's not me, but this is first time someone spent some time to check whats going on :) I don't disagree with that. You may be right. But I am the maintainer of libcurl and I have *never* gotten a report about this before, and I rather base my actions and assumptions on true reports from actual developers with whom I can discuss and delve into details with (like you and me right now). Basing decisions on vague statements posted elsewhere by unknown people is for sure a road into sadness. Anyway, now I'm off topic. I'm glad you could fix the problem. Thanks for flying git + libcurl! =) -- / daniel.haxx.se
Re: What's cooking in git.git (Jun 2013, #02; Tue, 4)
Am 6/5/2013 1:45, schrieb Junio C Hamano: * jk/test-exit-code-by-signal (2013-06-02) 1 commit (merged to 'next' on 2013-06-03 at 25af892) + t0005: test git exit code from signal death Will merge to 'master'. I haven't gotten around to run this new test on Windows. I've reason to believe that it won't pass as is. Please don't let it graduate, yet. -- 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: git archive --worktree-attributes doesn't exclude .gitattributes anymore
On Wed, Jun 05, 2013 at 07:10:01AM +0100, Gianfranco Costamagna wrote: Just a question, is it possible to add a new test for handling this kind of regressions? Yes, I added one in commit efa5f825. And second, is it possible to have a patch for this problem? I'll be glad to backport in debian, or to take care of updating the whole git package (it is already in experimental BTW) The fix took a fair bit of refactoring; it's in commits 9db9eec..efa5f825. You can either cherry-pick that range, or just use git diff to generate a patch. However, for Debian, I don't see much point. Everything up through unstable does not have the bug (it is all 1.7.10.x or older), and the version in experimental is already 1.8.3. So as far as I can tell, no Debian release is currently shipping a buggy version. -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 (Jun 2013, #02; Tue, 4)
On Wed, Jun 05, 2013 at 08:59:15AM +0200, Johannes Sixt wrote: Am 6/5/2013 1:45, schrieb Junio C Hamano: * jk/test-exit-code-by-signal (2013-06-02) 1 commit (merged to 'next' on 2013-06-03 at 25af892) + t0005: test git exit code from signal death Will merge to 'master'. I haven't gotten around to run this new test on Windows. I've reason to believe that it won't pass as is. Please don't let it graduate, yet. Yeah, I sort of assumed that we would need to check for either 3 or 131 on Windows, but I wasn't sure which. I don't think there is any rush on it. -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: [PATCH 00/15] Towards a more awesome git-branch
Duy Nguyen wrote: I'm still hung up one the detached HEAD thing. It's a bit quirky to put in for-each-ref, but for-each-ref can't truly replace branch --list until it can display detached HEAD. But I think we can finish this part and get it in first. Should be useful for some people already. Right. The branch is called hot-branch; let's polish it for inclusion, and rebase for-each-ref-pretty onto it when it's done (no need to stall work there). Signed-off-by: me for the whole series. Thanks. Will add in the re-roll. -- 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 10/15] for-each-ref: introduce format specifier %(*) and %(*)
Duy Nguyen wrote: I mentioned it before and I do it again. This is not optimal. Yeah, I'll attempt to fix this, but it's not urgent. But I guess it's ok in this shape unless you run this over hundreds of refs. Oh, you can run over a hundred refs just fine, for scripting purposes; but why would you want to run over a hundred refs with a pretty that includes %(*) or %(*)? -- 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: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 06:57:34PM -0400, Phil Hord wrote: On Tue, Jun 4, 2013 at 8:48 AM, John Keeping j...@keeping.me.uk wrote: The problem is that sometimes you do want to adjust the path and sometimes you don't. Reading git-submodule(1), it says: This may be either an absolute URL, or (if it begins with ./ or ../), the location relative to the superproject’s origin repository. [snip] If the superproject doesn’t have an origin configured the superproject is its own authoritative upstream and the current working directory is used instead. So I think it's quite reasonable to have a server layout that looks like this: project |- libs | |- libA | `- libB |- core.git and with only core.git on your local system do: cd core/libs git submodule add ../libs/libB expecting that to point to libB. But if we adjust the path then the user has to do: git submodule add ../../libs/libB However, it is also perfectly reasonable to have no remote configured and the library next to the repository itself. In which case we do want to specify the additional ../ so that shell completion works in the natural way. In submodule add, the leading '../' prefix on the repository url has always meant that the url is relative to the url of the current repo. The given repo-url is precisely what ends up in .gitmodules' submodule.$name.url. It works this way whether there is a remote configured or not. It does seem like we need to be cautious around this change. The only way I can see to resolve the ambiguity is to die when we hit this particular case. This should be acceptable because people shouldn't be adding new submodules anywhere near as often as they perform other submodule operations and it doesn't affect absolute URLs. I don't think I like that. But I don't know if I like anything I dreamed up, either. I've decided that I will make it die (unless anyone comes up with an obviously correct solution before I get around to sending the reroll) because it's a lot easier to loosen that in the future than to change it if we get the behaviour wrong now. I don't want to hold every other subcommand hostage to this one 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: [PATCH 10/15] for-each-ref: introduce format specifier %(*) and %(*)
On Wed, Jun 5, 2013 at 3:14 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Duy Nguyen wrote: I mentioned it before and I do it again. This is not optimal. Yeah, I'll attempt to fix this, but it's not urgent. Agreed it's not urgent. But I guess it's ok in this shape unless you run this over hundreds of refs. Oh, you can run over a hundred refs just fine, for scripting purposes; but why would you want to run over a hundred refs with a pretty that includes %(*) or %(*)? Good point. git for-each-ref|wc -l on my git repo says I have 673 refs. A naive use for-each-ref --pretty without any ref patterns could happen. But I guess people will quickly learn to limit the ref selection soon after doing that :-) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote-mediawiki: use Git.pm functions for credentials
From: Benoit Person benoit.per...@ensimag.fr
In 52dce6d, a new credential function was added to Git.pm, based on
git-remote-mediawiki's functions. The logical follow-up is to use
those functions in git-remote-mediawiki.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
contrib/mw-to-git/git-remote-mediawiki.perl | 66 -
1 file changed, 9 insertions(+), 57 deletions(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9c14c1f..6672e4c 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -13,6 +13,7 @@
use strict;
use MediaWiki::API;
+use Git;
use DateTime::Format::ISO8601;
# By default, use UTF-8 to communicate with Git and the user
@@ -156,57 +157,6 @@ while (STDIN) {
## Functions ##
-## credential API management (generic functions)
-
-sub credential_read {
- my %credential;
- my $reader = shift;
- my $op = shift;
- while ($reader) {
- my ($key, $value) = /([^=]*)=(.*)/;
- if (not defined $key) {
- die ERROR receiving response from git credential
$op:\n$_\n;
- }
- $credential{$key} = $value;
- }
- return %credential;
-}
-
-sub credential_write {
- my $credential = shift;
- my $writer = shift;
- # url overwrites other fields, so it must come first
- print $writer url=$credential-{url}\n if exists $credential-{url};
- while (my ($key, $value) = each(%$credential) ) {
- if (length $value $key ne 'url') {
- print $writer $key=$value\n;
- }
- }
-}
-
-sub credential_run {
- my $op = shift;
- my $credential = shift;
- my $pid = open2(my $reader, my $writer, git credential $op);
- credential_write($credential, $writer);
- print $writer \n;
- close($writer);
-
- if ($op eq fill) {
- %$credential = credential_read($reader, $op);
- } else {
- if ($reader) {
- die ERROR while running git credential $op:\n$_;
- }
- }
- close($reader);
- waitpid($pid, 0);
- my $child_exit_status = $? 8;
- if ($child_exit_status != 0) {
- die 'git credential $op' failed with code $child_exit_status.;
- }
-}
-
# MediaWiki API instance, created lazily.
my $mediawiki;
@@ -217,22 +167,24 @@ sub mw_connect_maybe {
$mediawiki = MediaWiki::API-new;
$mediawiki-{config}-{api_url} = $url/api.php;
if ($wiki_login) {
- my %credential = (url = $url);
- $credential{username} = $wiki_login;
- $credential{password} = $wiki_passwd;
- credential_run(fill, \%credential);
+ my %credential = (
+ 'url' = $url,
+ 'username' = $wiki_login,
+ 'password' = $wiki_passwd
+ );
+ Git::credential(\%credential);
my $request = {lgname = $credential{username},
lgpassword = $credential{password},
lgdomain = $wiki_domain};
if ($mediawiki-login($request)) {
- credential_run(approve, \%credential);
+ Git::credential(\%credential, 'approve');
print STDERR Logged in mediawiki user
\$credential{username}\.\n;
} else {
print STDERR Failed to log in mediawiki user
\$credential{username}\ on $url\n;
print STDERR (error .
$mediawiki-{error}-{code} . ': ' .
$mediawiki-{error}-{details} . )\n;
- credential_run(reject, \%credential);
+ Git::credential(\%credential, 'reject');
exit 1;
}
}
--
1.8.3.rc3.7.gc2f33ed.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: git-daemon: needs /root/.config/git/config?
On Wed, 05 Jun 2013 13:19:18 +, Ian Kumlien wrote: ... Well, I have no idea of how to control HOME in xinetd - access to the machine is limited and x doesn't give that much access (nothing really important is actually stored in /root) Make xinetd execute '/usr/bin/env HOME=/home/yourstruly git ...' instead of 'git ...'. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- 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 08/15] for-each-ref: get --pretty using format_commit_message
On Wed, Jun 5, 2013 at 4:12 AM, Eric Sunshine sunsh...@sunshineco.com wrote: +Caveats: + +1. Many of the placeholders in PRETTY FORMATS are designed to work + specifically on commit objects: when non-commit objects are + supplied, those placeholders won't work. Should won't work be expanded upon? It's not clear if this means that git will outright crash, or if it will abort with an appropriate error message, or if the directive will be displayed as-is or removed from the output. It will be displayed as-is but that's a bit inconsistent: %(unknown) prints error and aborts while %unknown simply produces %unknown. The latter is how git log --format does it. But I think we could make for-each-ref --pretty to do the former for %unknown. It'll be consistent with %(unknown) and we do not need to elaborate much (it's pretty obvious when it does not work). -- Duy -- 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: [Administrivia] On ruby and contrib/
On Tue, Jun 4, 2013 at 10:02 PM, David Lang da...@lang.hm wrote: On Tue, 4 Jun 2013, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: On Ruby: Assuming related is a good idea, to make it as the proper part of the system out of contrib/ when its design review phase is finished, one of these things has to happen: 1. Find a volunteer to rewrite it in one of the languages that we know the platforms our current users use already support, which means either C (not a good match), POSIX shell (not the best match), or Perl. 2. Promote Ruby to the first-class citizen status, which involves making sure people on platforms that matter do not have problem adding dependency on it (I am primarily worried about MinGW folks), and also making sure core developers do not mind reviewing code written in it. As long as we can get as high quality reviews on changes written in Ruby as we do for the current codebase, it is OK to go route #2, and that may hopefully happen in the longer term as and there will be some people, among competent Ruby programmers, who have understood how the pieces of entire Git are designed to fit together by the time it happens. I however do not know how much extra burden it would place to add dependencies to platform folks, so obviously the safer approach is 1 at least in the immediate future. My understanding is that msysgit folks are already having trouble with Python, and we do not want to go route #2 at least for now. Having to ship a variant of Git with NO_PYTHON is already bad enough. And that is why the option 1 above does not list Python as a possible candidate. As someone who builds minimalist builds (firewalls, openwrt, raspberry pi, etc), having to pull in a full ruby install to get git installed would not be something I'd like to see. You wouldn't _have_ to, just like you don't _have_ to install Python right now. -- 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: [Administrivia] On ruby and contrib/
On Tue, Jun 4, 2013 at 7:04 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: I however do not know how much extra burden it would place to add dependencies to platform folks, so obviously the safer approach is 1 at least in the immediate future. My understanding is that msysgit folks are already having trouble with Python, and we do not want to go route #2 at least for now. Having to ship a variant of Git with NO_PYTHON is already bad enough. And that is why the option 1 above does not list Python as a possible candidate. This rests on the assumption that Ruby would be as difficult to distribute as Python, which might not be the case. As the maintainer, I've been thinking about closing contrib/ area for new stuff, and shrinking existing ones, either by moving stuff that are only useful within the context of Git to main part of the tree (e.g. contrib/workdir may move to a new directory addons/, some of remote-helpers in contrib/ may move to remote-helpers/, etc.), and removing others from contrib/, for this reason. Of course, interested folks can take the last version of the removed ones and continue improving them as standalone projects. This does make sense, however, I do think some parts of Git might be more maintainable if they have their own Makefile (e.g. bash completion), where it's clear where they should be installed by default. Either way, the user might want to do 'install-all' or 'install-addons', to install all these things, and I think a good rule of thumb is that if we don't want 'install-all' to install certain script (eventually), then that script probably doesn't belong in 'contrib' (or anywhere in Git). The rest is just a personal opinion. If we were looking at a compelling and sizeable web application that depends on Rails, it is very likely that it would not make much sense to rewrite it in other languages only to avoid a new language dependency on Ruby. But related is read and extract some info out of text files, spawn a 'blame' (or two) based on that info, read to collect further info and summarize, for which Ruby does not especially shine compared to Perl, which is the language we already depend on. Because of this, I am moderately reluctant to add Ruby dependency only for this script. Unless I know people who regularly give us high quality reviews, and those who support various platforms, are fine with it, that is. In the shorter term (read: up to 2.0), I am inclined to vote that we should go route #1 (i.e. rewrite in Perl once the design settles). That might make sense for the shorter term, but in longer term I see Perl as declining in favor of other languages. It's only a matter of time before Ruby surpasses Perl in popularity, and soon enough new contributors to the Git project will have problems trying to improve Git because parts of it are written in a language they are not familiar with, and have trouble learning (isn't that already happening?). The Ruby vs. Python is another question altogether, I could go into detail about why I think Ruby is a better choice, but my point right now is that Perl is not a good choice for the future. -- 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/8] cherry-pick: add --skip-empty option
On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: You didn't answer, what happens when you run --skip-empty and --allow-empty? I'll answer to a slightly different question: What should happen? I think it should error out, because --allow-empty is about allowing empty commits to be preserved, and --skip-empty is about skipping empty commits (as it says on the package). Exactly, so they are related after all. However, --allow-empty says this: By default, cherry-picking an empty commit will fail, Should we change that too if we introduce --skip-empty? I don't think so. I think it makes more sense to have a new --empty-commits=[keep|skip|fail] option, so we don't have to document how each option affects the other, and what is the default. Or rather; the option documents that. In fact, it might make sense to change the default in v2.0 to --empty-commits=skip. If it makes sense in 'git rebase', why doesn't it in 'git cherry-pick'? -- 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
deletion of branch seems to modify tag as well?
$ git push origin :ABRANCHNAME remote: Firing Pre - receive hook remote: remote: remote: Firing Post receive hook remote: remote: Branch is ABRANCHNAME -- not creating Trigger file since this is not _int branch remote: error: Trying to write ref refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363 with nonexistent object remote: fatal: refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363: cannot update the ref To ssh://git@ourgitserver/repositoryname.git - [deleted] ABRANCHNAME I would not expect this behavior. Is git attempting to modify the tags associated to the HEAD commit on the branch in addition to the branch? If so, I would like to consider this a bug report... Thanks, Roy Lyons -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-daemon: needs /root/.config/git/config?
On Wed, Jun 05, 2013 at 01:19:18PM +0200, Ian Kumlien wrote: Older versions of git silently ignored errors reading config files, but it was tightened in v1.8.1.1, as there can be quite serious implications to failing to read expected config (e.g., imagine transfer.fsckobjects, or receive.deny* is ignored). Yes, i agree, it's suboptimal but I for one would use getpwuid to get the home directory of the executing user to avoid this - though i don't know how portable it is (or if there is any other issues) We considered having git-daemon's --user option do that, but: 1. It would be a regression for people who are intentionally setting HOME to get different config profiles. And it would be a surprise to admins, as other user-switching daemons (e.g., inetd) do not tweak HOME. 2. It would not have covered all cases, including yours. xinetd is the one doing the user-switching here. -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: deletion of branch seems to modify tag as well?
On Wed, Jun 05, 2013 at 03:26:18PM +, Lyons, Roy wrote: $ git push origin :ABRANCHNAME remote: Firing Pre - receive hook remote: remote: remote: Firing Post receive hook remote: remote: Branch is ABRANCHNAME -- not creating Trigger file since this is not _int branch remote: error: Trying to write ref refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363 with nonexistent object remote: fatal: refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363: cannot update the ref To ssh://git@ourgitserver/repositoryname.git - [deleted] ABRANCHNAME I would not expect this behavior. Is git attempting to modify the tags associated to the HEAD commit on the branch in addition to the branch? If so, I would like to consider this a bug report... I do not think git is doing anything of the sort. The output you see above comes from custom hooks on the server. We cannot say for certain without seeing the hook's code, but it looks like the post-receive hook is trying to create a tag to point to the tip of every push, but whoever wrote the hook did not take into account branch deletions (and the fact that you cannot create a tag pointing at a deletion). -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: [PATCH] git-remote-mediawiki: use Git.pm functions for credentials
On Wed, Jun 05, 2013 at 12:58:00PM +0200, benoit.per...@ensimag.fr wrote: From: Benoit Person benoit.per...@ensimag.fr In 52dce6d, a new credential function was added to Git.pm, based on git-remote-mediawiki's functions. The logical follow-up is to use those functions in git-remote-mediawiki. Signed-off-by: Benoit Person benoit.per...@ensimag.fr Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr Thanks, this looks correct to me. -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: deletion of branch seems to modify tag as well?
Excellent response. I had not even considered that the hook for creating these tags would be in the mix. I withdraw my bug report happily :) Thanks, Roy Lyons On 6/5/13 10:51 AM, Jeff King p...@peff.net wrote: On Wed, Jun 05, 2013 at 03:26:18PM +, Lyons, Roy wrote: $ git push origin :ABRANCHNAME remote: Firing Pre - receive hook remote: remote: remote: Firing Post receive hook remote: remote: Branch is ABRANCHNAME -- not creating Trigger file since this is not _int branch remote: error: Trying to write ref refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363 with nonexistent object remote: fatal: refs/tags/ABRANCHNAME!SN-BL-20130605_100513_04363: cannot update the ref To ssh://git@ourgitserver/repositoryname.git - [deleted] ABRANCHNAME I would not expect this behavior. Is git attempting to modify the tags associated to the HEAD commit on the branch in addition to the branch? If so, I would like to consider this a bug report... I do not think git is doing anything of the sort. The output you see above comes from custom hooks on the server. We cannot say for certain without seeing the hook's code, but it looks like the post-receive hook is trying to create a tag to point to the tip of every push, but whoever wrote the hook did not take into account branch deletions (and the fact that you cannot create a tag pointing at a deletion). -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 rev-parse --show-toplevel doesn't work inside git-dir
On Wed, Jun 05, 2013 at 10:34:11AM +0300, Orgad Shaneh wrote: Running git rev-parse --show-toplevel doesn't print anything when it is run inside .git dir (on all levels) This is by design. --show-toplevel does not print anything when you do not have a working tree, and you do not have one if you are inside the .git directory. If you were to do: cd .git git --work-tree=.. rev-parse --show-toplevel (or use GIT_WORK_TREE, or set core.worktree, etc), you would get back the toplevel of the working tree. -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
Git server, chroot, git gc --auto and uname: not found
Hi, I'm having issues with a Git hosting in a chroot (based on fusion forge). The problem is that receive-pack triggers a git gc --auto, which itself triggers a git repack, which is a shell-script. The shell script needs basic commands [1], which are not available within the chroot. Is there a clean solution to this problem? I guess the right solution is send a patch that ports git-repack.sh to C (I thought we already had the server-side in C only, but just discovered it isn't the case), but I wont have time for that (not soon at least). Thanks, [1] uname, used in git-sh-setup, and then /bin/rm /usr/bin/find /bin/sed /usr/bin/tr /bin/mkdir /bin/chmod and /bin/mv according to strace. -- 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 v4 0/7] git send-email suppress-cc=self fixes
This includes bugfixes related to handling of --suppress-cc=self flag. Tests are also included. Changes from v3: - v3 submission was missing one patch (1/7). Re-add it. Changes from v2: - add a new test, split patches differently add code comments to address comments by Junio - rename example addresses in tests from redhat.com to example.com Changes from v1: - tweak coding style in tests to address comments by Junio Michael S. Tsirkin (7): t/send-email.sh: add test for suppress-cc=self send-email: fix suppress-cc=self on cccmd t/send-email: test suppress-cc=self on cccmd send-email: make --suppress-cc=self sanitize input t/send-email: add test with quoted sender t/send-email: test suppress-cc=self with non-ascii test-send-email: test for pre-sanitized self name git-send-email.perl | 23 ++-- t/t9001-send-email.sh | 75 +++ 2 files changed, 90 insertions(+), 8 deletions(-) -- MST -- 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/8] cherry-pick: add --skip-empty option
Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: You didn't answer, what happens when you run --skip-empty and --allow-empty? I'll answer to a slightly different question: What should happen? I think it should error out, because --allow-empty is about allowing empty commits to be preserved, and --skip-empty is about skipping empty commits (as it says on the package). Exactly, so they are related after all. However, --allow-empty says this: By default, cherry-picking an empty commit will fail, OK, that is a very good point. Especially because by the time reader reaches this new description, --allow-empty has already mentioned this, we can just be brief and it is sufficient to say Instead of failing, upfront. In fact, it might make sense to change the default in v2.0 to --empty-commits=skip. If it makes sense in 'git rebase', why doesn't it in 'git cherry-pick'? I think the primary reason behind the Why are you picking a no-op? Let me stop to make sure you didn't make a mistake. is because cherry-pick and revert have long been operations for a single commit explicitly given by the user, not bunch of commits in a range. These two operating modes are conceptually very different, even when we consider scripted use. A script that feeds one commit at a time has a chance to do patch equivalence or user-interactive filtering on its own, and would be helped by the are you sure you meant to record that no-op? check if it filtered in a wrong way (e.g. the user specified an empty commit by mistake). One that feeds a range, on the other hand, relies on or at least expects cherry-pick to have such smart, and a smarter cherry-pick could select what to pick from the given range. In the longer term, especially if we envision to move large part of logic to prepare the sequencer insn list from rebase to cherry-pick, a ranged cherry-pick should learn a way to filter the range with patch equivalence, for example. Once that happens, the behaviour would become inconsistent at the end-user level if we stopped at a commit only because it was not exactly patch equivalent to another one that is already on the branch we are cherry-picking to, but ended up to be a no-op, while we did not stop at another commit because patch equivalence filtered it out beforehand to skip it. Skipping a no-op by default would remove that inconsistency. So in that sense, one could argue that it may be a bugfix to skip commits that become no-op when replayed, when picking a range of commits (but not a single one). And I do not think you would need to wait until 2.0 for such a change. -- 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] gitweb: fix problem causing erroneous project list
Charles McGarvey chazmcgar...@brokenzipper.com writes:
The bug is manifest when running gitweb in a persistent process (e.g.
FastCGI, PSGI), and it's easy to reproduce. If a gitweb request
includes the searchtext parameter (i.e. s), subsequent requests using
the project_list action--which is the default action--and without
a searchtext parameter will be filtered by the searchtext value of the
first request. This is because the value of the $search_regexp global
(the value of which is based on the searchtext parameter) is currently
being persisted between requests.
Instead, clear $search_regexp before dispatching each request.
Signed-off-by: Charles McGarvey chazmcgar...@brokenzipper.com
---
I don't think there are currently any persistent-process gitweb tests to
copy from, so writing a test for this seems to be non-trivial.
The problem description makes sense to me, and clearing with undef
is in line with the case where the CGI is run for the first time, so
I think this patch is correct.
Will wait for a while to give Jakub some time to comment on (like:
Ack ;-) and then apply. Thanks.
By the way, I looked at how $search_regexp is used in the code:
* esc_html_match_hl and esc_html_match_hl__chopped (called from
git_project_list_rows, for example) want to have undef; an
empty string will not do.
* search_projects_list (called from git_project_list_body)
x git_search_files and git_search_grep_body assume that
$search_regexp can be interpolated in m//, which is not very
nice. They want an empty string.
So as an independent fix, the two subs may want to be fixed if we
want to be undef clean. Or am I missing something? Jakub, isn't
this kind of undef reference t9500 was designed to catch?
gitweb/gitweb.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 80950c0..8d69ada 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1086,7 +1086,7 @@ sub evaluate_and_validate_params {
our $search_use_regexp = $input_params{'search_use_regexp'};
our $searchtext = $input_params{'searchtext'};
- our $search_regexp;
+ our $search_regexp = undef;
if (defined $searchtext) {
if (length($searchtext) 2) {
die_error(403, At least two characters are required
for search parameter);
--
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 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
Am 04.06.2013 23:06, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: There are many instances where the treatment of symbolic links in the object model and the algorithms are tested, but where it is not necessary to actually have a symbolic link in the worktree. Make adjustments to the tests and remove the SYMLINKS prerequisite when appropriate in trivial cases, where trivial means: - merely a replacement of 'ln -s a b' to test_ln_s or of 'ln -s a b git add b' to test_ln_s_add is needed; - a test for symbolic link on the file system can be split off (and remains protected by SYMLINKS); - existing code is equivalent to test_ln_s[_add]. This is too big to review in one go, so I may have separate messages later on this same patch. diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh index ff163cf..bd17ba2 100755 --- a/t/t2003-checkout-cache-mkdir.sh +++ b/t/t2003-checkout-cache-mkdir.sh @@ -19,10 +19,10 @@ test_expect_success 'setup' ' git update-index --add path0 path1/file1 ' -test_expect_success SYMLINKS 'have symlink in place where dir is expected.' ' +test_expect_success 'have symlink in place where dir is expected.' ' rm -fr path0 path1 mkdir path2 -ln -s path2 path1 +test_ln_s path2 path1 git checkout-index -f -a test ! -h path1 test -d path1 test -f path1/file1 test ! -f path2/file1 I do not think this hunk is correct. [...] Under the precondition checkout-index runs in this test, a casual echo rezrov path1/file1 would leave path1 as a symlink without turning it into a real directory, and we will end up creating path2/file1. We are making sure that checkout-index does not behave that way, and it is essential to have symlink support in the working tree for the bug to trigger. @@ -79,10 +79,10 @@ test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' ' test -h tmp ' -test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' ' +test_expect_success 'use --prefix=tmp- where tmp-path1 is a symlink' ' rm -fr path0 path1 path2 tmp* mkdir tmp1 -ln -s tmp1 tmp-path1 +test_ln_s tmp1 tmp-path1 git checkout-index --prefix=tmp- -f -a test -f tmp-path0 test ! -h tmp-path1 This change has the same issue, I think. Yes, agreed. The converted tests -- when SYMLINKS are not available -- just repeat what is tested elsewhere. Nice catch. After I've found a hammer (test_ln_s) I was mindlessly looking for nails and found two of them in these two instances. ;-) -- 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: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
Am 04.06.2013 23:55, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh index 88be904..563ac7f 100755 --- a/t/t3000-ls-files-others.sh +++ b/t/t3000-ls-files-others.sh @@ -19,12 +19,7 @@ filesystem. test_expect_success 'setup ' ' date path0 -if test_have_prereq SYMLINKS -then -ln -s xyzzy path1 -else -date path1 -fi +test_ln_s xyzzy path1 mkdir path2 path3 path4 date path2/file2 date path2-junk This also is not appropriate, but it is not as bad as the one in t2003 I earlier commented on. This test wants an untracked symbolic link in the working tree as path1 and wants to see ls-files -o report it as other. On a filesystem that lack symbolic link, we obviously cannot have one, so as a substitute we just create another regular file to make the expected output and comparison simpler. Exactly. This is just a convenience. The issue is not introduced by the conversion, but dates back 4 years when I added the SYMLINKS check. We now only use a less random string on !SYMLINKS filesystems. test_expect_success 'git ls-files -k to show killed files.' ' date path2 -if test_have_prereq SYMLINKS -then -ln -s frotz path3 -ln -s nitfol path5 -else -date path3 -date path5 -fi +test_ln_s frotz path3 +test_ln_s nitfol path5 This falls into the same category as the one in t3000 above. The only thing that matters in this test is path3 and path5 are non directories so that the former is killed when path3/file3 needs to be checked out, while path5 will not appear in ls-files -k output. Ideally we would want to test regular files and symlinks as two different kinds of non directory filesystem entities, but on platforms that lack symbolic links we cannot do so, hence we punt and create a regular file. Indeed. Same answer. diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh -test_expect_success SYMLINKS 'diff identical, but newly created symlink and file' ' +test_expect_success 'diff identical, but newly created symlink and file' ' expected rm -f frotz nitfol echo xyzzy nitfol test-chmtime +10 nitfol -ln -s xyzzy frotz +test_ln_s xyzzy frotz git diff-index -M -p $tree current compare_diff_patch expected current As the point of test is to compare $tree that has symlink with another symlink that is identical but newly created one, I think this _does_ want the filesystem entity to be a symbolic link, but the index has frotz as a symlink and the mode propagates to what we read from the filesystem on !has_symlinks systems, so this conversion may be a correct one, though it is a bit tricky. Yes, this test depends on the mode propagation. I'll add a comment along these lines and keep the change in this patch with a title marked trivial cases. @@ -100,7 +103,7 @@ test_expect_success SYMLINKS 'diff different symlink and file' ' +yxyyz EOF rm -f frotz -ln -s yxyyz frotz +test_ln_s yxyyz frotz echo yxyyz nitfol git diff-index -M -p $tree current compare_diff_patch expected current Likewise. -- 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
[PATCH 1/4] clear parsed flag when we free tree buffers
Many code paths will free a tree object's buffer and set it
to NULL after finishing with it in order to keep memory
usage down during a traversal. However, out of 8 sites that
do this, only one actually unsets the parsed flag back.
Those sites that don't are setting a trap for later users of
the tree object; even after calling parse_tree, the buffer
will remain NULL, causing potential segfaults.
It is not known whether this is triggerable in the current
code. Most commands do not do an in-memory traversal
followed by actually using the objects again. However, it
does not hurt to be safe for future callers.
In most cases, we can abstract this out to a
free_tree_buffer helper. However, there are two
exceptions:
1. The fsck code relies on the parsed flag to know that we
were able to parse the object at one point. We can
switch this to using a flag in the flags field.
2. The index-pack code sets the buffer to NULL but does
not free it (it is freed by a caller). We should still
unset the parsed flag here, but we cannot use our
helper, as we do not want to free the buffer.
Signed-off-by: Jeff King p...@peff.net
---
This shouldn't have any behavior change, but I'd worry a bit that I
missed some case in builtin/fsck.c where the new HAS_OBJ flag would need
set.
builtin/fsck.c | 17 -
builtin/index-pack.c | 1 +
builtin/reflog.c | 3 +--
http-push.c | 3 +--
list-objects.c | 3 +--
reachable.c | 3 +--
revision.c | 3 +--
tree.c | 8
tree.h | 1 +
walker.c | 5 +
10 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index bb9a2cd..579fdcc 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -16,6 +16,7 @@
#define REACHABLE 0x0001
#define SEEN 0x0002
+#define HAS_OBJ 0x0004
static int show_root;
static int show_tags;
@@ -101,7 +102,7 @@ static int mark_object(struct object *obj, int type, void
*data)
if (obj-flags REACHABLE)
return 0;
obj-flags |= REACHABLE;
- if (!obj-parsed) {
+ if (!(obj-flags HAS_OBJ)) {
if (parent !has_sha1_file(obj-sha1)) {
printf(broken link from %7s %s\n,
typename(parent-type),
sha1_to_hex(parent-sha1));
@@ -127,16 +128,13 @@ static int traverse_one_object(struct object *obj)
struct tree *tree = NULL;
if (obj-type == OBJ_TREE) {
- obj-parsed = 0;
tree = (struct tree *)obj;
if (parse_tree(tree) 0)
return 1; /* error already displayed */
}
result = fsck_walk(obj, mark_object, obj);
- if (tree) {
- free(tree-buffer);
- tree-buffer = NULL;
- }
+ if (tree)
+ free_tree_buffer(tree);
return result;
}
@@ -178,7 +176,7 @@ static void check_reachable_object(struct object *obj)
* except if it was in a pack-file and we didn't
* do a full fsck
*/
- if (!obj-parsed) {
+ if (!(obj-flags HAS_OBJ)) {
if (has_sha1_pack(obj-sha1))
return; /* it is in pack - forget about it */
printf(missing %s %s\n, typename(obj-type),
sha1_to_hex(obj-sha1));
@@ -306,8 +304,7 @@ static int fsck_obj(struct object *obj)
if (obj-type == OBJ_TREE) {
struct tree *item = (struct tree *) obj;
- free(item-buffer);
- item-buffer = NULL;
+ free_tree_buffer(item);
}
if (obj-type == OBJ_COMMIT) {
@@ -340,6 +337,7 @@ static int fsck_sha1(const unsigned char *sha1)
return error(%s: object corrupt or missing,
sha1_to_hex(sha1));
}
+ obj-flags |= HAS_OBJ;
return fsck_obj(obj);
}
@@ -352,6 +350,7 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum
object_type type,
errors_found |= ERROR_OBJECT;
return error(%s: object corrupt or missing,
sha1_to_hex(sha1));
}
+ obj-flags = HAS_OBJ;
return fsck_obj(obj);
}
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 79dfe47..20cf284 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -765,6 +765,7 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
if (obj-type == OBJ_TREE) {
struct tree *item = (struct tree *) obj;
item-buffer = NULL;
+ obj-parsed = 0;
}
if (obj-type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 54184b3..ba27f7c 100644
--- a/builtin/reflog.c
+++
[PATCH 4/4] archive: ignore blob objects when checking reachability
We cannot create an archive from a blob object, so we would not expect anyone to provide one to us. And if they do, we will fail anyway just after the reachability check. We can therefore optimize our reachability check to ignore blobs completely, and not even create a struct blob for them. Depending on the repository size and the exact place we find the reachable object in the traversal, this can save 20-25%, a we can avoid many lookups in the object hash. The downside of this is that a blob provided to a remote archive process will fail with no such object rather than object is not a tree (we could organize the code to retain the old message, but since we no longer know whether the blob is reachable or not, we would potentially be leaking information about the existence of unreachable objects). Signed-off-by: Jeff King p...@peff.net --- archive.c | 1 + 1 file changed, 1 insertion(+) diff --git a/archive.c b/archive.c index 4d77624..98691cd 100644 --- a/archive.c +++ b/archive.c @@ -290,6 +290,7 @@ static int object_is_reachable(const unsigned char *sha1) save_commit_buffer = 0; init_revisions(data.revs, NULL); setup_revisions(ARRAY_SIZE(argv) - 1, argv, data.revs, NULL); + data.revs.blob_objects = 0; if (prepare_revision_walk(data.revs)) return 0; -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] list-objects: optimize revs-blob_objects = 0 case
If we are traversing trees during a --objects traversal, we may skip blobs if the blob_objects field of rev_info is not set. But we do so as the first thing in process_blob(), only after we have actually created the struct blob object, incurring a hash lookup. We can optimize out this no-op call completely. This does not actually affect any current code, as all of the current traversals always set blob_objects when looking at objects, anyway. Signed-off-by: Jeff King p...@peff.net --- list-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/list-objects.c b/list-objects.c index c8c3463..77e6ec5 100644 --- a/list-objects.c +++ b/list-objects.c @@ -116,7 +116,7 @@ static void process_tree(struct rev_info *revs, process_gitlink(revs, entry.sha1, show, me, entry.path, cb_data); - else + else if (revs-blob_objects) process_blob(revs, lookup_blob(entry.sha1), show, me, entry.path, -- 1.8.3.rc2.14.g7eee6b3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] upload-archive: restrict remote objects with reachability check
When serving a remote request, git-upload-archive tries to
restrict access to unreachable objects, which matches the
behavior of upload-pack. However, we did so by restricting
the requested tree to ref[:path], because it is fast.
That covers the common cases, but does not allow requesting
items by a specific sha1 (either a tree or a commit sha1).
Instead, let's do the correct-but-slower method of actually
walking back from the tips to see if the requested object is
reachable. The performance impact of this is roughly:
1. For a recent commit, the speed is about the same (we
traverse in reverse chronological order, so we see it
almost immediately).
2. For an older commit, even one pointed at directly by a
ref (e.g., an old tag), we are slower, because we
traverse from the more recent tips. We are bounded in
this case by the time to look at all commits (i.e.,
time git rev-list --all).
3. When we see $ref:$path, we typically perform much
worse, because our traversal looks at all commits
first, followed by all trees.
4. The worst case (which we hit for an unreachable object)
is equivalent to time rev-list --objects --all, which
is about the same amount of time pack-objects spends
preparing a full clone (which can be in the tens of
seconds for a large repository).
The implementation is a fairly straightforward application
of the traverse_commit_list function. Using the
mark_objects_reachable function would seem more appropriate,
but it has no mechanism for looking for a specific object,
which lets us end the traversal early in common cases.
Signed-off-by: Jeff King p...@peff.net
---
The slowdown from points (2) and (3) makes me hesitate on this. We can
address (2) by checking if the get_sha1 lookup used a refname
explicitly, but I'm no sure about (3).
archive.c | 70 +++--
t/t5005-archive-resolution.sh | 91 +++
2 files changed, 150 insertions(+), 11 deletions(-)
create mode 100755 t/t5005-archive-resolution.sh
diff --git a/archive.c b/archive.c
index d254fa5..4d77624 100644
--- a/archive.c
+++ b/archive.c
@@ -5,6 +5,9 @@
#include archive.h
#include parse-options.h
#include unpack-trees.h
+#include diff.h
+#include revision.h
+#include list-objects.h
static char const * const archive_usage[] = {
N_(git archive [options] tree-ish [path...]),
@@ -241,6 +244,59 @@ static void parse_pathspec_arg(const char **pathspec,
}
}
+struct reachable_object_data {
+ struct rev_info revs;
+ struct object *obj;
+};
+
+static void check_object(struct object *obj, const struct name_path *path,
+const char *name, void *vdata)
+{
+ struct reachable_object_data *data = vdata;
+ /*
+* We found it; the caller will take care of marking it SEEN,
+* but we can end the traversal early.
+*/
+ if (obj == data-obj) {
+ free_commit_list(data-revs.commits);
+ data-revs.commits = NULL;
+
+ free(data-revs.pending.objects);
+ data-revs.pending.nr = 0;
+ data-revs.pending.alloc = 0;
+ data-revs.pending.objects = NULL;
+ }
+}
+
+static void check_commit(struct commit *commit, void *vdata)
+{
+ check_object(commit-object, NULL, NULL, vdata);
+}
+
+static int object_is_reachable(const unsigned char *sha1)
+{
+ static const char *argv[] = {
+ rev-list,
+ --objects,
+ --all,
+ NULL
+ };
+ struct reachable_object_data data;
+
+ data.obj = parse_object(sha1);
+ if (!data.obj)
+ return 0;
+
+ save_commit_buffer = 0;
+ init_revisions(data.revs, NULL);
+ setup_revisions(ARRAY_SIZE(argv) - 1, argv, data.revs, NULL);
+ if (prepare_revision_walk(data.revs))
+ return 0;
+
+ traverse_commit_list(data.revs, check_commit, check_object, data);
+ return data.obj-flags SEEN;
+}
+
static void parse_treeish_arg(const char **argv,
struct archiver_args *ar_args, const char *prefix,
int remote)
@@ -252,20 +308,12 @@ static void parse_treeish_arg(const char **argv,
const struct commit *commit;
unsigned char sha1[20];
- /* Remotes are only allowed to fetch actual refs */
- if (remote) {
- char *ref = NULL;
- const char *colon = strchr(name, ':');
- int refnamelen = colon ? colon - name : strlen(name);
-
- if (!dwim_ref(name, refnamelen, sha1, ref))
- die(no such ref: %.*s, refnamelen, name);
- free(ref);
- }
-
if (get_sha1(name, sha1))
die(Not a valid object name);
+ if (remote !object_is_reachable(sha1))
+ die(Not a valid object name);
+
commit =
Re: [PATCH 08/15] for-each-ref: get --pretty using format_commit_message
On Thu, Jun 6, 2013 at 12:09 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jun 5, 2013 at 4:12 AM, Eric Sunshine sunsh...@sunshineco.com wrote: +Caveats: + +1. Many of the placeholders in PRETTY FORMATS are designed to work + specifically on commit objects: when non-commit objects are + supplied, those placeholders won't work. Should won't work be expanded upon? It's not clear if this means that git will outright crash, or if it will abort with an appropriate error message, or if the directive will be displayed as-is or removed from the output. It will be displayed as-is but that's a bit inconsistent: %(unknown) prints error and aborts while %unknown simply produces %unknown. The latter is how git log --format does it. But I think we could make for-each-ref --pretty to do the former for %unknown. It'll be consistent with %(unknown) and we do not need to elaborate much (it's pretty obvious when it does not work). The Caveat Eric is asking about talks about what happens to a %(field) that only makes sense for a commit when showing a ref pointing at a non-commit?, but you are answering what happend to a %(invalidfield) that is not defined, aren't you? Because %(field) is treated like %(invalidfield) in this case. I'm not saying this is the best thing to do though. IIRC, the reason we show literal from log --format is to make it easier for the person who misspelt %placeholder to spot it in the output, and also make it easier for the person who use %placeholder meant for newer versions of Git with an older one. It would be a bit unnice to die for the latter, especially if the format string is in a script or something. That makes more sense for for-each-ref than log because for-each-ref is a plumbing and should support scripting. But old for-each-ref will happily reject %(newplacholder) right away. Should we take this opportunity to change this behavior in for-each-ref too? To log --format, all input objects are expected to be commits, so it does not have the what does %(authordate) give when given a blob issue. But for for-each-ref --format, it is perfectly normal that you may feed a non-commit; it makes the mechanism unusable if you errored out %(authordate) when showing a ref that points at a tag, doesn't it? Substituting an inapplicable placeholder with an empty string would be an easies way out, unless it learns a flexible/elaborate conditional formatting mechanism, I would think. There is a blurred line here. %H (or %h) should produce an object name even for tags or blobs, but right now it produces %H instead. The same applies for %ad and friends, but these are clearly for commits and should probably behave like %(authordate), i.e. producing empty string. -- Duy -- 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] l10n: de.po: Fix a typo
From: Wieland Hoffmann themi...@gmail.com Signed-off-by: Wieland Hoffmann themi...@gmail.com --- po/de.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/de.po b/po/de.po index 4901488..cd6919f 100644 --- a/po/de.po +++ b/po/de.po @@ -885,7 +885,7 @@ msgstr[1] Ihr Zweig ist vor '%s' um %d Versionen.\n #: remote.c:1787 msgid (use \git push\ to publish your local commits)\n -msgstr (benutzen Sie \git push\ um lokalen Versionen herauszubringen)\n +msgstr (benutzen Sie \git push\ um lokale Versionen herauszubringen)\n #: remote.c:1790 #, c-format -- 1.8.2.999.gd2ad0c7 -- 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/8] cherry-pick: add --skip-empty option
On Wed, Jun 5, 2013 at 1:13 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: You didn't answer, what happens when you run --skip-empty and --allow-empty? I'll answer to a slightly different question: What should happen? I think it should error out, because --allow-empty is about allowing empty commits to be preserved, and --skip-empty is about skipping empty commits (as it says on the package). Exactly, so they are related after all. However, --allow-empty says this: By default, cherry-picking an empty commit will fail, OK, that is a very good point. Especially because by the time reader reaches this new description, --allow-empty has already mentioned this, we can just be brief and it is sufficient to say Instead of failing, upfront. In fact, it might make sense to change the default in v2.0 to --empty-commits=skip. If it makes sense in 'git rebase', why doesn't it in 'git cherry-pick'? I think the primary reason behind the Why are you picking a no-op? Let me stop to make sure you didn't make a mistake. is because cherry-pick and revert have long been operations for a single commit explicitly given by the user, not bunch of commits in a range. Yeah, but this has changed already. These two operating modes are conceptually very different, even when we consider scripted use. A script that feeds one commit at a time has a chance to do patch equivalence or user-interactive filtering on its own, and would be helped by the are you sure you meant to record that no-op? check if it filtered in a wrong way (e.g. the user specified an empty commit by mistake). One that feeds a range, on the other hand, relies on or at least expects cherry-pick to have such smart, and a smarter cherry-pick could select what to pick from the given range. How would a script know that a single pick ends up as a no-op? It can't know what is the reason the cherry-pick failed. The only way to know for sure is to check the last commit, and for that we don't need the last cherry-pick to fail. In the longer term, especially if we envision to move large part of logic to prepare the sequencer insn list from rebase to cherry-pick, a ranged cherry-pick should learn a way to filter the range with patch equivalence, for example. Once that happens, the behaviour would become inconsistent at the end-user level if we stopped at a commit only because it was not exactly patch equivalent to another one that is already on the branch we are cherry-picking to, but ended up to be a no-op, while we did not stop at another commit because patch equivalence filtered it out beforehand to skip it. Skipping a no-op by default would remove that inconsistency. So in that sense, one could argue that it may be a bugfix to skip commits that become no-op when replayed, when picking a range of commits (but not a single one). And I do not think you would need to wait until 2.0 for such a change. Right. But first we need to agree that failing an empty cherry-pick serves no purpose. -- 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
