[PATCH] Fix build with LDFLAGS=-Wl,--as-needed
Signed-off-by: Lars Wendler polynomia...@gentoo.org --- contrib/svn-fe/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile index e8651aa..b90cf87 100644 --- a/contrib/svn-fe/Makefile +++ b/contrib/svn-fe/Makefile @@ -74,7 +74,7 @@ endif endif svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB) - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o $(LIBS) + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ svn-fe.o $(LIBS) $(EXTLIBS) svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h $(QUIET_CC)$(CC) $(CFLAGS) -I../../vcs-svn -o $*.o -c $ -- 2.2.0 -- 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 add -i: allow list (un)selection by regexp
Hello! This patch makes it possible to select or unselect files in `git add -i` by regular expression instead of unique prefix only. The command syntax is `/foo` for selection and `-/foo` for unselection. I don't think the syntax will conflict with any existing use cases, but feel free to prove me wrong. I'm not a Perl programmer, but I've tried to follow the style of the existing code as much as possible. :) Note I'm currently not on the mailing list, so please cc. Best regards, Aarni Koskela From 53c12d9c9928dc93a57595e92d785ecc0b245390 Mon Sep 17 00:00:00 2001 From: Aarni Koskela a...@iki.fi Date: Mon, 1 Dec 2014 11:06:10 +0200 Subject: [PATCH] git-add--interactive: allow list (un)selection by regular expression Teach `list_and_choose` to allow `/regexp` and `-/regexp` syntax to select items based on regular expression match. For instance, `/jpg$` will select all options whose display name ends with `jpg`. Signed-off-by: Aarni Koskela a...@iki.fi --- git-add--interactive.perl | 27 +++ 1 file changed, 27 insertions(+) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 1fadd69..34cc603 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -483,6 +483,8 @@ sub is_valid_prefix { !($prefix =~ /[\s,]/) # separators !($prefix =~ /^-/) # deselection !($prefix =~ /^\d+/) # selection + !($prefix =~ /^\//)# regexp selection + !($prefix =~ /^-\//) # regexp unselection ($prefix ne '*') # all wildcard ($prefix ne '?');# prompt help } @@ -585,6 +587,28 @@ sub list_and_choose { prompt_help_cmd(); next TOPLOOP; } + if ($line =~ /^(-)*\/(.+)$/) { + my $choose = !($1 $1 eq '-'); + my $re = $2; + my $found = 0; + for ($i = 0; $i @stuff; $i++) { + my $val = $stuff[$i]; + my $ref = ref $val; + if ($ref eq 'ARRAY') { + $val = $val-[0]; + } + elsif ($ref eq 'HASH') { + $val = $val-{VALUE}; + } + if ($val =~ /$re/) { + $chosen[$i] = $choose; + $found = 1; + last if $opts-{SINGLETON}; + } + } + last if $found ($opts-{IMMEDIATE}); + next TOPLOOP; + } for my $choice (split(/[\s,]+/, $line)) { my $choose = 1; my ($bottom, $top); @@ -635,6 +659,7 @@ sub singleton_prompt_help_cmd { Prompt help: 1 - select a numbered item foo- select item based on unique prefix +/regexp- select item based on regular expression - (empty) select nothing EOF } @@ -648,6 +673,8 @@ Prompt help: foo- select item based on unique prefix -... - unselect specified items * - choose all items +/regexp- select items based on regular expression +-/regexp - unselect items based on regular expression - (empty) finish selecting EOF } -- 1.9.2.msysgit.0 -- 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-svn: Support for git-svn propset
Alfred Perlstein alf...@freebsd.org wrote: This change allows git-svn to support setting subversion properties. Very useful for manually setting properties when committing to a subversion repo that *requires* properties to be set without requiring moving your changeset to separate subversion checkout in order to set props. Thanks Alfred, that's a good reason for supporting this feature (something I wasn't convinced of with the original). This change is initially from David Fraser davidf () sjsoft ! com Appearing here: http://marc.info/?l=gitm=125259772625008w=2 I've added David to the Cc: (but I never heard back from him regarding comments from his original patch). Alfred: I had some comments on David's original patch here that were never addressed: http://mid.gmane.org/20090923085812.ga20...@dcvr.yhbt.net I suspect my concerns about .gitattributes in the source tree from 2009 are less relevant now in 2014 as git-svn seems more socially acceptable in SVN-using projects. Some of my other concerns about mimicking existing Perl style still apply, and we have a Perl section in Documenting/CodingGuidelines nowadays. They are now forward ported to most recent git along with fixes to deal with files in subdirectories. Developer's Certificate of Origin 1.1 snip no need for the whole DCO in the commit message. just the S-o-b: Signed-off-by: Alfred Perlstein alf...@freebsd.org Since this was originally written by David, his sign-off from the original email should be here (Cc: for bigger changes) --- git-svn.perl | 50 +- perl/Git/SVN/Editor.pm | 47 +++ We need a test case written for this new feature. Most folks (including myself) who were ever involved with git-svn rarely use it anymore, and this will likely be rarely-used. 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/git-svn.perl b/git-svn.perl index b6e2186..91423a8 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -115,7 +115,7 @@ my ($_stdin, $_help, $_edit, $_before, $_after, $_merge, $_strategy, $_preserve_merges, $_dry_run, $_parents, $_local, $_prefix, $_no_checkout, $_url, $_verbose, - $_commit_url, $_tag, $_merge_info, $_interactive); + $_commit_url, $_tag, $_merge_info, $_interactive, $_set_svn_props); # This is a refactoring artifact so Git::SVN can get at this git-svn switch. sub opt_prefix { return $_prefix || '' } @@ -193,6 +193,7 @@ my %cmd = ( 'dry-run|n' = \$_dry_run, 'fetch-all|all' = \$_fetch_all, 'commit-url=s' = \$_commit_url, + 'set-svn-props=s' = \$_set_svn_props, 'revision|r=i' = \$_revision, 'no-rebase' = \$_no_rebase, 'mergeinfo=s' = \$_merge_info, @@ -228,6 +229,9 @@ my %cmd = ( 'propget' = [ \cmd_propget, 'Print the value of a property on a file or directory', { 'revision|r=i' = \$_revision } ], +'propset' = [ \cmd_propset, +'Set the value of a property on a file or directory - will be set on commit', +{} ], 'proplist' = [ \cmd_proplist, 'List all properties of a file or directory', { 'revision|r=i' = \$_revision } ], @@ -1376,6 +1380,50 @@ sub cmd_propget { print $props-{$prop} . \n; } +# cmd_propset (PROPNAME, PROPVAL, PATH) +# +# Adjust the SVN property PROPNAME to PROPVAL for PATH. +sub cmd_propset { + my ($propname, $propval, $path) = @_; + $path = '.' if not defined $path; + $path = $cmd_dir_prefix . $path; + usage(1) if not defined $propname; + usage(1) if not defined $propval; + my $file = basename($path); + my $dn = dirname($path); + # diff has check_attr locally, so just call direct + #my $current_properties = check_attr( svn-properties, $path ); I prefer leaving out dead code entirely instead of leaving it commented out. + my $current_properties = Git::SVN::Editor::check_attr( svn-properties, $path ); + my $new_properties = ; Since some lines are too long, local variables can be shortened to cur_props, new_props without being any less descriptive. + if ($current_properties eq unset || $current_properties eq || $current_properties eq set) { + $new_properties = $propname=$propval; + } else { + # TODO: handle combining properties better + my @props = split(/;/, $current_properties); + my $replaced_prop = 0; + foreach my $prop (@props) { + # Parse 'name=value' syntax and set the property. + if ($prop =~ /([^=]+)=(.*)/) { + my ($n,$v) = ($1,$2); +
Re: [PATCH 24/34] checkout: reject if the branch is already checked out elsewhere
On Sun, Nov 30, 2014 at 12:18:40PM -0500, Mark Levedahl wrote: On 11/30/2014 03:24 AM, Nguyễn Thái Ngọc Duy wrote: One branch obviously can't be checked out at two places (but detached heads are ok). Give the user a choice in this case: --detach, -b new-branch, switch branch in the other checkout first or simply 'cd' and continue to work there. This seems too restrictive and is not obvious to me: I currently use git-new-workdir to have multiple checkouts of the same branch, with no ill effect. While those who do not understand what is going on underneath might be confused by one checkout suddenly showing uncommitted diffs, I don't accept that as a reason to outright prevent such use. I suggest, at the very least, that this behavior be overridden by a --force flag? Prevention is a strong word. It's more about safety for the mere mortals. It's certainly possible to do something like this patch (--force can't be reused, it already carries some other meaning). Should I add this one in the next (hic) reroll? -- 8 -- Subject: [PATCH] checkout: add --ignore-other-wortrees Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-checkout.txt | 6 ++ builtin/checkout.c | 19 +++ t/t2025-checkout-to.sh | 7 +++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 0c13825..71d9e4e 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -232,6 +232,12 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode. specific files such as HEAD, index... See MULTIPLE WORKING TREES section for more information. +--ignore-other-worktrees:: + `git checkout` refuses when the wanted ref is already checked out + by another worktree. This option makes it check the + ref out anyway. In other words, the ref is held by more than one + worktree. + branch:: Branch to checkout; if it refers to a branch (i.e., a name that, when prepended with refs/heads/, is a valid ref), then that diff --git a/builtin/checkout.c b/builtin/checkout.c index 953b763..8b2bf20 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -37,6 +37,7 @@ struct checkout_opts { int writeout_stage; int overwrite_ignore; int ignore_skipworktree; + int ignore_other_worktrees; const char *new_branch; const char *new_branch_force; @@ -1079,11 +1080,12 @@ static void check_linked_checkouts(struct branch_info *new) static int parse_branchname_arg(int argc, const char **argv, int dwim_new_local_branch_ok, struct branch_info *new, - struct tree **source_tree, - unsigned char rev[20], - const char **new_branch, - int force_detach) + struct checkout_opts *opts, + unsigned char rev[20]) { + struct tree **source_tree = opts-source_tree; + const char **new_branch = opts-new_branch; + int force_detach = opts-force_detach; int argcount = 0; unsigned char branch_rev[20]; const char *arg; @@ -1209,7 +1211,8 @@ static int parse_branchname_arg(int argc, const char **argv, int flag; char *head_ref = resolve_refdup(HEAD, 0, sha1, flag); if (head_ref - (!(flag REF_ISSYMREF) || strcmp(head_ref, new-path))) + (!(flag REF_ISSYMREF) || strcmp(head_ref, new-path)) + !opts-ignore_other_worktrees) check_linked_checkouts(new); free(head_ref); } @@ -1340,6 +1343,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) N_(second guess 'git checkout no-such-branch')), OPT_FILENAME(0, to, opts.new_worktree, N_(check a branch out in a separate working directory)), + OPT_BOOL(0, ignore-other-worktrees, opts.ignore_other_worktrees, +N_(do not check if another worktree is holding the given ref)), OPT_END(), }; @@ -1420,9 +1425,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.track == BRANCH_TRACK_UNSPECIFIED !opts.new_branch; int n = parse_branchname_arg(argc, argv, dwim_ok, -new, opts.source_tree, -rev, opts.new_branch, -opts.force_detach); +new, opts, rev); argv += n; argc -= n; }
Re: [PATCH/RFC v2] Squashed changes for multiple worktrees vs. submodules
On Mon, Dec 1, 2014 at 6:27 AM, Max Kirillov m...@max630.net wrote: But, while hacking the submodule init I became more convinced that the modules directory should be common and submodules in checkout should be a checkouts of the submodule. Because this is looks like concept of submodules, that they are unique for the lifetime of repository, even if they do not exist in all revisions. And if anybody want to use fully independent checkout they can be always checked out manually. Actually, after a submodule is initialized and have a proper gitlink, it can be updated and inquired regardless of where it points to. Just throw something in for discussion. What about keeping $GIT_DIR/modules like it is now (i.e. not shared) and add $GIT_DIR/shared-modules, which is the same for all checkouts? That would keep current submodule code happy (no name collision or anything). New submodule code can start using $GIT_DIR/shared-modules while still keeping an eye on $GIT_DIR/modules for old setups. -- 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
[BUG] Documentation: git log: --exit-code undocumented?
Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? -- Sergey. -- 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] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote: On 12/01/2014 04:40 AM, David Michael wrote: On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de wrote: [snip] Could the code be more human-readable ? static inline mode_t mode_native_to_git(mode_t native_mode) { int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; if (S_ISBLK(native_mode)) return 006 | perm_bits; if (S_ISCHR(native_mode)) return 002 | perm_bits; if (S_ISFIFO(native_mode)) return 001 | perm_bits; /* Non-standard type bits were given. */ /* Shouldn't we die() here ?? */ return perm_bits; } Sure, I can send an updated patch with the new variable and without the elses. Regarding the question in the last comment: I was assuming if this case was ever reached, Git would handle the returned mode the same way as if it encountered an unknown/non-standard file type on a normal operating system, which could die() if needed in the function that called stat(). Should I send an updated patch that die()s there? David Not yet, please wait with a V2 patch until I finished my thinking ;-) I take back the suggestion with the die(). I was thinking how to handle unforeseen types, which may show up on the z/OS some day, So die() is not a good idea, it is better to ignore them, what the code does. Knowing that Git does not track block devices, nor character devices nor sockets, the above code could be simplyfied even more, by mapping everything which is not a directory, a file or a softlink to device type 0) This is just a suggestion, I want to here from others as well: int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK */ return perm_bits; I had considered omitting those three as well at first, but in this case it would mean that they will be unusable all together. The z/OS S_IFMT definition (i.e. the file type bit mask) is 0xFF00, and the common/translated S_IFMT definition is 0xF000. Since the S_ISxxx macros use the typical ((mode S_IFMT) == S_IFxxx) definition, they would never match a native z/OS mode after redefining S_IFMT. So translating those types isn't just for tracking files, it's to support any use of S_ISxxx anywhere in the code. It should be okay to remove any of those types if we know that Git will never need to use them. David -- 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] git rebase -X fails with merge-recursive error
Hi, I discovered this while rebasing a branch after having converted files to use LF line endings. I got around the problem by using --ignore-whitespace but the error still seems strange to me so I'm reporting it. The following script is equivalent: it creates a repo with a CRLF file, creates a feature branch, converts the file and then tries to rebase the feature branch with git rebase -X ignore-whitespace-at-eol: git init echo That's some catch, that Catch-1984 file.txt unix2dos file.txt git add file.txt git commit -m 'Initial commit' git checkout -b feature ex -sc 's/1984/22/|x' file.txt git commit -m Fix literary confusion file.txt git checkout master echo '* text=auto' .gitattributes dos2unix file.txt git add . git commit -m 'CRLF to LF' git checkout feature gti config merge.renormalize true git rebase -X ignore-space-at-eol master While rebasing the following error appears: error: addinfo_cache failed for path 'file.txt' It comes from the git-merge-recursive invocation git-rebase performs[1] and is printed because make_cache_entry() fails[2]. I also get this error when using the other 'ignore-*' strategy options, as well as 'theirs'. At this point file.txt still has its CRLF terminator, but simply: git add file.txt git rebase --continue has the desired effect (as it did in my real life situation). I've tested this on Git 2.2.0, 2.1.3 and 1.9.0. I don't know whether this is actually related to git-rebase. It's a way to reproduce it if nothing else. I haven't tried any experiments with git-merge-recursive directly, I get the impression I'm not supposed to since it doesn't have a manpage ;) Interestingly, removing the creation of .gitattributes makes the problem go away. Additionally, git-merge-recursive exits with status 0. This confuses git-rebase which will continue and then complain about the state of the index. (Interestingly, at this point my prompt thingy complains that .git/rebase-merge/done isn't there). Regards, Øsse [1]: https://github.com/git/git/blob/master/git-rebase--merge.sh#L70 [2]: https://github.com/git/git/blob/master/merge-recursive.c#L209 -- 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
HIstory simplification limited to requested refs ?
Hello, I'm trying to get gitk to draw the relationship between a bunch of integration branches. What I'm looking for is a graph that would show which of those branches is included by others. --simplify-by-decoration nearly does the job, but does not omit those heads merged into the requested branches. Did I miss a particular git-log flag that would allow something like --simplify-by-commandline ? Best regards, -- Yann -- 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] compat: convert modes to use portable file type values
On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? + buf-st_mode = mode_native_to_git(buf-st_mode); + return rc; +} -- 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: [PATCH/RFC v2] Squashed changes for multiple worktrees vs. submodules
On Mon, Dec 01, 2014 at 05:43:16PM +0700, Duy Nguyen wrote: On Mon, Dec 1, 2014 at 6:27 AM, Max Kirillov m...@max630.net wrote: But, while hacking the submodule init I became more convinced that the modules directory should be common and submodules in checkout should be a checkouts of the submodule. Because this is looks like concept of submodules, that they are unique for the lifetime of repository, even if they do not exist in all revisions. And if anybody want to use fully independent checkout they can be always checked out manually. Actually, after a submodule is initialized and have a proper gitlink, it can be updated and inquired regardless of where it points to. Just throw something in for discussion. What about keeping $GIT_DIR/modules like it is now (i.e. not shared) and add $GIT_DIR/shared-modules, which is the same for all checkouts? That would keep current submodule code happy (no name collision or anything). New submodule code can start using $GIT_DIR/shared-modules while still keeping an eye on $GIT_DIR/modules for old setups. I think it would be too complicated. To make fancy think user can always manually initialize a repository or a checkout. And all sumbodule functionality except adding/removing/(de)initialization should work with any repository or gitlink, regardless of where it points to. -- 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 in reflog of length 0x2BFF
Hi, I encountered a strange bug concerning the reflog. I suspect some kind of out-of-bounds access. The symptom is: %git rev-parse 'master@{52}' warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970 00:00:01 +. 0036 Try the following: git init gitbug cd gitbug git commit --allow-empty -m a cp ../reflog.bad .git/logs/refs/heads/master git rev-parse 'master@{52}' The source of cp is the attached file. This is from a reflog of stash. I just replaced all stuff by dummy values. This does not seem to affect the bug. Sorry, it must be this long. Some observations: * If you change the length of any line starting at line 3, the symptom vanishes. (The X at the line ends are free-form text.) * Starting at line three, there are 0x2BFF bytes till the end of file. Is there some dynamically growing buffer, which at some point reaches the size 0x2C00? * Changing the length of the first two lines has no effect. Is the file read backwards? * It happens at least with git 2.1.2 (amd64) and 2.2.0 (ia32). * 2.0.2 (amd64) and 2.1.0 (amd64) seem not to have this bug. Any ideas? Christoph 0037 0036 xxx@xxx 01 + X 0036 0035 xxx@xxx 01 + X 0035 0034 xxx@xxx 01 + 0034 0033 xxx@xxx 01 + XXX 0033 0032 xxx@xxx 01 + XX 0032 0031 xxx@xxx 01 + X 0031 0030 xxx@x 01 + XX 0030 002f xxx@x 01 + XXX 002f 002e xxx@x 01 + XXX 002e 002d xxx@x 01 + 002d 002c xxx@x 01 + XXX 002c 002b xxx@x 01 + XXX 002b 002a xxx@x 01 + X 002a 0029 xxx@x 01 + XXX 0029 0028 xxx@x 01 + XX 0028 0027 xxx@x 01 + XXX 0027 0026 xxx@x 01 + XXX 0026 0025 xxx@x 01 + XXX 0025 0024 xxx@x
Re: [PATCH] Fix build with LDFLAGS=-Wl,--as-needed
Lars Wendler polynomia...@gentoo.org writes: Subject: Re: [PATCH] Fix build with LDFLAGS=-Wl,--as-needed Please identify what area you are touching, e.g. Subject: contrib/svn-fe: avoid early $(EXTLIBS) on linker invocation or whatever. Fix build does not tell us why this change is needed; it does not say what breaks, how and most importantly why it breaks. Please have explanation in the commit message body, e.g. When attempting to build svn-fe with LDFLAGS=-Wl,--as-needed, I noticed that ... breaks in such and such way. This is because you must not have X before Y due to Z. Fix this by doing A, B and C, which makes sure X comes after Y. or somesuch. Signed-off-by: Lars Wendler polynomia...@gentoo.org --- contrib/svn-fe/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile index e8651aa..b90cf87 100644 --- a/contrib/svn-fe/Makefile +++ b/contrib/svn-fe/Makefile @@ -74,7 +74,7 @@ endif endif svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB) - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o $(LIBS) + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ svn-fe.o $(LIBS) $(EXTLIBS) I do not understand the original, which has EXTLIBS = GIT_LIB = ../../libgit.a VCSSVN_LIB = ../../vcs-svn/lib.a LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS) i.e. it shouldn't be necessary to explicitly list EXTLIBS, as it already has LIBS at the end, which in turn has EXTLIBS at the end. Which in turn means I do not understand your updated version, either. If having EXTLIBS before svn-fe.o is bad (and it is---a linker invocation lists *.o files that need to be linked first, and then libraries to find the symbols requested by those *.o files), wouldn't a fix be to just drop $(EXTLIBS)? Why is it necessary to list it twice? -- 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 in reflog of length 0x2BFF
This commit seems to introduce the bug: 4207ed285f31ad3e04f08254237c0c1a1609642b -- 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] Documentation: git log: --exit-code undocumented?
Sergey Organov sorga...@gmail.com writes: Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? It doesn't. That is why it is not listed. -- 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 add -i: allow list (un)selection by regexp
Aarni Koskela aarni.kosk...@andersinnovations.com writes: Hello! This patch makes it possible to select or unselect files in `git add -i` by regular expression instead of unique prefix only. The command syntax is `/foo` for selection and `-/foo` for unselection. I don't think the syntax will conflict with any existing use cases, but feel free to prove me wrong. Usually the responsibility to ensure correctness lies on the person who proposes a change, not those who relies on the continued correct operation of the existing code. I'm not a Perl programmer, but I've tried to follow the style of the existing code as much as possible. :) Note I'm currently not on the mailing list, so please cc. Best regards, Aarni Koskela All of the above do not belong to the commit log message. Please have these commentaries after the three-dash line you have under your Signed-off-by line. From 53c12d9c9928dc93a57595e92d785ecc0b245390 Mon Sep 17 00:00:00 2001 From: Aarni Koskela a...@iki.fi Date: Mon, 1 Dec 2014 11:06:10 +0200 Subject: [PATCH] git-add--interactive: allow list (un)selection by regular expression Remove these four lines when sending out a patch in the e-mail. Teach `list_and_choose` to allow `/regexp` and `-/regexp` syntax to select items based on regular expression match. For instance, `/jpg$` will select all options whose display name ends with `jpg`. It has been a long time since I wrote this code, but is this about the selection that happens after showing you a list of filenames to choose from? all options together with jpg made me go Huh? Does any of our command have jpeg related options? We are not in the image processing business. Perhaps s/all options/all files/ or something. Signed-off-by: Aarni Koskela a...@iki.fi --- git-add--interactive.perl | 27 +++ 1 file changed, 27 insertions(+) diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 1fadd69..34cc603 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -483,6 +483,8 @@ sub is_valid_prefix { !($prefix =~ /[\s,]/) # separators !($prefix =~ /^-/) # deselection !($prefix =~ /^\d+/) # selection + !($prefix =~ /^\//)# regexp selection + !($prefix =~ /^-\//) # regexp unselection ($prefix ne '*') # all wildcard ($prefix ne '?');# prompt help } @@ -585,6 +587,28 @@ sub list_and_choose { prompt_help_cmd(); next TOPLOOP; } + if ($line =~ /^(-)*\/(.+)$/) { You want zero or one '-', not zero or arbitrary large number of '-', as a sign to unchoose. (-)? instead of (-)*, perhaps? + my $choose = !($1 $1 eq '-'); The first $1 is an attempt to catch non-negative case where it is undef; it might be more explicit this way? my $choose = !(defined $1); + my $re = $2; Mental note. $re is an end-user supplied random string that may or may not be a valid regular expression. + my $found = 0; + for ($i = 0; $i @stuff; $i++) { + my $val = $stuff[$i]; + my $ref = ref $val; + if ($ref eq 'ARRAY') { + $val = $val-[0]; + } + elsif ($ref eq 'HASH') { + $val = $val-{VALUE}; + } + if ($val =~ /$re/) { ... which makes me wonder what happens when $re is a bogus regular expression here. Does the program die? Does the user get an error message about bad regexp and the condition to this if expression is false? Something else? Perhaps validating and catching regexp errors early like this might be sufficient: my $re = eval { qr/$2/ }; if (!$re) { print STDERR $@\n; next TOPLOOP; } 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
Re: [BUG] Documentation: git log: --exit-code undocumented?
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? It doesn't. That is why it is not listed. Then, how can --check possibly interfer with it? -- Sergey. -- 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] Documentation: git log: --exit-code undocumented?
Sergey Organov sorga...@gmail.com writes: Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? It doesn't. That is why it is not listed. Then, how can --check possibly interfer with it? The description is shared with git diff and friends, which is invoked via git log -p. As log does not give the exit code of individual diff-tree invocation for each commit, --exit-code option is irrelevant. -- 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] Documentation: git log: --exit-code undocumented?
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? It doesn't. That is why it is not listed. Then, how can --check possibly interfer with it? The description is shared with git diff and friends, which is invoked via git log -p. As log does not give the exit code of individual diff-tree invocation for each commit, --exit-code option is irrelevant. Do you agree there is some minor problem here? First, git log silently eats the --exit-code option that does nothing. Next, git-log manual page doesn't include --exit-log description while refers to it elsewhere. What's proposed resulution? -- Sergey. -- 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] run-command.c: retire unused run_hook_with_custom_index()
This was originally meant to be used to rewrite run_commit_hook() that only special cases the GIT_INDEX_FILE environment, but the run_hook_ve() refactoring done earlier made the implementation of run_commit_hook() thin and clean enough. Nobody uses this, so retire it as an unfinished clean-up made unnecessary. Signed-off-by: Junio C Hamano gits...@pobox.com --- run-command.c | 17 - run-command.h | 4 2 files changed, 21 deletions(-) diff --git a/run-command.c b/run-command.c index 35a3ebf..ae36852 100644 --- a/run-command.c +++ b/run-command.c @@ -792,20 +792,3 @@ int run_hook_le(const char *const *env, const char *name, ...) return ret; } - -int run_hook_with_custom_index(const char *index_file, const char *name, ...) -{ - const char *hook_env[3] = { NULL }; - char index[PATH_MAX]; - va_list args; - int ret; - - snprintf(index, sizeof(index), GIT_INDEX_FILE=%s, index_file); - hook_env[0] = index; - - va_start(args, name); - ret = run_hook_ve(hook_env, name, args); - va_end(args); - - return ret; -} diff --git a/run-command.h b/run-command.h index ea73de3..9e8cd9c 100644 --- a/run-command.h +++ b/run-command.h @@ -53,10 +53,6 @@ LAST_ARG_MUST_BE_NULL extern int run_hook_le(const char *const *env, const char *name, ...); extern int run_hook_ve(const char *const *env, const char *name, va_list args); -LAST_ARG_MUST_BE_NULL -__attribute__((deprecated)) -extern int run_hook_with_custom_index(const char *index_file, const char *name, ...); - #define RUN_COMMAND_NO_STDIN 1 #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ #define RUN_COMMAND_STDOUT_TO_STDERR 4 -- 2.2.0-133-g5229807 -- 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] Documentation: git log: --exit-code undocumented?
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Hello, $ git help log | grep exit-code problems are found. Not compatible with --exit-code. $ What --exit-code does in git log? It doesn't. That is why it is not listed. I disagree that --exit-code does nothing: it indicates whether the listed log is empty. So for example git log -1 --exit-code a..b /dev/null can be used to figure out whether a is a proper ancestor of b or not. -- David Kastrup -- 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 24/34] checkout: reject if the branch is already checked out elsewhere
Duy Nguyen pclo...@gmail.com writes: On Sun, Nov 30, 2014 at 12:18:40PM -0500, Mark Levedahl wrote: On 11/30/2014 03:24 AM, Nguyễn Thái Ngọc Duy wrote: One branch obviously can't be checked out at two places (but detached heads are ok). Give the user a choice in this case: --detach, -b new-branch, switch branch in the other checkout first or simply 'cd' and continue to work there. This seems too restrictive and is not obvious to me: I currently use git-new-workdir to have multiple checkouts of the same branch, with no ill effect. While those who do not understand what is going on underneath might be confused by one checkout suddenly showing uncommitted diffs, I don't accept that as a reason to outright prevent such use. I suggest, at the very least, that this behavior be overridden by a --force flag? Prevention is a strong word. It's more about safety for the mere mortals. It's certainly possible to do something like this patch (--force can't be reused, it already carries some other meaning). Should I add this one in the next (hic) reroll? Sorry, what is a hic? If this were an existing feature like git-new-workdir, even though it is from contrib, making it impossible to do something that used to be possible, even if that something is what mere mortals would never want to to to avoid risking confusion, would be a regression that needs an escape hatch. But this is a new feature. I am not sure why you need to make this overridable in the first place. Those who want to have multiple checkouts of the same commit can just detach HEAD at the same commit in multiple working trees, as the first thing they need to do would be to run git reset --hard $branch to synchronize the HEAD and the working tree state to work in the other out-of-sync repositories either case anyway. -- 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] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 7:48 AM, David Michael fedora@gmail.com wrote: On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote: On 12/01/2014 04:40 AM, David Michael wrote: On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de wrote: [snip] Could the code be more human-readable ? static inline mode_t mode_native_to_git(mode_t native_mode) { int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; if (S_ISBLK(native_mode)) return 006 | perm_bits; if (S_ISCHR(native_mode)) return 002 | perm_bits; if (S_ISFIFO(native_mode)) return 001 | perm_bits; /* Non-standard type bits were given. */ /* Shouldn't we die() here ?? */ return perm_bits; } Sure, I can send an updated patch with the new variable and without the elses. Regarding the question in the last comment: I was assuming if this case was ever reached, Git would handle the returned mode the same way as if it encountered an unknown/non-standard file type on a normal operating system, which could die() if needed in the function that called stat(). Should I send an updated patch that die()s there? David Not yet, please wait with a V2 patch until I finished my thinking ;-) I take back the suggestion with the die(). I was thinking how to handle unforeseen types, which may show up on the z/OS some day, So die() is not a good idea, it is better to ignore them, what the code does. Knowing that Git does not track block devices, nor character devices nor sockets, the above code could be simplyfied even more, by mapping everything which is not a directory, a file or a softlink to device type 0) This is just a suggestion, I want to here from others as well: int perm_bits = native_mode 0; if (S_ISREG(native_mode)) return 010 | perm_bits; if (S_ISDIR(native_mode)) return 004 | perm_bits; if (S_ISLNK(native_mode)) return 012 | perm_bits; /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK */ return perm_bits; I had considered omitting those three as well at first, but in this case it would mean that they will be unusable all together. The z/OS S_IFMT definition (i.e. the file type bit mask) is 0xFF00, and the common/translated S_IFMT definition is 0xF000. Since the S_ISxxx macros use the typical ((mode S_IFMT) == S_IFxxx) definition, they would never match a native z/OS mode after redefining S_IFMT. So translating those types isn't just for tracking files, it's to support any use of S_ISxxx anywhere in the code. It should be okay to remove any of those types if we know that Git will never need to use them. Apologies, in my pre-coffee reply, I confused myself into thinking the omitted types were going to be returned unchanged as opposed to being changed to zero. That second paragraph is irrelevant. But regarding the last paragraph: a quick grep for instances of using those file types in the Git source found S_ISFIFO and S_ISSOCK in git.c. I just noticed that I copied the list of standard file type macros from SUSv2, and S_IFSOCK was added after that. z/OS does implement S_IFSOCK, so I think I should add it to the v2 patch to support the test in git.c. Another grep found no instances of testing for block or character devices, so it should be okay to remove those from the patch if Git is unlikely to use them in the future (unless we just want to provide all 7 types from http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html ). David -- 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] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? Okay, it makes sense to only do the conversion for a successful return code. Should it test for both a zero return code and a non-null pointer? I don't know if there are any cases where passing a null pointer is legal. The standard doesn't seem to explicitly forbid it. z/OS returns -1 and sets errno to EFAULT when stat() is given NULL, but this patch should be able to be used on any platform. David -- 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] compat: convert modes to use portable file type values
David Michael fedora@gmail.com writes: On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? Okay, it makes sense to only do the conversion for a successful return code. Should it test for both a zero return code and a non-null pointer? I don't know if there are any cases where passing a null pointer is legal. The standard doesn't seem to explicitly forbid it. z/OS returns -1 and sets errno to EFAULT when stat() is given NULL, but this patch should be able to be used on any platform. Huh? I am confused. Since when is it legal to give NULL as statbuf to (l)stat(2)? Wouldn't something like this be sufficient and necessary? int rc = stat(path, buf); if (rc) return rc; That is, let the underlying stat(2) diagnose any and all problems (and leave clues in errno) and parrot its return value to the caller to signal the failure? -- 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: Deprecation warnings under XCode
Torsten Bögershausen tbo...@web.de writes: On 12/01/2014 04:02 AM, Michael Blume wrote: I have no idea whether this should concern anyone, but my mac build of git shows CC imap-send.o imap-send.c:183:36: warning: 'ERR_error_string' is deprecated: first deprecated in OS X 10.7 [-Wdeprecated-declarations] fprintf(stderr, %s: %s\n, func, ERR_error_string(ERR_get_error(), NULL)); ^ [] Isn't the warning a warning ;-) I don't see this warnings because my openssl comes from /opt/local/include (Mac ports) Does anybody know which new functions exist in Mac OS X versions = 10.7 ? I am not a Mac person, but is this about APPLE_COMMON_CRYPTO support added in 4dcd7732 (Makefile: add support for Apple CommonCrypto facility, 2013-05-19) and be4c828b (imap-send: eliminate HMAC deprecation warnings on Mac OS X, 2013-05-19)? Specifically, the log message for 4dcd7732 begins like so: Makefile: add support for Apple CommonCrypto facility As of Mac OS X 10.7, Apple deprecated all OpenSSL functions due to OpenSSL ABI instability, thus leading to build warnings. As a replacement, Apple encourages developers to migrate to its own (stable) CommonCrypto facility. In the Makefile we seem to have this: # Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X # and do not want to use Apple's CommonCrypto library. This allows you # to provide your own OpenSSL library, for example from MacPorts. which makes it sound like using APPLE_COMMON_CRYPTO is the default for Mac. Perhaps those who do want to use CommonCrypto to avoid warnings should not define that macro? -- 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] Documentation: git log: --exit-code undocumented?
Junio C Hamano gits...@pobox.com writes: David Kastrup d...@gnu.org writes: I disagree that --exit-code does nothing: it indicates whether the listed log is empty. So for example git log -1 --exit-code a..b /dev/null can be used to figure out whether a is a proper ancestor of b or not. Hmph. $ git log --exit-code master..maint /dev/null; echo $? 0 $ git log --exit-code maint..master /dev/null; echo $? 1 That is a strange way to use --exit-code. I suspect that if you did this, you will get 0 from the log between HEAD~..HEAD $ git checkout master^0 $ git commit --allow-empty -m empty $ git log --exit-code HEAD~..HEAD even though HEAD~ is a proper ancestor of HEAD, so it is not giving us anything useful. Isn't it a mere artifact that log happens to share the underlying machinery with diff that --exit-code shows a non-zero exit when there is any single commit in the range that has any change? Possibly: I haven't checked the underlying code for the details. At any rate, it is an option git log accepts for whatever reason. -- David Kastrup -- 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 in reflog of length 0x2BFF
So I am running a 3.13.0-40-generic x86_64 linux (so its's amd64) and git version 2.1.2 and I cannot reproduce the bug you are describing. :( $ git rev-parse 'master@{52}' 0035 What I noticed though is there are 2 linefeeds at the end of each line, is that intended or did it break during transmission? Stefan -- 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] fetch: add a config option to always use the depth argument
When having a repository, which deals with large amounts of data, i.e. graphics, music, films, you may want to keep the git directory at the smallest size possible. The depth option helped us in achieving this goal by removing the sizable history and just keep recent history around. In the case of having large amounts of data around, you probably want to use the depth option at any fetch you do, so it would be convenient to have an option for this. Change-Id: I45a569239639f20e24fbae32fb2046bc478c5f07 Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/config.txt| 6 ++ Documentation/fetch-options.txt | 2 ++ builtin/fetch.c | 5 + 3 files changed, 13 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9220725..418e21f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1106,6 +1106,12 @@ fetch.prune:: If true, fetch will automatically behave as if the `--prune` option was given on the command line. See also `remote.name.prune`. +fetch.depth:: + If set, fetch will automatically behave as if the `--depth` + option was given on the command line. This allows users to keep + the git directory at low space requirements, and thus comes in handy + for users with large binary files in the repository. + format.attach:: Enable multipart/mixed attachments as the default for 'format-patch'. The value can also be a double quoted string diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index b09a783..81131d0 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -12,6 +12,8 @@ `git clone` with `--depth=depth` option (see linkgit:git-clone[1]) to the specified number of commits from the tip of each remote branch history. Tags for the deepened commits are not fetched. + You can configure git to always use the depth option, see + `fetch.depth` in linkgit:git-config[1] --unshallow:: If the source repository is complete, convert a shallow diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..30fa15b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -68,6 +68,11 @@ static int git_fetch_config(const char *k, const char *v, void *cb) fetch_prune_config = git_config_bool(k, v); return 0; } + if (!strcmp(k, fetch.depth)) { + if (git_config_string(depth, k, v)) + return -1; + return 0; + } return git_default_config(k, v, cb); } -- 2.1.2 -- 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] compat: convert modes to use portable file type values
On Mon, Dec 1, 2014 at 12:57 PM, Junio C Hamano gits...@pobox.com wrote: David Michael fedora@gmail.com writes: On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote: On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote: +int git_stat(const char *path, struct stat *buf) +{ + int rc; + rc = stat(path, buf); + if (buf != NULL) It's a minor thing, but maybe test !rc instead of buf != NULL? Okay, it makes sense to only do the conversion for a successful return code. Should it test for both a zero return code and a non-null pointer? I don't know if there are any cases where passing a null pointer is legal. The standard doesn't seem to explicitly forbid it. z/OS returns -1 and sets errno to EFAULT when stat() is given NULL, but this patch should be able to be used on any platform. Huh? I am confused. Since when is it legal to give NULL as statbuf to (l)stat(2)? Wouldn't something like this be sufficient and necessary? int rc = stat(path, buf); if (rc) return rc; That is, let the underlying stat(2) diagnose any and all problems (and leave clues in errno) and parrot its return value to the caller to signal the failure? Alright, it wasn't immediately clear to me from the OpenGroup page on stat() if that would always be safe. I will just test the return code in v2. Thanks. David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: This allows the callback to use 'base' as a temporary buffer to quickly assemble full path without extra allocation. The callback has to restore it afterwards of course. Hmph, what's the quote around 'without' doing there? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Hmph, that's sad. Should the below say test_expect_failure without test_must_fail, anticipating a fix later? t/t3102-ls-tree-wildcards.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh index 83fca8d..93127a0 100755 --- a/t/t3102-ls-tree-wildcards.sh +++ b/t/t3102-ls-tree-wildcards.sh @@ -27,4 +27,8 @@ EOF test_cmp expected actual ' +test_expect_success 'ls-tree rejects negated pathspec' ' + test_must_fail git ls-tree -r HEAD :(exclude)a a* +' + test_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: [PATCH 00/19] Add git-list-files
Does this contain a lot of borrowed code or something? The style violation in the patches are unusually high, even compared with your other series. I've tried to fix it up and will push out the result on 'pu' if things work OK, but this does not even have tests, so it is unlikely that it would break anything but itself ;-) -- 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] compat: convert modes to use portable file type values
David Michael fedora@gmail.com writes: Huh? I am confused. Since when is it legal to give NULL as statbuf to (l)stat(2)? Wouldn't something like this be sufficient and necessary? int rc = stat(path, buf); if (rc) return rc; That is, let the underlying stat(2) diagnose any and all problems (and leave clues in errno) and parrot its return value to the caller to signal the failure? Alright, it wasn't immediately clear to me from the OpenGroup page on stat() if that would always be safe. I will just test the return code in v2. It is irrelevant if that is safe or not. As long as we are emulating the underlying stat(), whether it is unsafe for stat(), we should just throw whatever the user throws at us at the underlying stat() and let it behave as if our emulation layer were not there. Which is what the above snippet does. -- 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] fetch: add a config option to always use the depth argument
Stefan Beller sbel...@google.com writes: When having a repository, which deals with large amounts of data, i.e. graphics, music, films, you may want to keep the git directory at the smallest size possible. The depth option helped us in achieving this goal by removing the sizable history and just keep recent history around. In the case of having large amounts of data around, you probably want to use the depth option at any fetch you do, so it would be convenient to have an option for this. Change-Id: I45a569239639f20e24fbae32fb2046bc478c5f07 Signed-off-by: Stefan Beller sbel...@google.com --- Documentation/config.txt| 6 ++ Documentation/fetch-options.txt | 2 ++ builtin/fetch.c | 5 + 3 files changed, 13 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9220725..418e21f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1106,6 +1106,12 @@ fetch.prune:: If true, fetch will automatically behave as if the `--prune` option was given on the command line. See also `remote.name.prune`. +fetch.depth:: + If set, fetch will automatically behave as if the `--depth` + option was given on the command line. This allows users to keep + the git directory at low space requirements, and thus comes in handy + for users with large binary files in the repository. + Hmm, is this something a user would typically want repository-wide? I am wondering if remote.$nick.fetchDepth or something scoped to remote is more appropriate. format.attach:: Enable multipart/mixed attachments as the default for 'format-patch'. The value can also be a double quoted string diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index b09a783..81131d0 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -12,6 +12,8 @@ `git clone` with `--depth=depth` option (see linkgit:git-clone[1]) to the specified number of commits from the tip of each remote branch history. Tags for the deepened commits are not fetched. + You can configure git to always use the depth option, see + `fetch.depth` in linkgit:git-config[1] --unshallow:: If the source repository is complete, convert a shallow diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..30fa15b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -68,6 +68,11 @@ static int git_fetch_config(const char *k, const char *v, void *cb) fetch_prune_config = git_config_bool(k, v); return 0; } + if (!strcmp(k, fetch.depth)) { + if (git_config_string(depth, k, v)) + return -1; + return 0; + } return git_default_config(k, v, cb); } -- 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] On watchman support
On Fri, 2014-11-28 at 18:13 +0700, Duy Nguyen wrote: On Wed, Nov 19, 2014 at 1:12 AM, David Turner dtur...@twopensource.com wrote: Or will you go with cityhash now.. I ask because you have another sse optimization for hashmap on your watchman branch and that could reduce init time for name-hash. Name-hash is used often on case-insensitive fs (less often on case-sensitive fs). Cityhash would be better, because it has actual engineering effort put into it; what I did on my branch is a hack that happens to work decently. As the comment notes, I did not spend much effort on tuning my implementation. Also, Cityhash doesn't require SSE, so it's more portable. Cityhash looks less appealing to me. For one thing it's C++ so linking to C can't be done. I could add a few extern C to make it work. But if we plan to support it eventually, cityhash must support C out of the box. Then cityhash does not support case-insensitive hashing. I had to make a CityHash32i version based on CityHash32. It's probably my bugs there, but performance is worse (~120ms) than original hashmap.c (90ms). Enabling sse4.2 helps a bit, but still worse. Using the case-sensitive version in place for memihash and strihash does make cityhash win over hashmap.c, around 50ms (with or without sse4.2). But that's still not as good as your version (~35ms).. Can you post your CityHash32i? Have you tried this C port of Cityhash? https://github.com/santeri-io/cityhash-c -- 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] fetch: add a config option to always use the depth argument
Junio C Hamano gits...@pobox.com writes: Stefan Beller sbel...@google.com writes: +fetch.depth:: +If set, fetch will automatically behave as if the `--depth` +option was given on the command line. This allows users to keep +the git directory at low space requirements, and thus comes in handy +for users with large binary files in the repository. + Hmm, is this something a user would typically want repository-wide? I am wondering if remote.$nick.fetchDepth or something scoped to remote is more appropriate. Regardless of what configuration variable is used, I think setting depth directly from the config will mean the user cannot defeat a configured value by passing --unshallow because of this code sequence in builtin/fetch.c; am I mistaken? ... git_config(git_fetch_config, NULL); argc = parse_options(argc, argv, prefix, builtin_fetch_options, builtin_fetch_usage, 0); if (unshallow) { if (depth) die(_(--depth and --unshallow cannot be used together)); ... I think depth variable should be left alone. The right solution may be to leave the above unshallow and depth are incompatible check done only for the command line options as the original code, and much later in the code path after you figure out which remote we are talking to, only when neither --depth nor --unshallow is given, consult the configuration system to see if there is a default, perhaps in prepare_transport(). That approach will let you later support remote.$nick.fetchDepth, even if you start with a repository-wide fetch.depth option, I would think. -- 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 in reflog of length 0x2BFF
Am 01.12.14 19:53, schrieb Stefan Beller: So I am running a 3.13.0-40-generic x86_64 linux (so its's amd64) and git version 2.1.2 and I cannot reproduce the bug you are describing. :( ): I can reproduce it with * OS X, i386 binary, git 2.2.0 * FreeBSD, amd64, git 2.1.0 and up (bisected it there) * FreeBSD, amd64, git 2.1.2 (different machine) I cannot reproduce it with * Linux, amd64, git 2.1.0 $ git rev-parse 'master@{52}' 0035 On a machine, where you see the bug, you get entry /0...036/. This btw causes havoc: git stash list shows all entries, but e.g. git stash drop drops the wrong stash after @{52}. What I noticed though is there are 2 linefeeds at the end of each line, is that intended or did it break during transmission? That broke. It should be a normal reflog file. Try this: http://tron.yamagi.org/zeug/reflog.bad Still 4207ed285f31ad3e04f08254237c0c1a1609642b seems a plausible cause, because it's about reflogs. Though I suspect the actual bug was introduced before, because this commit only uses machinery, which was added earlier. Christoph -- 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 in reflog of length 0x2BFF
Hi Christoph, Christoph Mallon wrote: % git rev-parse 'master@{52}' warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970 00:00:01 +. 0036 Can you say more? What output did you expect and how does this differ from it? I tried, with git 2.2.0, git init gitbug cd gitbug git commit --allow-empty -m a wget http://tron.yamagi.org/zeug/reflog.bad mv reflog.bad .git/logs/refs/heads/master sha1sum .git/logs/refs/heads/master git rev-parse 'master@{52}' The output: 9ffe44715d0e542a60916255f144c74e6760ffd0 .git/logs/refs/heads/master 0035 Could you make a test script that illustrates and reproduces the problem? I.e., a patch to a file like t/t1410-reflog.sh, such that if I run cd git make cd t ./t1410-reflog.sh then I can reproduce the bug? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] Add another option for receive.denyCurrentBranch
Johannes Schindelin johannes.schinde...@gmx.de writes: +static const char *update_worktree(unsigned char *sha1) +{ +... + const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..; I overlooked this one, but is there a reason why this has to look at an internal implementatino detail which is git_work_tree_cfg, instead of asking get_git_work_tree()? I am wondering if that reason is a valid rationale to fix whatever makes get_git_work_tree() unsuitable for this purpose. Cc'ing Duy who has been working on the part of the system to determine and set-up work-tree and git-dir. -- 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 v5] Add another option for receive.denyCurrentBranch
Junio C Hamano gits...@pobox.com writes: Johannes Schindelin johannes.schinde...@gmx.de writes: +static const char *update_worktree(unsigned char *sha1) +{ +... +const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..; I overlooked this one, but is there a reason why this has to look at an internal implementatino detail which is git_work_tree_cfg, instead of asking get_git_work_tree()? I am wondering if that reason is a valid rationale to fix whatever makes get_git_work_tree() unsuitable for this purpose. Cc'ing Duy who has been working on the part of the system to determine and set-up work-tree and git-dir. Answering myself... This is because you know receive-pack runs inside the $GIT_DIR, whether it is a bare or non-bare repository, so either core.worktree points at a directory that is otherwise unrelated to the $GIT_DIR (but is the correct $GIT_WORK_TREE), or the top of the working tree must be .. for a non-bare repository. I haven't checked the code, but we probably are not even doing the repository/working-tree discovery to set up the data necessary for get_git_work_tree() to give us a correct answer. Doing an optional set-up to make get_git_work_tree() work would involve more damage to the codebase than necessary, I would suspect, so let's keep this part of the code as-is. That is, unless I am over-estimating the damage necessary. 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
Re: Bug in reflog of length 0x2BFF
Hi, so I have installed git version 2.2.0 currently, because I was trying to reproduce Bug in reflog of length 0x2BFF. Now I was using git as a normal user, rebasing some stuff (incidentally the reflog improvements) and an error occurred: $ git rebase --continue error: Ref refs/heads/todo_sb13_ref-transactions-reflog-as-file is at c48602d0aaa11b9099440c647ac028604fde4b14 but expected d1bbdc6f161ae7550805d565cad1d930487dad34 fatal: update_ref failed for ref 'refs/heads/todo_sb13_ref-transactions-reflog-as-file': Cannot lock the ref 'refs/heads/todo_sb13_ref-transactions-reflog-as-file'. So I think we definitely have a bug in the reflog code already in version 2.2.0. Trying to reproduce the error did not yield success. Thanks, Stefan -- 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] refs.c: add a transaction function to append a reflog entry
When performing a reflog transaction update, only write to the reflog iff msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform an update that only truncates but does not write. This change only affects whether or not a reflog entry should be generated and written. If msg == NULL then no such entry will be written. Orthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to tell the transaction system to truncate the reflog and thus discard all previous users. At the current time the only place where we use msg == NULL is also the place, where we use REFLOG_TRUNCATE. Even though these two settings are currently only ever used together it still makes sense to have them through two separate knobs. This allows future consumers of this API that may want to do things differently. For example someone can do: msg=Reflog truncated by Bob because ... + REFLOG_TRUNCATE and have it truncate the log and have it start fresh with an initial message that explains the log was truncated. This API allows that. During one transaction we allow to make multiple reflog updates to the same ref. This means we only need to lock the reflog once, during the first update that touches the reflog, and that all further updates can just write the reflog entry since the reflog is already locked. This allows us to write code such as: t = transaction_begin() transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL); loop-over-something... transaction_reflog_update(t, foo, 0, message); transaction_commit(t) where we first truncate the reflog and then build the new content one line at a time. Change-Id: I83923b2dcfa29aadb86a942826060180ac6f3d07 Signed-off-by: Stefan Beller sbel...@google.com --- refs.c | 112 +++-- refs.h | 21 + 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 58de60c..d0d2be7 100644 --- a/refs.c +++ b/refs.c @@ -3557,6 +3557,7 @@ struct transaction { struct ref_update **ref_updates; size_t alloc; size_t nr; + struct string_list reflog_updates; enum transaction_state state; }; @@ -3564,7 +3565,10 @@ struct transaction *transaction_begin(struct strbuf *err) { assert(err); - return xcalloc(1, sizeof(struct transaction)); + struct transaction *ret = xcalloc(1, sizeof(struct transaction)); + string_list_init(ret-reflog_updates, 1); + + return ret; } void transaction_free(struct transaction *transaction) @@ -3629,6 +3633,102 @@ int transaction_update_ref(struct transaction *transaction, return 0; } +/* Returns a non null fd, 0 on error. */ +static int get_reflog_updates_fd(struct transaction *transaction, + const char *refname) +{ + char *path; + struct lock_file *lock; + struct string_list_item *item = string_list_insert( + transaction-reflog_updates, + refname); + if (!item-util) { + item-util = xcalloc(1, sizeof(struct lock_file)); + lock = item-util; + path = git_path(logs/locks/%s, refname); + if (safe_create_leading_directories(path) 0) + return 0; + + if (hold_lock_file_for_update(lock, path, 0) 0) + return 0; + } + + lock = item-util; + + return lock-fd; +} + +int transaction_update_reflog(struct transaction *transaction, + const char *refname, + const unsigned char *new_sha1, + const unsigned char *old_sha1, + const char *email, + unsigned long timestamp, int tz, + const char *msg, int flags, + struct strbuf *err) +{ + /* +* We cannot handle reflogs the same way, we handle refs. +* +* Naming conflicts in the file system +* If renaming a ref from foo/foo to foo or the other way round, +* we need to be careful as we need the basic foo/ from being a +* directory to be a file or vice versa. When handling the refs +* this can be solved easily by first recording all we want into +* the packed refs file and then deleting all the loose refs. By +* doing it that way, we always have a valid state on disk. +* +* We don't have an equivalent of a packed refs file when dealing +* with reflog updates, but the API for updating the refs turned +* out to be conveniant, so let's introduce an intermediate file +* outside the $GIT_DIR/logs/refs/heads/ directory helping us +* avoiding this naming conflict for the reflogs. The intermediate +* lock file, in which we
[PATCH 2/4] refs.c: rename transaction.updates to ref_updates
The updates are only holding refs not reflogs, so express it to the reader. Change-Id: Ifdc87722f0e7314f3d0febb970aa7769eada6d33 Signed-off-by: Stefan Beller sbel...@google.com --- refs.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/refs.c b/refs.c index f0f0d23..58de60c 100644 --- a/refs.c +++ b/refs.c @@ -3554,7 +3554,7 @@ enum transaction_state { * as atomically as possible. This structure is opaque to callers. */ struct transaction { - struct ref_update **updates; + struct ref_update **ref_updates; size_t alloc; size_t nr; enum transaction_state state; @@ -3575,10 +3575,10 @@ void transaction_free(struct transaction *transaction) return; for (i = 0; i transaction-nr; i++) { - free(transaction-updates[i]-msg); - free(transaction-updates[i]); + free(transaction-ref_updates[i]-msg); + free(transaction-ref_updates[i]); } - free(transaction-updates); + free(transaction-ref_updates); free(transaction); } @@ -3589,8 +3589,8 @@ static struct ref_update *add_update(struct transaction *transaction, struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1); strcpy((char *)update-refname, refname); - ALLOC_GROW(transaction-updates, transaction-nr + 1, transaction-alloc); - transaction-updates[transaction-nr++] = update; + ALLOC_GROW(transaction-ref_updates, transaction-nr + 1, transaction-alloc); + transaction-ref_updates[transaction-nr++] = update; return update; } @@ -3712,7 +3712,7 @@ int transaction_commit(struct transaction *transaction, int ret = 0, delnum = 0, i; const char **delnames; int n = transaction-nr; - struct ref_update **updates = transaction-updates; + struct ref_update **updates = transaction-ref_updates; assert(err); -- 2.2.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] reflog.c: use a reflog transaction when writing during expire
From: Ronnie Sahlberg sahlb...@google.com Use a transaction for all updates during expire_reflog. Change-Id: Ieb81b2660cefeeecf0bcb3cdbc1ef3cbb86e7eb8 Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Stefan Beller sbel...@google.com --- builtin/reflog.c | 85 1 file changed, 37 insertions(+), 48 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 2d85d26..6bb7454 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb { int recno; }; +static struct strbuf err = STRBUF_INIT; + struct expire_reflog_cb { - FILE *newlog; + struct transaction *t; + const char *refname; enum { UE_NORMAL, UE_ALWAYS, @@ -316,20 +319,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1, if (cb-cmd-recno --(cb-cmd-recno) == 0) goto prune; - if (cb-newlog) { - char sign = (tz 0) ? '-' : '+'; - int zone = (tz 0) ? (-tz) : tz; - fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s, - sha1_to_hex(osha1), sha1_to_hex(nsha1), - email, timestamp, sign, zone, - message); + if (cb-t) { + if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1, + email, timestamp, tz, message, 0, + err)) + return -1; hashcpy(cb-last_kept_sha1, nsha1); } if (cb-cmd-verbose) printf(keep %s, message); return 0; prune: - if (!cb-newlog) + if (!cb-t) printf(would prune %s, message); else if (cb-cmd-verbose) printf(prune %s, message); @@ -353,29 +354,26 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, { struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; - struct ref_lock *lock; - char *log_file, *newlog_path = NULL; struct commit *tip_commit; struct commit_list *tips; int status = 0; memset(cb, 0, sizeof(cb)); + cb.refname = ref; - /* -* we take the lock for the ref itself to prevent it from -* getting updated. -*/ - lock = lock_any_ref_for_update(ref, sha1, 0, NULL); - if (!lock) - return error(cannot lock ref '%s', ref); - log_file = git_pathdup(logs/%s, ref); if (!reflog_exists(ref)) goto finish; - if (!cmd-dry_run) { - newlog_path = git_pathdup(logs/%s.lock, ref); - cb.newlog = fopen(newlog_path, w); + cb.t = transaction_begin(err); + if (!cb.t) { + status |= error(%s, err.buf); + goto cleanup; + } + if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1, + NULL, 0, 0, NULL, REFLOG_TRUNCATE, + err)) { + status |= error(%s, err.buf); + goto cleanup; } - cb.cmd = cmd; if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) { @@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, mark_reachable(cb); } - for_each_reflog_ent(ref, expire_reflog_ent, cb); + if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) { + status |= error(%s, err.buf); + goto cleanup; + } if (cb.unreachable_expire_kind != UE_ALWAYS) { if (cb.unreachable_expire_kind == UE_HEAD) { @@ -420,32 +421,20 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, } } finish: - if (cb.newlog) { - if (fclose(cb.newlog)) { - status |= error(%s: %s, strerror(errno), - newlog_path); - unlink(newlog_path); - } else if (cmd-updateref - (write_in_full(lock-lock_fd, - sha1_to_hex(cb.last_kept_sha1), 40) != 40 || -write_str_in_full(lock-lock_fd, \n) != 1 || -close_ref(lock) 0)) { - status |= error(Couldn't write %s, - lock-lk-filename.buf); - unlink(newlog_path); - } else if (rename(newlog_path, log_file)) { - status |= error(cannot rename %s to %s, - newlog_path, log_file); - unlink(newlog_path); - } else if (cmd-updateref
[PATCH 1/4] refs.c: rename the transaction functions
From: Ronnie Sahlberg sahlb...@google.com Rename the transaction functions. Remove the leading ref_ from the names and append _ref to the names for functions that create/delete/ update sha1 refs. todo(sbeller): notes below 3 dashes This commit can be reproduced via find . -name *.[ch] -print | xargs sed -i ' \ s/REF_TRANSACTION_OPEN/TRANSACTION_OPEN/; \ s/REF_TRANSACTION_CLOSED/TRANSACTION_CLOSED/; \ s/ref_transaction_begin/transaction_begin/; \ s/ref_transaction_commit/transaction_commit/; \ s/ref_transaction_create/transaction_create_ref/; \ s/ref_transaction_delete/transaction_delete_ref/; \ s/ref_transaction_free/transaction_free/; \ s/ref_transaction_update/transaction_update_ref/; \ s/ref_transaction/transaction/' modulo white space changes for alignment. Change-Id: Iffdc56536be71c5061caa23040ce0d89efd52196 Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Stefan Beller sbel...@google.com --- branch.c | 13 + builtin/commit.c | 10 +++ builtin/fetch.c| 12 builtin/receive-pack.c | 13 - builtin/replace.c | 10 +++ builtin/tag.c | 10 +++ builtin/update-ref.c | 26 - fast-import.c | 22 +++--- refs.c | 78 +- refs.h | 36 +++ sequencer.c| 12 walker.c | 10 +++ 12 files changed, 126 insertions(+), 126 deletions(-) diff --git a/branch.c b/branch.c index 4bab55a..c8462de 100644 --- a/branch.c +++ b/branch.c @@ -279,16 +279,17 @@ void create_branch(const char *head, log_all_ref_updates = 1; if (!dont_change_ref) { - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref.buf, sha1, - null_sha1, 0, !forcing, msg, err) || - ref_transaction_commit(transaction, err)) + transaction_update_ref(transaction, ref.buf, sha1, + null_sha1, 0, !forcing, msg, + err) || + transaction_commit(transaction, err)) die(%s, err.buf); - ref_transaction_free(transaction); + transaction_free(transaction); strbuf_release(err); } diff --git a/builtin/commit.c b/builtin/commit.c index e108c53..f50b7df 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct stat statbuf; struct commit *current_head = NULL; struct commit_extra_header *extra = NULL; - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; if (argc == 2 !strcmp(argv[1], -h)) @@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix) strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg)); strbuf_insert(sb, strlen(reflog_msg), : , 2); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, HEAD, sha1, + transaction_update_ref(transaction, HEAD, sha1, current_head ? current_head-object.sha1 : NULL, 0, !!current_head, sb.buf, err) || - ref_transaction_commit(transaction, err)) { + transaction_commit(transaction, err)) { rollback_index_files(); die(%s, err.buf); } - ref_transaction_free(transaction); + transaction_free(transaction); unlink(git_path(CHERRY_PICK_HEAD)); unlink(git_path(REVERT_HEAD)); diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..0be0b09 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -404,7 +404,7 @@ static int s_update_ref(const char *action, { char msg[1024]; char *rla = getenv(GIT_REFLOG_ACTION); - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; int ret, df_conflict = 0; @@ -414,23 +414,23 @@ static int s_update_ref(const char *action, rla = default_rla.buf; snprintf(msg, sizeof(msg), %s: %s, rla, action); - transaction = ref_transaction_begin(err); + transaction =
Re: [PATCH 1/4] refs.c: rename the transaction functions
This patch series is not really ready for public digestion, I messed up sending it to the list anyway. Please ignore this series. -- 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
http-protocol question
In Documentation/technical/http-protocol.txt, there is the following statement: S: Parse the git-upload-pack request: Verify all objects in `want` are directly reachable from refs. The server MAY walk backwards through history or through the reflog to permit slightly stale requests. Is there actually logic somewhere in Git that does that MAY walk backwards step? Or is that something another implementation of Git may do but the standard Git does not? Thanks, Bryan Turner -- 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 09/19] Add git-list-files, a user friendly version of ls-files and more
On Sunday, November 30, 2014, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: This is more user friendly version of ls-files: * it's automatically colored and columnized * it refreshes the index like all porcelain commands * it defaults to non-recursive behavior like ls * :(glob) is on by default so '*.c' means a.c but not a/b.c, use '**/*.c' for that. * auto pager The name 'ls' is not taken. It is left for the user to make an alias with better default options. I understand that your original version was named git-ls and that you renamed it to git-list-files in order to leave 'ls' available so users can create an 'ls' alias specifying their own default options. Would it make sense, however, to restore the name to git-ls and allow users to set default options via a config variable instead? Doing so would make the short-and-sweet git-ls command work for all users out-of-the-box, which might be well appreciated by Unix users. More below. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/git-list-files.txt b/Documentation/git-list-files.txt new file mode 100644 index 000..3039e1e --- /dev/null +++ b/Documentation/git-list-files.txt @@ -0,0 +1,80 @@ +git-list-files(1) +=== + +NAME + +git-list-files - List files + +SYNOPSIS + +[verse] +'git list-files [options] [pathspec...] + +DESCRIPTION +--- +List files (by default in current working directory) that are in the +index. Depending on the chosen options, maybe only modified files in +working tree are shown, or untracked files... + +OPTIONS +--- +-c:: +--cached:: + Show cached files (default) I realize that this mirrors what is in git-ls-files.txt, but: s/$/./ +-d:: +--deleted:: + Show cached files that are deleted on working directory s/$/./ +-m:: +--modified:: + Show cached files that have modification on working directory s/$/./ +-o:: +--others:: + Show untracked files (and only unignored ones unless -i is s/-i/`-i`/ + specified) s/$/./ +-i:: +--ignored:: + Show only ignored files. When showing files in the index, + print only those matched by an exclude pattern. When showing + other files, show only those matched by an exclude pattern. + +-u:: +--unmerged:: + Show unmerged files s/$/./ +--color[=when]:: +--no-color:: + Color file names. The value must be `always`, `never`, or + `auto`. `--no-color` is equivalent to + `--color=never`. `--color` is equivalent to + `--color=auto`. See configuration variable `color.list-files` + for the default settings. + +--column[=options]:: +--no-column:: + Display files in columns. See configuration variable column.ui s/column.ui/`column.ui`/ + for option syntax. `--column` and `--no-column` without options + are equivalent to 'always' and 'never' respectively. + +--max-depth=depth:: + For each pathspec given on command line, descend at most depth + levels of directories. A negative value means no limit. + This option is ignored if pathspec contains active wildcards. + In other words if a* matches a directory named a*, + * is matched literally so --max-depth is still effective. + The default is `--max-depth=0`. + +pathspec:: + Files to show. :(glob) magic is enabled and recursion disabled + by default. + +SEE ALSO + +linkgit:git-ls-files[1] + +GIT +--- +Part of the linkgit:git[1] suite -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules
Junio C Hamano gits...@pobox.com writes: And thinking about the names again, I have a feeling that updateInstead and mergeInstead are both probably misnomer. Let me take this part back. After all, I do not think I would design the mechanism to implement an alternative logic that decides when it is safe to allow the update of the ref and to reflect the changes to the working tree, and that actually does the checkout to the working tree by using a new value like mergeInstead. So we would only need a single name, and updateInstead is not too bad. ... The mechanism I would employ when doing an alternative logic, possibly looser one but does not necessarily so, would be to have a hook script push-to-checkout. When denyCurrentBranch is set to updateInstead, if there is no such hook, the working tree has to be absolutely clean and we would do a 'read-tree -m -u $old $new' (which is the same as 'reset --hard $new' under the precondition) you implemented will be used as the logic that decides when it is safe, and that does the checkout to the working tree. When the push-to-checkout hook exists, however, we just invoke that hook with $new as argument, and it can decide when it is safe in whatever way it chooses to, and it can checkout the $new to the working tree in whatever way it wants. So here comes a two-patch series on top of your series (with the test update I sent earlier). As I never do push to deploy that requires no changes to the working tree or to the index, while I have seem myself in a situation where I have to emulate a git pull with a git push in the opposite direction (and work it around if the target of the 'git pull' I wanted to do were the current branch, by first pushing into a throw-away branch, because of denyCurrent), I could imagine myself using this variant. Having said that, this is primarily so that I do not want to forget and discard the brain cycles we spent discussing this to the waste, more than that I cannot wait to use this feature myself ;-) The first one here is a pure refactoring. The second one is the real fun. -- 8 -- Subject: [PATCH 1/2] receive-pack: refactor updateInstead codepath Keep the there is nothing to update in a bare repository, when the check and update process runs, here are the GIT_DIR and GIT_WORK_TREE logic, which will be common regardless of how the decision to update and the actual update are done, in the original update_worktree() function, and split out the working tree and the index must match the original HEAD exactly and use two-way read-tree to update the working tree into a new push_to_deploy() helper function. This will allow customizing the logic more cleanly and easily. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/receive-pack.c | 53 ++ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c047418..11800cd 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -733,7 +733,9 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si) return 0; } -static const char *update_worktree(unsigned char *sha1) +static const char *push_to_deploy(unsigned char *sha1, + struct argv_array *env, + const char *work_tree) { const char *update_refresh[] = { update-index, -q, --ignore-submodules, --refresh, NULL @@ -748,69 +750,70 @@ static const char *update_worktree(unsigned char *sha1) const char *read_tree[] = { read-tree, -u, -m, NULL, NULL }; - const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..; - struct argv_array env = ARGV_ARRAY_INIT; struct child_process child = CHILD_PROCESS_INIT; - if (is_bare_repository()) - return denyCurrentBranch = updateInstead needs a worktree; - - argv_array_pushf(env, GIT_DIR=%s, absolute_path(get_git_dir())); - child.argv = update_refresh; - child.env = env.argv; + child.env = env-argv; child.dir = work_tree; child.no_stdin = 1; child.stdout_to_stderr = 1; child.git_cmd = 1; - if (run_command(child)) { - argv_array_clear(env); + if (run_command(child)) return Up-to-date check failed; - } /* run_command() does not clean up completely; reinitialize */ child_process_init(child); child.argv = diff_files; - child.env = env.argv; + child.env = env-argv; child.dir = work_tree; child.no_stdin = 1; child.stdout_to_stderr = 1; child.git_cmd = 1; - if (run_command(child)) { - argv_array_clear(env); + if (run_command(child)) return Working directory has unstaged changes; - } child_process_init(child); child.argv =
[PATCH 2/2] receive-pack: support push-to-checkout hook
When receive.denyCurrentBranch is set to updateInstead, this hook can be used to override the built-in push-to-deploy logic, which insists that the working tree and the index must be unchanged relative to HEAD. The hook receives the commit with which the tip of the current is going to be updated, and is responsible to make any necessary changes to the working tree and to the index to bring them to the desired state when the tip of the current branch is updated to the new commit. For example, the hook can simply run git read-tree -u -m HEAD $1 to the workflow to emulate 'git fetch' going in the reverse direction with 'git push' better than the push-to-deploy logic, as the two-tree form of read-tree -u -m is essentially the same as git checkout that switches branches while keeping the local changes in the working tree that do not interfere with the difference between the branches. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/receive-pack.c | 19 ++- t/t5516-fetch-push.sh | 63 ++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 11800cd..fc8ec9c 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -797,6 +797,20 @@ static const char *push_to_deploy(unsigned char *sha1, return NULL; } +static const char *push_to_checkout_hook = push-to-checkout; + +static const char *push_to_checkout(unsigned char *sha1, + struct argv_array *env, + const char *work_tree) +{ + argv_array_pushf(env, GIT_WORK_TREE=%s, absolute_path(work_tree)); + if (run_hook_le(env-argv, push_to_checkout_hook, + sha1_to_hex(sha1), NULL)) + return push-to-checkout hook declined; + else + return NULL; +} + static const char *update_worktree(unsigned char *sha1) { const char *retval; @@ -808,7 +822,10 @@ static const char *update_worktree(unsigned char *sha1) argv_array_pushf(env, GIT_DIR=%s, absolute_path(get_git_dir())); - retval = push_to_deploy(sha1, env, work_tree); + if (!find_hook(push_to_checkout_hook)) + retval = push_to_deploy(sha1, env, work_tree); + else + retval = push_to_checkout(sha1, env, work_tree); argv_array_clear(env); return retval; diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 85c7fec..e4436c1 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1434,4 +1434,67 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' ' ' +test_expect_success 'updateInstead with push-to-checkout hook' ' + rm -fr testrepo + git init testrepo + ( + cd testrepo + git pull .. master + git reset --hard HEAD^^ + git tag initial + git config receive.denyCurrentBranch updateInstead + write_script .git/hooks/push-to-checkout -\EOF + echo 2 updating from $(git rev-parse HEAD) + echo 2 updating to $1 + + git update-index -q --refresh + git read-tree -u -m HEAD $1 || { + status=$? + echo 2 read-tree failed + exit $status + } + EOF + ) + + # Try pushing into a pristine + git push testrepo master + ( + cd testrepo + git diff --quiet + git diff HEAD --quiet + test $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD) + ) + + # Try pushing into a repository with conflicting change + ( + cd testrepo + git reset --hard initial + echo conflicting path2 + ) + test_must_fail git push testrepo master + ( + cd testrepo + test $(git rev-parse initial) = $(git rev-parse HEAD) + test conflicting = $(cat path2) + git diff-index --quiet --cached HEAD + ) + + # Try pushing into a repository with unrelated change + ( + cd testrepo + git reset --hard initial + echo unrelated path1 + echo irrelevant path5 + git add path5 + ) + git push testrepo master + ( + cd testrepo + test $(cat path1) = unrelated + test $(cat path5) = irrelevant + test $(git diff --name-only --cached HEAD) = path5 + test $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD) + ) +' + test_done -- 2.2.0-141-gd3f4719 -- 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: http-protocol question
Hi Bryan, Bryan Turner wrote: Is there actually logic somewhere in Git that does that MAY walk backwards step? Yes. See upload-pack.c::check_non_tip and http://thread.gmane.org/gmane.comp.version-control.git/178814. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
On Tue, Dec 2, 2014 at 2:34 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi Bryan, Bryan Turner wrote: Is there actually logic somewhere in Git that does that MAY walk backwards step? Yes. See upload-pack.c::check_non_tip and http://thread.gmane.org/gmane.comp.version-control.git/178814. Jonathan, Thanks for the reply! I realize now I didn't really cite the part of that documentation that I'm most interested in, which is: through history _or through the reflog_. It's the reflog bit I'm looking for. Sorry for not being more clear. check_non_tip appears to look at ancestry, walking back up (or down, depending on your view) the DAG to see if the requested SHA-1 is reachable from a current ref, so it looks like that's covering the through history part of that blurb. The reason I ask is that there is a race condition that exists where the ref advertisement lists refs/heads/foo at abc1234, and then foo is deleted before the actual upload-pack request comes in. In that situation, walking backwards through _history_ will only allow the upload-pack to complete unless the deleted ref was merged into another ref prior to being deleted (if I understand the code correctly). It seems like looking at the reflogs, and seeing that refs/heads/foo did in fact exist and did in fact refer to abc1234, is the only approach that could allow the upload-pack to complete. The documentation hints that such a thing is possible, but I don't see any code in Git that seems to do that. Does that make more sense? Does that functionality exist and I've just overlooked it? Thanks again, Bryan Turner Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
On Tue, Dec 2, 2014 at 3:28 PM, Bryan Turner btur...@atlassian.com wrote: On Tue, Dec 2, 2014 at 2:34 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi Bryan, Bryan Turner wrote: Is there actually logic somewhere in Git that does that MAY walk backwards step? Yes. See upload-pack.c::check_non_tip and http://thread.gmane.org/gmane.comp.version-control.git/178814. Jonathan, Thanks for the reply! I realize now I didn't really cite the part of that documentation that I'm most interested in, which is: through history _or through the reflog_. It's the reflog bit I'm looking for. Sorry for not being more clear. check_non_tip appears to look at ancestry, walking back up (or down, depending on your view) the DAG to see if the requested SHA-1 is reachable from a current ref, so it looks like that's covering the through history part of that blurb. The reason I ask is that there is a race condition that exists where the ref advertisement lists refs/heads/foo at abc1234, and then foo is deleted before the actual upload-pack request comes in. In that situation, walking backwards through _history_ will only allow the upload-pack to complete unless the deleted ref was merged into another s/unless/if, sorry. In that situation, walking backwards through history will only allow the upload-pack to complete if the deleted ref was merged into another ref. ref prior to being deleted (if I understand the code correctly). It seems like looking at the reflogs, and seeing that refs/heads/foo did in fact exist and did in fact refer to abc1234, is the only approach that could allow the upload-pack to complete. The documentation hints that such a thing is possible, but I don't see any code in Git that seems to do that. Does that make more sense? Does that functionality exist and I've just overlooked it? Thanks again, Bryan Turner Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
Hi, Bryan Turner wrote: The reason I ask is that there is a race condition that exists where the ref advertisement lists refs/heads/foo at abc1234, and then foo is deleted before the actual upload-pack request comes in. Can you say more about the context? For example, was this actually happening, or is this for the sake of understanding the protocol better? I ask because knowing the context might help us give more specific advice. Sometimes when people delete a branch they really want those objects to be inaccessible *right away*. So for such people, git's behavior of failing the request unless the objects are still accessible by some other path is helpful. A stateful server could theoretically cache the list of objects they have advertised for a short time, to avoid clients having to suffer when something becomes inaccessible during the window between the upload-pack advertisement and upload-pack request. Or a permissive server can allow all wants except a specific blacklist (and some people do that). Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
On Tue, Dec 2, 2014 at 3:45 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Bryan Turner wrote: The reason I ask is that there is a race condition that exists where the ref advertisement lists refs/heads/foo at abc1234, and then foo is deleted before the actual upload-pack request comes in. Can you say more about the context? For example, was this actually happening, or is this for the sake of understanding the protocol better? I ask because it's actually happening. Heavy CI load sometimes has builds fail because git clone dies with not our ref. That's the specific context I'm working to try and address right now. Some teams use rebase-heavy workflows, which also evades the check_non_tip easing and fails with not our ref, so I can't be 100% certain it's ref deletion in specific causing it (but I guess which of those it is is probably largely academic; as long as hosting spans multiple requests it seems like this type of race condition is unavoidable). I'm trying to decide if there is something I can enable or tune to prevent it, and the type of twilighting hinted at by the http-protocol documentation seemed like it could be just the thing. I ask because knowing the context might help us give more specific advice. Sometimes when people delete a branch they really want those objects to be inaccessible *right away*. So for such people, git's behavior of failing the request unless the objects are still accessible by some other path is helpful. A stateful server could theoretically cache the list of objects they have advertised for a short time, to avoid clients having to suffer when something becomes inaccessible during the window between the upload-pack advertisement and upload-pack request. Or a permissive server can allow all wants except a specific blacklist (and some people do that). Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
Bryan Turner wrote: I ask because it's actually happening. Heavy CI load sometimes has builds fail because git clone dies with not our ref. That's the specific context I'm working to try and address right now. Thanks --- that makes sense. Some teams use rebase-heavy workflows, which also evades the check_non_tip easing and fails with not our ref, so I can't be 100% certain it's ref deletion in specific causing it (but I guess which of those it is is probably largely academic; as long as hosting spans multiple requests it seems like this type of race condition is unavoidable). Yeah, everything mentioned before about ref deletion also applies to non-fast-forward updates. I'm trying to decide if there is something I can enable or tune to prevent it, and the type of twilighting hinted at by the http-protocol documentation seemed like it could be just the thing. If you control the side that clones, then just cloning the single branch you are building (with --single-branch and -b) can help. Using a bidirectional protocol like git:// or ssh:// (where the ref advertisement and check of wants happen in the same connection) would avoid the problem we're talking about. On the server side, I agree that either mining reflogs or storing advertisements to disk would be a nice way to take care of this. No one has implemented either of those, but it would be a nice setting to have. :) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: http-protocol question
On Tue, Dec 2, 2014 at 12:17 PM, Jonathan Nieder jrnie...@gmail.com wrote: On the server side, I agree that either mining reflogs or storing advertisements to disk would be a nice way to take care of this. No one has implemented either of those, but it would be a nice setting to have. :) and could be dangerous too. If I accidentally add a password file, then delete it (way before a clone is performed), I don't want that part of reflog visible to the cloner. Problem is we don't know how far we should look back in reflog unless we keep some state. -- 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: http-protocol question
On Tue, Dec 02, 2014 at 04:04:11PM +1100, Bryan Turner wrote: Can you say more about the context? For example, was this actually happening, or is this for the sake of understanding the protocol better? I ask because it's actually happening. Heavy CI load sometimes has builds fail because git clone dies with not our ref. That's the specific context I'm working to try and address right now. Some teams use rebase-heavy workflows, which also evades the check_non_tip easing and fails with not our ref, so I can't be 100% certain it's ref deletion in specific causing it (but I guess which of those it is is probably largely academic; as long as hosting spans multiple requests it seems like this type of race condition is unavoidable). There is a practical reason to care. Ref deletion will also delete the reflog, leaving no trace of the reachability. Whereas a non-fast-forward push could be resolved by looking in the reflog. One problem with hunting for sha1s in the reflog is that upload-pack does not know which ref the client thinks they are requesting. So a search would involve looking in _every_ reflog, which could be expensive. It might not be too painful if you do the search only after hitting a not our ref condition, which should in theory be rare. A malicious client could convince you to grep your reflogs repeatedly, but that is hardly the only way to convince upload-pack to chew CPU. Asking it to make a pack comes to mind. :) I'm trying to decide if there is something I can enable or tune to prevent it, and the type of twilighting hinted at by the http-protocol documentation seemed like it could be just the thing. For a public repository, it might make sense to provide a config option to loosen the is_our_ref check completely (i.e., to just has_sha1_file). But such an option does not yet exist. -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: http-protocol question
On Tue, Dec 2, 2014 at 12:17 PM, Jonathan Nieder jrnie...@gmail.com wrote: I'm trying to decide if there is something I can enable or tune to prevent it, and the type of twilighting hinted at by the http-protocol documentation seemed like it could be just the thing. If you control the side that clones, then just cloning the single branch you are building (with --single-branch and -b) can help. Or we could extend this a bit on the server side, if one ref fail, let the clone continue with the remaining refs (and warn loudly to the user). -- 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: [PATCH 00/19] Add git-list-files
On Sun, Nov 30, 2014 at 03:55:48PM +0700, Nguyễn Thái Ngọc Duy wrote: This is something else that's been sitting in my tree for a while now. It adds git list-files, intended to be aliased as ls with your favourite display options. When I read the subject, I thought why isn't this called git-ls?. Then when I read this paragraph, I thought if the point is for everybody to make their own ls alias, why do we need list-files at all, instead of just adding options to ls-files? I'll let you decide which (if any) you'd like to answer. :) My guesses: 1. If it were git-ls, it would stomp on existing aliases people have constructed. 2. If it is a wrapper around ls-files, then the options may be constrained; ls-files already squats on useful options like -d (which, if we are matching traditional ls, is more like our -t). I somewhat feel like (1) can be mitigated by the fact that your command is what people were probably trying to approximate with their aliases, and that as porcelain it should be very configurable (so they should be able to accomplish the same things as their aliases). But I dunno. I do not have an ls alias, so I am biased. :) As a side note, I wonder if it would be sensible to whitelist some commands as porcelain, and allow aliases to override them (either entirely, or just to add-in some options). -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: http-protocol question
On Tue, Dec 2, 2014 at 4:33 PM, Jeff King p...@peff.net wrote: On Tue, Dec 02, 2014 at 04:04:11PM +1100, Bryan Turner wrote: Can you say more about the context? For example, was this actually happening, or is this for the sake of understanding the protocol better? I ask because it's actually happening. Heavy CI load sometimes has builds fail because git clone dies with not our ref. That's the specific context I'm working to try and address right now. Some teams use rebase-heavy workflows, which also evades the check_non_tip easing and fails with not our ref, so I can't be 100% certain it's ref deletion in specific causing it (but I guess which of those it is is probably largely academic; as long as hosting spans multiple requests it seems like this type of race condition is unavoidable). There is a practical reason to care. Ref deletion will also delete the reflog, leaving no trace of the reachability. Whereas a non-fast-forward push could be resolved by looking in the reflog. A fair point. I had mistakenly thought that reflogs would survive the ref's deletion and be pruned as part of garbage collection, but a quick test shows that, as I'm sure you already know, that's not true. Thanks for correcting my mistake! Bryan Turner -- 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: http-protocol question
On Tue, Dec 02, 2014 at 04:47:50PM +1100, Bryan Turner wrote: There is a practical reason to care. Ref deletion will also delete the reflog, leaving no trace of the reachability. Whereas a non-fast-forward push could be resolved by looking in the reflog. A fair point. I had mistakenly thought that reflogs would survive the ref's deletion and be pruned as part of garbage collection, but a quick test shows that, as I'm sure you already know, that's not true. I wish it worked that way. Unfortunately there are complications with keeping the old reflogs in place, because they sometimes cause conflicts with new refs being created (e.g., a reflog in .git/logs/refs/heads/foo would prevent .git/logs/refs/heads/foo/bar from being created). I had some patches long ago to try to keep a reflog graveyard around, but they were quite invasive, and there were some corner cases that caused weird errors. Handling this sort of D/F conflict more gracefully is one of the things I'd like to experiment with once we have pluggable ref backends (I think we'll also disallow foo/bar if foo exists, but the storage could at least keep the reflogs around). -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: Bug in reflog of length 0x2BFF
Hi Jonathan, Am 02.12.14 00:35, schrieb Jonathan Nieder: Christoph Mallon wrote: % git rev-parse 'master@{52}' warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970 00:00:01 +. 0036 Can you say more? What output did you expect and how does this differ from it? sorry, I thought it is obvious that the warning should not be there. As far as I understand the code, this warning is shown, when the old commit id of one entry does not equal the new commit id of its predecessor. But this reflog file does not have such a gap. Also the correct result ist 0...035, not 0...036. I.e. one entry is erroneously skipped. I tried, with git 2.2.0, git init gitbug cd gitbug git commit --allow-empty -m a wget http://tron.yamagi.org/zeug/reflog.bad mv reflog.bad .git/logs/refs/heads/master sha1sum .git/logs/refs/heads/master git rev-parse 'master@{52}' These steps look right. The output: 9ffe44715d0e542a60916255f144c74e6760ffd0 .git/logs/refs/heads/master The checksum is fine. 0035 You do not see the bug. |: Could you make a test script that illustrates and reproduces the problem? I.e., a patch to a file like t/t1410-reflog.sh [...] http://tron.yamagi.org/zeug/0001-t1410-Test-erroneous-skipping-of-reflog-entries.patch (also attached) This test works for me at v2.0.4 and fails at v2.1.0 and up (v2.2.0, the current master). Bisect says the symptom appears at 4207ed285f31ad3e04f08254237c0c1a1609642b. Christoph From 82115da194adc42143b8603063e0a419fbbf4928 Mon Sep 17 00:00:00 2001 From: Christoph Mallon christoph.mal...@gmx.de Date: Tue, 2 Dec 2014 07:03:11 +0100 Subject: [PATCH] t1410: Test erroneous skipping of reflog entries. --- t/t1410-reflog.sh | 63 +++ 1 file changed, 63 insertions(+) diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 8cf4461..cb77c27 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -287,4 +287,67 @@ test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' ' test_cmp expect actual ' +test_expect_success 'erroneous skipping of reflog entries' ' + git checkout -b reflogskip + cat .git/logs/refs/heads/reflogskip EOF +0037 0036 xxx@xxx 01 + X +0036 0035 xxx@xxx 01 + X +0035 0034 xxx@xxx 01 + +0034 0033 xxx@xxx 01 + XXX +0033 0032 xxx@xxx 01 + XX +0032 0031 xxx@xxx 01 + X +0031 0030 xxx@x 01 + XX +0030 002f xxx@x 01 + XXX +002f 002e xxx@x 01 + XXX +002e 002d xxx@x 01 + +002d 002c xxx@x 01 + XXX +002c 002b xxx@x 01 + XXX +002b 002a xxx@x 01 +
Re: git status / git diff -C not detecting file copy
On Sun, Nov 30, 2014 at 12:54:53PM +1100, Bryan Turner wrote: I'll let someone a little more intimately familiar with the internals of git status comment on why the documentation for that mentions copies. I don't think there is a good reason. git-status has used renames since mid-2005. The documentation mentioning copies was added much later, along with the short and porcelain formats. That code handles whatever the diff engine throws at it. I don't think anybody considered at that time the fact that you cannot actually provoke status to look for copies. Interestingly, the rename behavior dates all the way back to: commit 753fd78458b6d7d0e65ce0ebe7b62e1bc55f3992 Author: Linus Torvalds torva...@ppc970.osdl.org Date: Fri Jun 17 15:34:19 2005 -0700 Use -M instead of -C for git diff and git status The C in -C may stand for Cool, but it's also pretty slow, since right now it leaves all unmodified files to be tested even if there are no new files at all. That just ends up being unacceptably slow for big projects, especially if it's not all in the cache. I suspect that the copy code may be much faster these days (it sounds like we did not even have the find-copies-harder distinction then, and these days we certainly take the quick return if there are no copy destination candidates). To get a rough sense of how much effort is entailed in the various options, here are git log --raw timings for git.git (all timings are warm cache, best-of-five, wall clock time): log --raw: 0m2.311s log --raw -M:0m2.362s log --raw -C:0m2.576s log --raw -C -C: 1m4.462s You can see that rename detection adds a little, and copy detections adds a little more. That makes sense; it's rare for new files to appear at the same that old files are going away (renames), so most of the time it does nothing. Copies introduce a bit more work; we have to compare against any changed files, and there are typically several in each commit. find-copies-harder is...well, very expensive. These timings are of diffs between commits and their parents, of course. But if we assume that git status will show diffs roughly similar to what gets committed, then this should be comparable. There are about 30K non-merge commits we traversed there, so adding 200ms is an average of not very much per commit. Of course the cost is disproportionately borne by diffs which have an actual file come into being. There are ~2000 commits that introduce a file, so it's probably accurate to say that it either adds nothing in most cases, or ~1/10th of a millisecond in others. Note this is also doing inexact detection, which involves actually looking at the contents of candidate blobs (whereas exact detection can be done by comparing sha1s, which is very fast). If you set diff.renamelimit to 1, then we do only exact detections. Here are timings there: log --raw: 0m02.311s(for reference) log --raw -M:0m02.337s log --raw -C:0m02.347s log --raw -C -C: 0m24.419s That speeds things up a fair bit, even for -C (we don't have to access the blobs anymore, so I suspect the time is going to just accessing all of the trees; normally diff does not descend into subtrees that have the same sha1). Of course, you probably wouldn't want to turn off inexact renames completely. I suspect what you'd want is a --find-copies-moderately where we look for cheap copies using -C, and then follow up with -C -C only using exact renames. So from these timings, I'd conclude that: 1. It's probably fine to turn on copies for git status. 2. It's probably even OK to use -C -C for some projects. Even though 22s looks scary there, that's only 11ms for git.git (remember, spread across 2000 commits). For linux.git, it's much, much worse. I killed my -C -C run after 10 minutes, and it had only gone through 1/20th of the commits. Extrapolating, you're looking at 500ms or so added to a git status run. So you'd almost certainly want this to be configurable. Does either of you want to try your hand at a patch? Just enabling copies should be a one-liner. Making it configurable is more involved, but should also be pretty straightforward. -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] introduce git root
On Mon, Dec 01, 2014 at 05:17:22AM +0100, Christian Couder wrote: On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano gits...@pobox.com wrote: If I were redoing this today, I would probably nominate the git potty as such a kitchen synk command. We have --man-path that shows the location of the manual pages, --exec-path[=path] that either shows or allows us to override the path to the subcommands, and --show-prefix, --show-toplevel, and friends may feel quite at home there. I wonder if we could reuse git config which is already a kitchen synk command to get/set a lot of parameters. Maybe we could dedicate a git or virtual or proc or sys (like /proc or /sys in Linux) namespace for these special config parameters that would not necessarily reflect something in the config file. git config git.man-path would be the same as git --man-path. git config git.root would be the same as git rev-parse --show-toplevel. git config git.exec-path mypath would allow us to override the path to the subcommands, probably by saving something in the config file. What would: git config git.root foo git config git.root output? No matter what the answer is, I do not relish the thought of trying to explain it in the documentation. :) There is also git var, which is a catch-all for printing some deduced environmental defaults. I'd be just as happy to see it go away, though. Having: git --exec-path git --toplevel git --author-ident all work would make sense to me (I often get confused between git --foo and git rev-parse --foo when trying to get the exec-path and git-dir). And I don't think it's too late to move in this direction. We'd have to keep the old interfaces around, of course, but it would immediately improve discoverability and consistency. -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
[PATCH 4/4] reflog.c: use a reflog transaction when writing during expire
From: Ronnie Sahlberg sahlb...@google.com Use a transaction for all updates during expire_reflog. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- no changes since sending it 5 days ago. builtin/reflog.c | 86 1 file changed, 37 insertions(+), 49 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index 2d85d26..55f3023 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -33,7 +33,8 @@ struct cmd_reflog_expire_cb { }; struct expire_reflog_cb { - FILE *newlog; + struct transaction *t; + const char *refname; enum { UE_NORMAL, UE_ALWAYS, @@ -290,7 +291,7 @@ static int unreachable(struct expire_reflog_cb *cb, struct commit *commit, unsig static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1, const char *email, unsigned long timestamp, int tz, - const char *message, void *cb_data) + const char *message, void *cb_data, struct strbuf *err) { struct expire_reflog_cb *cb = cb_data; struct commit *old, *new; @@ -316,20 +317,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1, if (cb-cmd-recno --(cb-cmd-recno) == 0) goto prune; - if (cb-newlog) { - char sign = (tz 0) ? '-' : '+'; - int zone = (tz 0) ? (-tz) : tz; - fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s, - sha1_to_hex(osha1), sha1_to_hex(nsha1), - email, timestamp, sign, zone, - message); + if (cb-t) { + if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1, + email, timestamp, tz, message, 0, + err)) + return -1; hashcpy(cb-last_kept_sha1, nsha1); } if (cb-cmd-verbose) printf(keep %s, message); return 0; prune: - if (!cb-newlog) + if (!cb-t) printf(would prune %s, message); else if (cb-cmd-verbose) printf(prune %s, message); @@ -353,29 +352,27 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, { struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; - struct ref_lock *lock; - char *log_file, *newlog_path = NULL; struct commit *tip_commit; struct commit_list *tips; + struct strbuf err = STRBUF_INIT; int status = 0; memset(cb, 0, sizeof(cb)); + cb.refname = ref; - /* -* we take the lock for the ref itself to prevent it from -* getting updated. -*/ - lock = lock_any_ref_for_update(ref, sha1, 0, NULL); - if (!lock) - return error(cannot lock ref '%s', ref); - log_file = git_pathdup(logs/%s, ref); if (!reflog_exists(ref)) goto finish; - if (!cmd-dry_run) { - newlog_path = git_pathdup(logs/%s.lock, ref); - cb.newlog = fopen(newlog_path, w); + cb.t = transaction_begin(err); + if (!cb.t) { + status |= error(%s, err.buf); + goto cleanup; + } + if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1, + NULL, 0, 0, NULL, REFLOG_TRUNCATE, + err)) { + status |= error(%s, err.buf); + goto cleanup; } - cb.cmd = cmd; if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) { @@ -407,7 +404,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, mark_reachable(cb); } - for_each_reflog_ent(ref, expire_reflog_ent, cb); + if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) { + status |= error(%s, err.buf); + goto cleanup; + } if (cb.unreachable_expire_kind != UE_ALWAYS) { if (cb.unreachable_expire_kind == UE_HEAD) { @@ -420,32 +420,20 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, } } finish: - if (cb.newlog) { - if (fclose(cb.newlog)) { - status |= error(%s: %s, strerror(errno), - newlog_path); - unlink(newlog_path); - } else if (cmd-updateref - (write_in_full(lock-lock_fd, - sha1_to_hex(cb.last_kept_sha1), 40) != 40 || -write_str_in_full(lock-lock_fd, \n) != 1 || -close_ref(lock) 0)) { - status |= error(Couldn't write %s, -
[PATCH 1/4] refs.c: rename the transaction functions
From: Ronnie Sahlberg sahlb...@google.com Rename the transaction functions. Remove the leading ref_ from the names and append _ref to the names for functions that create/delete/ update sha1 refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- This commit can be reproduced via find . -name *.[ch] -print | xargs sed -i ' \ s/REF_TRANSACTION_OPEN/TRANSACTION_OPEN/; \ s/REF_TRANSACTION_CLOSED/TRANSACTION_CLOSED/; \ s/ref_transaction_begin/transaction_begin/; \ s/ref_transaction_commit/transaction_commit/; \ s/ref_transaction_create/transaction_create_ref/; \ s/ref_transaction_delete/transaction_delete_ref/; \ s/ref_transaction_free/transaction_free/; \ s/ref_transaction_update/transaction_update_ref/; \ s/ref_transaction/transaction/' modulo white space changes for alignment. No changes since sending it 5 days ago. branch.c | 13 + builtin/commit.c | 10 +++ builtin/fetch.c| 12 builtin/receive-pack.c | 13 - builtin/replace.c | 10 +++ builtin/tag.c | 10 +++ builtin/update-ref.c | 26 - fast-import.c | 22 +++--- refs.c | 78 +- refs.h | 36 +++ sequencer.c| 12 walker.c | 10 +++ 12 files changed, 126 insertions(+), 126 deletions(-) diff --git a/branch.c b/branch.c index 4bab55a..c8462de 100644 --- a/branch.c +++ b/branch.c @@ -279,16 +279,17 @@ void create_branch(const char *head, log_all_ref_updates = 1; if (!dont_change_ref) { - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref.buf, sha1, - null_sha1, 0, !forcing, msg, err) || - ref_transaction_commit(transaction, err)) + transaction_update_ref(transaction, ref.buf, sha1, + null_sha1, 0, !forcing, msg, + err) || + transaction_commit(transaction, err)) die(%s, err.buf); - ref_transaction_free(transaction); + transaction_free(transaction); strbuf_release(err); } diff --git a/builtin/commit.c b/builtin/commit.c index e108c53..f50b7df 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct stat statbuf; struct commit *current_head = NULL; struct commit_extra_header *extra = NULL; - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; if (argc == 2 !strcmp(argv[1], -h)) @@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix) strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg)); strbuf_insert(sb, strlen(reflog_msg), : , 2); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, HEAD, sha1, + transaction_update_ref(transaction, HEAD, sha1, current_head ? current_head-object.sha1 : NULL, 0, !!current_head, sb.buf, err) || - ref_transaction_commit(transaction, err)) { + transaction_commit(transaction, err)) { rollback_index_files(); die(%s, err.buf); } - ref_transaction_free(transaction); + transaction_free(transaction); unlink(git_path(CHERRY_PICK_HEAD)); unlink(git_path(REVERT_HEAD)); diff --git a/builtin/fetch.c b/builtin/fetch.c index 7b84d35..0be0b09 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -404,7 +404,7 @@ static int s_update_ref(const char *action, { char msg[1024]; char *rla = getenv(GIT_REFLOG_ACTION); - struct ref_transaction *transaction; + struct transaction *transaction; struct strbuf err = STRBUF_INIT; int ret, df_conflict = 0; @@ -414,23 +414,23 @@ static int s_update_ref(const char *action, rla = default_rla.buf; snprintf(msg, sizeof(msg), %s: %s, rla, action); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref-name,
[PATCHv2 0/4] refs.c: use transactions for the reflog
This is the core part of the refs-transactions-reflog series[1], which was in discussion for a bit already. The idea is to have the reflog being part of the transactions, which the refs are already using, so the we're moving towards a database like API in the long run. This makes git easier to maintain as well as opening the possibility to replace the backend with a real database. The first patch is essentially just some sed magic with reformatting the code, so the naming convention fits better, because the transactions will handle both the refs as well as the reflog after this series. The second patch is also just a rename patch. The meat and most of the lines of code are found in the 3rd patch. It was completely rewritten from scratch using ideas provided by Jonathan, thanks! We'll treat the refs and reflogs a bit differently within the transaction updates data structure, as the hard part for the refs is during the commit phase, while the hard part for the reflogs is during the preparation phase now. We're using a temporary file for creating the new reflog and rename it into place afterwards utilizing the lock file API. The last patch makes actually use of the reflog transaction API. Any comments would be appreciated, Thanks, Stefan -- 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] refs.c: rename transaction.updates to transaction.ref_updates
The updates are only holding refs not reflogs, so express it to the reader. Signed-off-by: Stefan Beller sbel...@google.com --- * only renaming, no extra code in this patch. * new to the series. refs.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/refs.c b/refs.c index f0f0d23..58de60c 100644 --- a/refs.c +++ b/refs.c @@ -3554,7 +3554,7 @@ enum transaction_state { * as atomically as possible. This structure is opaque to callers. */ struct transaction { - struct ref_update **updates; + struct ref_update **ref_updates; size_t alloc; size_t nr; enum transaction_state state; @@ -3575,10 +3575,10 @@ void transaction_free(struct transaction *transaction) return; for (i = 0; i transaction-nr; i++) { - free(transaction-updates[i]-msg); - free(transaction-updates[i]); + free(transaction-ref_updates[i]-msg); + free(transaction-ref_updates[i]); } - free(transaction-updates); + free(transaction-ref_updates); free(transaction); } @@ -3589,8 +3589,8 @@ static struct ref_update *add_update(struct transaction *transaction, struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1); strcpy((char *)update-refname, refname); - ALLOC_GROW(transaction-updates, transaction-nr + 1, transaction-alloc); - transaction-updates[transaction-nr++] = update; + ALLOC_GROW(transaction-ref_updates, transaction-nr + 1, transaction-alloc); + transaction-ref_updates[transaction-nr++] = update; return update; } @@ -3712,7 +3712,7 @@ int transaction_commit(struct transaction *transaction, int ret = 0, delnum = 0, i; const char **delnames; int n = transaction-nr; - struct ref_update **updates = transaction-updates; + struct ref_update **updates = transaction-ref_updates; assert(err); -- 2.2.0 -- 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] refs.c: add a transaction function to append a reflog entry
When performing a reflog transaction update, only write to the reflog iff msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform an update that only truncates but does not write. This change only affects whether or not a reflog entry should be generated and written. If msg == NULL then no such entry will be written. Orthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to tell the transaction system to truncate the reflog and thus discard all previous users. At the current time the only place where we use msg == NULL is also the place, where we use REFLOG_TRUNCATE. Even though these two settings are currently only ever used together it still makes sense to have them through two separate knobs. This allows future consumers of this API that may want to do things differently. For example someone can do: msg=Reflog truncated by Bob because ... + REFLOG_TRUNCATE and have it truncate the log and have it start fresh with an initial message that explains the log was truncated. This API allows that. During one transaction we allow to make multiple reflog updates to the same ref. This means we only need to lock the reflog once, during the first update that touches the reflog, and that all further updates can just write the reflog entry since the reflog is already locked. This allows us to write code such as: t = transaction_begin() transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL); loop-over-something... transaction_reflog_update(t, foo, 0, message); transaction_commit(t) where we first truncate the reflog and then build the new content one line at a time. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- This is a complete rewrite of the patch you would have found earlier on the list. The approach was changed to deal with the reflogs differently and not toss them into the array containing all the ref_updates, but keep them in a separate string list. As I am borrowing some text for the commit message and some ideas how to approach the problem, I still added Ronnies sign off. refs.c | 127 +++-- refs.h | 21 +++ 2 files changed, 146 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 58de60c..2861486 100644 --- a/refs.c +++ b/refs.c @@ -3557,6 +3557,12 @@ struct transaction { struct ref_update **ref_updates; size_t alloc; size_t nr; + + /* +* Sorted list of reflogs to be committed, +* the util points to the lock_file +*/ + struct string_list reflog_updates; enum transaction_state state; }; @@ -3564,7 +3570,10 @@ struct transaction *transaction_begin(struct strbuf *err) { assert(err); - return xcalloc(1, sizeof(struct transaction)); + struct transaction *ret = xcalloc(1, sizeof(struct transaction)); + string_list_init(ret-reflog_updates, 1); + + return ret; } void transaction_free(struct transaction *transaction) @@ -3629,6 +3638,112 @@ int transaction_update_ref(struct transaction *transaction, return 0; } +/* Returns a fd, -1 on error. */ +static int get_reflog_updates_fd(struct transaction *transaction, +const char *refname, +struct strbuf *err) +{ + char *path; + struct lock_file *lock; + struct string_list_item *item = string_list_insert( + transaction-reflog_updates, + refname); + if (!item-util) { + item-util = xcalloc(1, sizeof(struct lock_file)); + lock = item-util; + path = git_path(logs/locks/%s, refname); + if (safe_create_leading_directories(path)) { + strbuf_addf(err, + creating temporary directories %s failed., + path); + return -1; + } + + if (hold_lock_file_for_update(lock, path, 0) 0) { + strbuf_addf(err, + creating temporary file %s failed., + path); + return -1; + } + } + + lock = item-util; + + return lock-fd; +} + +int transaction_update_reflog(struct transaction *transaction, + const char *refname, + const unsigned char *new_sha1, + const unsigned char *old_sha1, + const char *email, + unsigned long timestamp, int tz, + const char *msg, int flags, + struct strbuf *err) +{ + /* +* We cannot handle reflogs the same way we handle refs because of +*