Re: [msysGit] [PATCH 07/14] Fix BASIC_LDFLAGS and COMPAT_CFLAGS for 64bit MinGW-w64
On Thu, Oct 09, 2014 at 09:22:19PM +0200, Johannes Schindelin wrote: > Hi, > > On Wed, 8 Oct 2014, Marat Radchenko wrote: > > > +CC_MACH := $(shell sh -c '$(CC) -dumpmachine 2>/dev/null || echo not') > > There is a rather huge problem with that. The latest mingw-w64 release, > 4.9.1, does not do what you expect here: while '.../mingw32/bin/gcc -m32 > -o 32.exe test.c' and '.../mingw32/bin/gcc -m64 -o 64.exe test.c' work > fine, producing i686 and x86_64 executables respectively, > '.../mingw32/bin/gcc -dumpmachine' prints i686-w64-mingw32 *always*, even > when specifying the -m64 option. > > So unfortunately, the test introduced by this patch (intended to figure > out whether the build targets i686, and skip a compiler and a linker > option otherwise) is incorrect. According to [1], it is by design. For now, I suggest using separate gcc binaries for 32/64, without messing with -m32. Of course we can fallback to `./configure` that will determine bitness by compiling something. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52096#c1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] "-x" tracing option for tests
On 10/13/2014 11:07 PM, Jeff King wrote: > On Mon, Oct 13, 2014 at 11:24:42AM -0700, Junio C Hamano wrote: >> [...] >> Is your plan to reroll the prune-mtime stuff on top of these? The >> additional safety those patches would give us is valuable and they >> are pretty straight-forward---I was hoping to have them in the 2.2 >> release. > > Yes, I've delayed while thinking about the issues that Michael raised. > There are basically two paths I see: > > 1. These do not solve all problems/races, but are a solid base and > sensible path forward for further changes which we can worry about > later. > > 2. There is a better way to provide prune safety, and these patches > will get in the way of doing that. > > I wanted to make sure we are on path (1) and not path (2). :) FWIW I think we are on path (1). Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
brian m. carlson wrote: On Mon, Oct 13, 2014 at 01:41:31PM -0700, Junio C Hamano wrote: "brian m. carlson" writes: The second two patches implement some basic support for building with Asciidoctor. The first of these moves some items into variables due to some differences between the AsciiDoc and Asciidoctor command lines. The user can then override these values when invoking make. The final patch adds support for the linkgit macro. Asciidoctor uses Ruby extensions to implement macro support, unlike AsciiDoc, which uses a configuration file. What I do not understand is that 3/4 lets you drop inclusion of asciidoc.conf which contains a lot more than just linkgit: definition. Asciidoctor just doesn't understand the -f argument, so trying to pass it is going to fail. For Asciidoctor, you're going to want to do something like "-I. -rasciidoctor/extensions -rextensions" there instead. As for the rest of the asciidoc.conf file, the DocBook manpage header declarations are implemented automatically by Asciidoctor after my recent patches. The paragraph hacks do not appear to be necessary with Asciidoctor, so they've been omitted. That leaves the attributes. All but litdd are built-in to Asciidoctor, and I can reroll with a modification to extensions.rb that implements that one. Would it be possible to automatically convert asciidoc.conf file to Asciidoctor extension? -- Jakub Narębski -- To unsubscribe from this list: send the line "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] [kernel] completion: silence "fatal: Not a git repository" error
It is possible that a user is trying to run a git command and fail to realize that they are not in a git repository or working tree. When trying to complete an operation, __git_refs would fall to a degenerate case and attempt to use "git for-each-ref", which would emit the error. Let's fix this by shunting the error message coming from "git for-each-ref". Signed-off-by: John Szakmeister --- contrib/completion/git-completion.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 5ea5b82..31b4739 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -388,7 +388,8 @@ __git_refs () ;; *) echo "HEAD" - git for-each-ref --format="%(refname:short)" -- "refs/remotes/$dir/" | sed -e "s#^$dir/##" + git for-each-ref --format="%(refname:short)" -- \ + "refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##" ;; esac } -- 2.0.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
On Tue, Oct 14, 2014 at 12:07:51PM +0200, Jakub Narębski wrote: > brian m. carlson wrote: > >On Mon, Oct 13, 2014 at 01:41:31PM -0700, Junio C Hamano wrote: > >> > >>What I do not understand is that 3/4 lets you drop inclusion of > >>asciidoc.conf which contains a lot more than just linkgit: > >>definition. > > > >Asciidoctor just doesn't understand the -f argument, so trying to pass > >it is going to fail. For Asciidoctor, you're going to want to do > >something like "-I. -rasciidoctor/extensions -rextensions" there > >instead. > > > >As for the rest of the asciidoc.conf file, the DocBook manpage header > >declarations are implemented automatically by Asciidoctor after my > >recent patches. The paragraph hacks do not appear to be necessary with > >Asciidoctor, so they've been omitted. > > > >That leaves the attributes. All but litdd are built-in to Asciidoctor, > >and I can reroll with a modification to extensions.rb that implements > >that one. > > Would it be possible to automatically convert asciidoc.conf file to > Asciidoctor extension? It is in theory possible, but it's going to result in a lot of messy code. I'm also not sure that Junio wants more than the minimal amount of Ruby possible, since the goal has been to move away from scripting languages and to C. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] Initialise hash variable to prevent compiler warnings
On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano wrote: > On Mon, Oct 13, 2014 at 2:53 PM, Felipe Franciosi wrote: >> >> On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano wrote: >>> >>> FNV/I/IDIV10/0 covers all the possibilities of (method & 3), I would >>> have to say that the compiler needs to be fixed. >>> >>> Or insert "default:" just before "case HASH_METHOD_0:" line? >>> >>> I dunno. >> >> Hmm... The "default:" would work, but is it really that bad to initialise a >> local variable in this case? >> >> In any case, the compilation warning is annoying. Do you prefer the default >> or the initialisation? > > If I really had to choose between the two, adding a useless initialization > would be the less harmful choice. Adding a meaningless "default:" robs > another chance from the compilers to diagnose a future breakage we > might add (namely, we may extend methods and forget to write a > corresponding case arm for the new method value, which a smart > compiler can and do diagnose as a switch that does not handle > all the possible values. > > Thanks. I see your point; the code is correct today because it covers all cases. Nevertheless, some versions of gcc (the one I used was 4.1.2 from CentOS 5.10 -- haven't tested others) might generate an annoying warning. Noting that, I also like my code to compile as cleanly as possible in all environments that it might be used. Being a bit defensive in that sense and initialising local variables is what I would do. On top of that (and putting the compiler flaw aside for a moment), having it sensibly initialised is another way of protecting the code against errors introduced in the future. What do you think? Cheers, F. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
On Sun, Oct 12, 2014 at 12:13 PM, Max Kirillov wrote: > These are fixes of issues with submodules with use of multiple working > trees. I think the patches look fine from the nd/multiple-work-trees writer's perspective. I know too little about submodules to judge if this is the right way and not that way.. -- 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 0/3] "-x" tracing option for tests
On Tue, Oct 14, 2014 at 10:52:30AM +0200, Michael Haggerty wrote: > >> Is your plan to reroll the prune-mtime stuff on top of these? The > >> additional safety those patches would give us is valuable and they > >> are pretty straight-forward---I was hoping to have them in the 2.2 > >> release. > > > > Yes, I've delayed while thinking about the issues that Michael raised. > > There are basically two paths I see: > > > > 1. These do not solve all problems/races, but are a solid base and > > sensible path forward for further changes which we can worry about > > later. > > > > 2. There is a better way to provide prune safety, and these patches > > will get in the way of doing that. > > > > I wanted to make sure we are on path (1) and not path (2). :) > > FWIW I think we are on path (1). Good. :) I was preparing this to re-send, but I realized there is one snag. I mentioned that we should probably be ignoring already-broken links from recent objects to missing objects. For the traversal in pack-objects, we can use revs->ignore_missing_links for this. But for the one in git-prune itself, we use mark_reachable, which does not respect that option. I think mark_reachable's traversal is essentially the same as the one in list-objects.c, and the two can be merged. I'll look into that, but I ran out of time for it tonight (er, this morning. Oops). -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: [RFC/PATCH] fsck: do not canonicalize modes in trees we are checking
On Mon, Oct 13, 2014 at 09:37:27AM +1100, Ben Aveling wrote: > A question about fsck - is there a reason it doesn't have an option to > delete bad objects? If the objects are reachable, then deleting them would create other big problems (i.e., we would be breaking the object graph!). If they are not, then it is probably safest for them to go away via the normal means (repack/prune via "git gc"). Deleting via fsck would mean replicating the reachability and deletion code found elsewhere. -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 0/4] Allow building Git with Asciidoctor
On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote: > This series is designed to implement the changes necessary to build Git > using Asciidoctor instead of AsciiDoc. Thanks. I had always taken the attitude that we wrote for the original Python AsciiDoc, and that using AsciiDoctor was a choice that git-scm.com made, and something they would have to deal with as far as compatibility (AFAIK, AsciiDoctor grew out of git-scm.com's home-grown asciidoc parser). What's the status on AsciiDoc versus AsciiDoctor? The latter seems more actively developed these days, but perhaps that is just my perception. The incompatibilities seem fairly minimal (if those first two patches are the extent of it, I have no problem at all trying to remain compatible with both). Would it ever make sense to switch to AsciiDoctor as our official command-line build program? I know it is supposed to be much faster (though a lot of the slowness in our build chain is due to docbook, not asciidoc itself). Specifically I'm not excited about getting into a state where we have to maintain both an asciidoc.conf file _and_ ruby extensions for asciidoctor. I don't mind if somebody wants to step up and keep the asciidoctor bits in sync with the asciidoc.conf, but I feel like one of them needs to be considered the "master". -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] [kernel] completion: silence "fatal: Not a git repository" error
On Tue, Oct 14, 2014 at 6:49 AM, John Szakmeister wrote: > It is possible that a user is trying to run a git command and fail to realize > that they are not in a git repository or working tree. When trying to > complete > an operation, __git_refs would fall to a degenerate case and attempt to use > "git for-each-ref", which would emit the error. > > Let's fix this by shunting the error message coming from "git for-each-ref". > > Signed-off-by: John Szakmeister > --- Sorry for the "[kernel]" in the subject. I must have forgotten to remove that off of my format-patch invocation. If you need me to resubmit without it, I can do that. Thanks! -John -- To unsubscribe from this list: send the line "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] receive-pack: plug minor memory leak in unpack()
On Mon, Oct 13, 2014 at 12:08:09PM -0700, Junio C Hamano wrote: > > I wonder if run-command should provide a managed env array similar > > to the "args" array. > > I took a look at a few of them: I took a brief look, too. I had hoped we could just all it "env" and everybody would be happy using it instead of bare pointers. But quite a few callers assign "local_repo_env" to to child_process.env. We could copy it into an argv_array, of course, but that really feels like working around the interface. So I think we would prefer to keep both forms available. That raises the question: what should it be called? The "argv_array" form of "argv" is called "args". The more I see it, the more I hate that name, as the two are easily confused. We could have: const char **argv; struct argv_array argv_array; const char **env; struct argv_array env_array; though "argv_array" is a lot to type when you have a string of argv_array_pushf() calls (which are already by themselves kind of verbose). Maybe that's not too big a deal, though. We could flip it to give the managed version the short name (and calling the unmanaged version "env_ptr" or something). That would require munging the existing callers, but the tweak would be simple. > - daemon.c::handle() uses a static set of environment variables >that are not built with argv_array(). Same for argv. Most of the callers you mentioned are good candidates. This one is tricky. The argv array gets malloc'd and set up by the parent git-daemon process. Then each time we get a client, we create a new struct child_process that references it. So using the managed argv-array would actually be a bit more complicated (and not save any memory; we just always point to the single copy for each child). For the environment, we build it in a function-local buffer, point the child_process's env field at it, start the child, and then copy the child_process into our global list of children. When we notice a child is dead (by linearly going through the list with waitpid), we free the list entry. So there are a few potentially bad things here: 1. We memcpy the child_process to put it on the list. Which does work, though it feels a little like we are violating the abstraction barrier. 2. The child_process in the list points to the local "env" buffer that is no longer valid. There's no bug because we don't ever look at it. Moving to a managed env would fix that. But I have to wonder if we even want to be keeping the "struct child_process" around in the first place (all we really care about is the pid). 3. If we do move to a managed env, then we expect it to get cleaned up in finish_command. But we never call that; we just free the list memory containing the child_process. We would want to call finish_command. Except that we will have reaped the process already with our call to waitpid() from check_dead_children. So we'd need a new call to do just the cleanup without the wait, I guess. 4. For every loop on the listen socket, we call waitpid() for each living child, which is a bit wasteful. We'd probably do better to call waitpid(0, &status, WNOHANG), and then look up the resulting pid in a hashmap or sorted list when we actually see something that died. I don't know that this is a huge problem in practice. We use git-daemon pretty heavily on our backend servers at GitHub, and it seems to use about 5% of a CPU constantly on each machine. Which is kind of lame, given that it isn't doing anything at all, but is hardly earth-shattering. So I'm not sure if it is worth converting to a managed env. There's a bit of ugliness, but none of it is causing any problems, and doing so opens a can of worms. The most interesting thing to fix (to me, anyway) is number 4, but that has nothing to do with the env in the first place. :) -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 2/2] Remove spurious 'no threads support' warnings
On Mon, Oct 13, 2014 at 9:54 PM, Junio C Hamano wrote: > Etienne Buira writes: > >> Threads count being defaulted to 0 (autodetect), and --disable-pthreads >> build checking that thread count==1, there were spurious warnings about >> threads being ignored, despite not specified on command line/conf. >> >> Fixes tests 5521 and 5526 that were broken in --disable-pthreads builds >> because of those warnings. >> >> Signed-off-by: Etienne Buira >> --- > > I am not sure if this is the right fix. > > Shouldn't a --threads=0 from the command line (when there is a > pack.threads configuration hardcoding some number to override it) > give a chance to the auto detection codepath to ask online_cpus() > and receive 1 on NO_PTHREADS build to avoid triggering the same > warning you are squelching with this patch? > > That is, something like this instead, perhaps? Indeed, your patch is better. > -- >8 -- > Subject: [PATCH] pack-objects: set number of threads before checking and > warning > > Under NO_PTHREADS build, we warn when delta_search_threads is not > set to 1, because that is the only sensible value on a single > threaded build. > > However, the auto detection that kicks in when that variable is set > to 0 (e.g. there is no configuration variable or command line option, > or an explicit --threads=0 is given from the command line to override > the pack.threads configuration to force auto-detection) was not done > before the condition to issue this warning was tested. > > Move the auto-detection code and place it at an appropriate spot. > > Signed-off-by: Junio C Hamano > --- > builtin/pack-objects.c | 6 -- > thread-utils.h | 4 > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c > index d391934..a715237 100644 > --- a/builtin/pack-objects.c > +++ b/builtin/pack-objects.c > @@ -1972,8 +1972,6 @@ static void ll_find_deltas(struct object_entry **list, > unsigned list_size, > > init_threaded_search(); > > - if (!delta_search_threads) /* --threads=0 means autodetect */ > - delta_search_threads = online_cpus(); > if (delta_search_threads <= 1) { > find_deltas(list, &list_size, window, depth, processed); > cleanup_threaded_search(); > @@ -2685,6 +2683,10 @@ int cmd_pack_objects(int argc, const char **argv, > const char *prefix) > pack_compression_level = Z_DEFAULT_COMPRESSION; > else if (pack_compression_level < 0 || pack_compression_level > > Z_BEST_COMPRESSION) > die("bad pack compression level %d", pack_compression_level); > + > + if (!delta_search_threads) /* --threads=0 means autodetect */ > + delta_search_threads = online_cpus(); > + > #ifdef NO_PTHREADS > if (delta_search_threads != 1) > warning("no threads support, ignoring --threads"); > diff --git a/thread-utils.h b/thread-utils.h > index 6fb98c3..d9a769d 100644 > --- a/thread-utils.h > +++ b/thread-utils.h > @@ -7,5 +7,9 @@ > extern int online_cpus(void); > extern int init_recursive_mutex(pthread_mutex_t*); > > +#else > + > +#define online_cpus() 1 > + > #endif > #endif /* THREAD_COMPAT_H */ > -- > 2.1.2-468-g1a77c5b > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Handle atexit list internaly for unthreaded builds
On Mon, Oct 13, 2014 at 10:00 PM, Junio C Hamano wrote: > Etienne Buira writes: > >> Wrap atexit()s calls on unthreaded builds to handle callback list >> internally. >> >> This is needed because on unthreaded builds, asyncs inherits parent's >> atexit() list, that gets run as soon as the async exit()s (and again at >> the end of the parent process). That led to remove temporary and lock >> files too early. > > ... that does not explain what you did to builtin/clone.c at all. > Care to enlighten us, please? Checking current pid against the one that registered the atexit() routine (what builtin/clone.c currently does) is another way to guard against the same issue, but it needs to store a pid for each atexit() call whenever code called after might use async. >From what I've seen, two out of all atexit() calls were guarded like that: - builtin/clone.c:cmd_clone - lockfile.c:lock_file (overlooked it at first, would you mind to s/temporary and lock files/temporary files/ the commit message if no other round is needed? I'll do it otherwise). So the changes in builtin/clone.c are just there because the patch makes this check redundant (still needed in lockfile.c, as it loops over a list that might refer to parent process's lockfiles). >> Fixes test 5537 (temporary shallow file vanished before unpack-objects >> could open it) >> >> V4: fix bogus preprocessor directives > > Please do not write this "V4:" line in the log message. People who > read "git log" output and find this description would not know > anything about the previous faulty ones. Putting it _after_ the > three-dash line below will help the reviewers a lot. > >> >> Thanks-to: Duy Nguyen >> Thanks-to: Andreas Schwab > > Usually we spell these "Helped-by: " instead. > >> Signed-off-by: Etienne Buira >> --- > > Thanks. > >> builtin/clone.c | 5 - >> git-compat-util.h | 5 + >> run-command.c | 40 >> shallow.c | 7 ++- >> 4 files changed, 47 insertions(+), 10 deletions(-) >> >> diff --git a/builtin/clone.c b/builtin/clone.c >> index bbd169c..e122f33 100644 >> --- a/builtin/clone.c >> +++ b/builtin/clone.c >> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char >> *dest_repo) >> >> static const char *junk_work_tree; >> static const char *junk_git_dir; >> -static pid_t junk_pid; >> static enum { >> JUNK_LEAVE_NONE, >> JUNK_LEAVE_REPO, >> @@ -417,8 +416,6 @@ static void remove_junk(void) >> break; >> } >> >> - if (getpid() != junk_pid) >> - return; >> if (junk_git_dir) { >> strbuf_addstr(&sb, junk_git_dir); >> remove_dir_recursively(&sb, 0); >> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char >> *prefix) >> struct refspec *refspec; >> const char *fetch_pattern; >> >> - junk_pid = getpid(); >> - >> packet_trace_identity("clone"); >> argc = parse_options(argc, argv, prefix, builtin_clone_options, >>builtin_clone_usage, 0); >> diff --git a/git-compat-util.h b/git-compat-util.h >> index f587749..6dd63dd 100644 >> --- a/git-compat-util.h >> +++ b/git-compat-util.h >> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst); >> const char *inet_ntop(int af, const void *src, char *dst, size_t size); >> #endif >> >> +#ifdef NO_PTHREADS >> +#define atexit git_atexit >> +extern int git_atexit(void (*handler)(void)); >> +#endif >> + >> extern void release_pack_memory(size_t); >> >> typedef void (*try_to_free_t)(size_t); >> diff --git a/run-command.c b/run-command.c >> index 35a3ebf..0f9a9b0 100644 >> --- a/run-command.c >> +++ b/run-command.c >> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void) >> return ret != NULL; >> } >> >> +#else >> + >> +static struct { >> + void (**handlers)(void); >> + size_t nr; >> + size_t alloc; >> +} git_atexit_hdlrs; >> + >> +static int git_atexit_installed; >> + >> +static void git_atexit_dispatch() >> +{ >> + size_t i; >> + >> + for (i=git_atexit_hdlrs.nr ; i ; i--) >> + git_atexit_hdlrs.handlers[i-1](); >> +} >> + >> +static void git_atexit_clear() >> +{ >> + free(git_atexit_hdlrs.handlers); >> + memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs)); >> + git_atexit_installed = 0; >> +} >> + >> +#undef atexit >> +int git_atexit(void (*handler)(void)) >> +{ >> + ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, >> git_atexit_hdlrs.alloc); >> + git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler; >> + if (!git_atexit_installed) { >> + if (atexit(&git_atexit_dispatch)) >> + return -1; >> + git_atexit_installed = 1; >> + } >> + return 0; >> +} >> +#define atexit git_atexit >> + >> #endif >> >> int start_async(struct async *async) >> @@ -682,6 +721,7 @@ int start_async(struct async *async) >>
Re: [PATCH] Initialise hash variable to prevent compiler warnings
On Tue, Oct 14, 2014 at 4:44 AM, Felipe Franciosi wrote: > On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano wrote: >> >> If I really had to choose between the two, adding a useless initialization >> would be the less harmful choice. Adding a meaningless "default:" robs >> ... > Being a bit defensive in that > sense and initialising local variables is what I would do. On top of > that (and putting the compiler flaw aside for a moment), having it > sensibly initialised is another way of protecting the code against > errors introduced in the future. That is a false sense of safety. You will not know if the new method introduced in the future would behave sensibly if the variable is left in a state the blanket initialization created, so setting it to 0 upfront is not really being defensive; you would rob compilers a chance to notice something is amiss in the future code with the initialization, just like a "default:" would. We need to accept that both are not about being defensive but are ways to work around stupid compilers from reporting false positives. I am not saying that we should not do a work around. I am only saying that it is wrong to try selling such a work around as a defensive good practice, which is not. > What do you think? Again, if I really had to choose between the two, adding a useless initialization would be the less harmful choice, as the other one has an extra downside. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
Jeff King writes: > On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote: > > Specifically I'm not excited about getting into a state where we have to > maintain both an asciidoc.conf file _and_ ruby extensions for > asciidoctor. I don't mind if somebody wants to step up and keep the > asciidoctor bits in sync with the asciidoc.conf, but I feel like one of > them needs to be considered the "master". My so-far-unstated inclination, since seeing the patch to fix the unbalanced example block separators from Brian (which was outside and before this four-patch series), has been to keep our Makefile in Documentation/ aware only of AsciiDoc while maintaining *.txt files in a state so that AsciiDoctor could also be used to process them, if people want to futz with their copies of Documentation/Makefile. I do not mind to have the machinery to run AsciiDoctor too much in my tree. It may make it easier for those who use it to spot places in *.txt that need (in)compatibility workarounds between the two formatters than keeping it outside. But somebody needs to maintain that machinery and that will not be me. 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: [PATCH 0/4] Multiple worktrees vs. submodules fixes
Am 14.10.2014 um 14:17 schrieb Duy Nguyen: On Sun, Oct 12, 2014 at 12:13 PM, Max Kirillov wrote: These are fixes of issues with submodules with use of multiple working trees. I think the patches look fine from the nd/multiple-work-trees writer's perspective. I know too little about submodules to judge if this is the right way and not that way.. Sorry, I was too busy to review this work until now, but here we go: If I understand multiple work trees correctly, everything except work tree local stuff is redirected into GIT_COMMON_DIR. This is a very cool feature I'd love to see on our CI server to reduce disk footprint and clone times, especially for submodules! But I can't see how that can work by just sharing the modules directory tree, as that contains work tree related files - e.g. the index - for each submodule. AFAICS sharing them between work trees will work only if the content of the modules directory is partly present in GIT_DIR - for work tree related files - and only the common stuff is taken from GIT_COMMON_DIR (Or did I just miss the magic that already does that?). And I didn't try to wrap my head around recursive submodules yet ... Until that problem is solved it looks wrong to pass GIT_COMMON_DIR into submodule recursion, I believe GIT_COMMON_DIR should be added to the local_repo_env array (and even if it is passed on later, we might have to append "/modules/" to make it point to the correct location). But maybe I'm missing something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
Jens Lehmann writes: > But I can't see how that can work by just sharing the modules directory > tree, as that contains work tree related files - e.g. the index - for > each submodule. AFAICS sharing them between work trees will work only > if the content of the modules directory is partly present in GIT_DIR - > for work tree related files - and only the common stuff is taken from > GIT_COMMON_DIR (Or did I just miss the magic that already does that?). The first time I saw the patch 3/4 in this series, my reaction was "Huh, why should the repository data and branch tips be separated out into multiple independent copies for the same module? Do we force users to synchronise between these copies? It does not make any sense at all." But that was until I read your message ;-) You are right that the index and HEAD are dependent to a particular working tree that is checked out. There may be other things that logically are per- working tree. And multiple-worktree _is_ about keeping the same repository and history data (i.e. object database, refs, rerere database, reflogs for refs/*) only once, while allowing multiple working trees attached to that single copy. So it appears to me that to create a checkout-to copy of a superproject with a submodule, a checkout-to copy of the superproject would have a submodule, which is a checkout-to copy of the submodule in the superproject. > And I didn't try to wrap my head around recursive submodules yet ... > > Until that problem is solved it looks wrong to pass GIT_COMMON_DIR into > submodule recursion, I believe GIT_COMMON_DIR should be added to the > local_repo_env array (and even if it is passed on later, we might have > to append "/modules/" to make it point to the correct > location). -- To unsubscribe from this list: send the line "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] pass config slots as pointers instead of offsets
From: Jonathan Nieder Date: Tue, 7 Oct 2014 15:16:57 -0400 Many config-parsing helpers, like parse_branch_color_slot, take the name of a config variable and an offset to the "slot" name (e.g., "color.branch.plain" is passed along with "13" to effectively pass "plain"). This is leftover from the time that these functions would die() on error, and would want the full variable name for error reporting. These days they do not use the full variable name at all. Passing a single pointer to the slot name is more natural, and lets us more easily adjust the callers to use skip_prefix to avoid manually writing offset numbers. This is effectively a continuation of 9e1a5eb, which did the same for parse_diff_color_slot. This patch covers all of the remaining similar constructs. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- * Can we have your Sign-off (and a quick eyeballing again), please? builtin/branch.c | 16 builtin/commit.c | 19 +-- builtin/log.c| 2 +- log-tree.c | 4 ++-- log-tree.h | 2 +- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0591b22..b2e1895c 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -62,19 +62,19 @@ static unsigned char merge_filter_ref[20]; static struct string_list output = STRING_LIST_INIT_DUP; static unsigned int colopts; -static int parse_branch_color_slot(const char *var, int ofs) +static int parse_branch_color_slot(const char *slot) { - if (!strcasecmp(var+ofs, "plain")) + if (!strcasecmp(slot, "plain")) return BRANCH_COLOR_PLAIN; - if (!strcasecmp(var+ofs, "reset")) + if (!strcasecmp(slot, "reset")) return BRANCH_COLOR_RESET; - if (!strcasecmp(var+ofs, "remote")) + if (!strcasecmp(slot, "remote")) return BRANCH_COLOR_REMOTE; - if (!strcasecmp(var+ofs, "local")) + if (!strcasecmp(slot, "local")) return BRANCH_COLOR_LOCAL; - if (!strcasecmp(var+ofs, "current")) + if (!strcasecmp(slot, "current")) return BRANCH_COLOR_CURRENT; - if (!strcasecmp(var+ofs, "upstream")) + if (!strcasecmp(slot, "upstream")) return BRANCH_COLOR_UPSTREAM; return -1; } @@ -88,7 +88,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) return 0; } if (starts_with(var, "color.branch.")) { - int slot = parse_branch_color_slot(var, 13); + int slot = parse_branch_color_slot(var + 13); if (slot < 0) return 0; if (!value) diff --git a/builtin/commit.c b/builtin/commit.c index 5ed6036..5a8a29e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1238,22 +1238,21 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix, return commitable ? 0 : 1; } -static int parse_status_slot(const char *var, int offset) +static int parse_status_slot(const char *slot) { - if (!strcasecmp(var+offset, "header")) + if (!strcasecmp(slot, "header")) return WT_STATUS_HEADER; - if (!strcasecmp(var+offset, "branch")) + if (!strcasecmp(slot, "branch")) return WT_STATUS_ONBRANCH; - if (!strcasecmp(var+offset, "updated") - || !strcasecmp(var+offset, "added")) + if (!strcasecmp(slot, "updated") || !strcasecmp(slot, "added")) return WT_STATUS_UPDATED; - if (!strcasecmp(var+offset, "changed")) + if (!strcasecmp(slot, "changed")) return WT_STATUS_CHANGED; - if (!strcasecmp(var+offset, "untracked")) + if (!strcasecmp(slot, "untracked")) return WT_STATUS_UNTRACKED; - if (!strcasecmp(var+offset, "nobranch")) + if (!strcasecmp(slot, "nobranch")) return WT_STATUS_NOBRANCH; - if (!strcasecmp(var+offset, "unmerged")) + if (!strcasecmp(slot, "unmerged")) return WT_STATUS_UNMERGED; return -1; } @@ -1291,7 +1290,7 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; } if (starts_with(k, "status.color.") || starts_with(k, "color.status.")) { - int slot = parse_status_slot(k, 13); + int slot = parse_status_slot(k + 13); if (slot < 0) return 0; if (!v) diff --git a/builtin/log.c b/builtin/log.c index 4389722..4c5fc4b 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -389,7 +389,7 @@ static int git_log_config(const char *var, const char *value, void *cb) return 0; } if (starts_with(var, "color.decorate.")) - return parse_decorate_color_config(var, 15, value); + return parse_decorate_color_config(var, var + 15, value); if (!strcmp(var, "log.ma
Re: [PATCH] pass config slots as pointers instead of offsets
Junio C Hamano wrote: > builtin/branch.c | 16 > builtin/commit.c | 19 +-- > builtin/log.c| 2 +- > log-tree.c | 4 ++-- > log-tree.h | 2 +- > 5 files changed, 21 insertions(+), 22 deletions(-) Signed-off-by: Jonathan Nieder -- To unsubscribe from this list: send the line "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] [kernel] completion: silence "fatal: Not a git repository" error
John Szakmeister writes: > It is possible that a user is trying to run a git command and fail to realize > that they are not in a git repository or working tree. When trying to > complete > an operation, __git_refs would fall to a degenerate case and attempt to use > "git for-each-ref", which would emit the error. > > Let's fix this by shunting the error message coming from "git for-each-ref". Hmph, do you mean this one? $ cd /var/tmp ;# not a git repository $ git checkout -> $ git checkout fatal: Not a git repository (or any of the parent directories): .git HEAD I agree it is ugly, but would it be an improvement for the end user, who did not realize that she was not in a directory where "git checkout" makes sense, not to tell her that she is not in a git repository in some way? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
On Tue, Oct 14, 2014 at 10:26:42AM -0700, Junio C Hamano wrote: > And multiple-worktree _is_ about keeping the same repository and > history data (i.e. object database, refs, rerere database, reflogs for > refs/*) only once, while allowing multiple working trees attached to > that single copy. > > So it appears to me that to create a checkout-to copy of a > superproject with a submodule, a checkout-to copy of the superproject > would have a submodule, which is a checkout-to copy of the submodule > in the superproject. That's right, this linking should be more implicit. But here are a lot of nuances. For example, it makes sense to have a superproject checkout without submodules being initialized (so that they don't waste space and machine time for working tree, which often is more than repository data). And it may happen so that this checkout is the master repository for superproject checkouts. But this should not prevent users from using initialized submodules in other checkouts. Then, a checkout copy of a submodule can be standalone (for example, git and git-html-docs are submodules of msysgit). Or, it can even belong to some other superproject. And in that cases they still should be able to be linked. Considering all above, and also the thing that I am quite new to submodules (but have to use them currently), I did not intend to create any new UI, only to make backend handle the already existing linked checkouts, which can be made manually. -- Max -- To unsubscribe from this list: send the line "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-prompt.sh: Hide prompt for ignored pwd
Am 14.10.2014 um 04:32 schrieb Jess Austin: > diff --git a/contrib/completion/git-prompt.sh > b/contrib/completion/git-prompt.sh > index c5473dc..d7559ff 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -84,6 +84,11 @@ > # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on > # the colored output of "git status -sb" and are available only when > # using __git_ps1 for PROMPT_COMMAND or precmd. > +# > +# If you would like __git_ps1 to do nothing in the case when the current > +# directory is set up to be ignored by git, then set > +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set > +# bash.hideOnIgnoredPwd to true in the repository configuration. > > # check whether printf supports -v > __git_printf_supports_v= > @@ -501,6 +506,13 @@ __git_ps1 () > local f="$w$i$s$u" > local gitstring="$c$b${f:+$z$f}$r$p" > > + if [ -n "$(git check-ignore .)" ] && > +( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] || > + [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] ) Ahem, no. Please do not punish users who are not interested in the new feature with two new processes every time __git_ps() is run. Think of Windows where fork() is really, *really* expensive. BTW, you can write '{ foo || bar; }' to bracket a || chain without a sub-process. > + then > + printf_format="" > + fi > + > if [ $pcmode = yes ]; then > if [ "${__git_printf_supports_v-}" != yes ]; then > gitstring=$(printf -- "$printf_format" "$gitstring") -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-prompt.sh: Hide prompt for ignored pwd
On 2014-10-14 14:47, Johannes Sixt wrote: > Am 14.10.2014 um 04:32 schrieb Jess Austin: >> diff --git a/contrib/completion/git-prompt.sh >> b/contrib/completion/git-prompt.sh >> index c5473dc..d7559ff 100644 >> --- a/contrib/completion/git-prompt.sh >> +++ b/contrib/completion/git-prompt.sh >> @@ -84,6 +84,11 @@ >> # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on >> # the colored output of "git status -sb" and are available only when >> # using __git_ps1 for PROMPT_COMMAND or precmd. >> +# >> +# If you would like __git_ps1 to do nothing in the case when the current >> +# directory is set up to be ignored by git, then set >> +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set >> +# bash.hideOnIgnoredPwd to true in the repository configuration. >> >> # check whether printf supports -v >> __git_printf_supports_v= >> @@ -501,6 +506,13 @@ __git_ps1 () >> local f="$w$i$s$u" >> local gitstring="$c$b${f:+$z$f}$r$p" >> >> +if [ -n "$(git check-ignore .)" ] && >> + ( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] || >> + [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] ) > > Ahem, no. Please do not punish users who are not interested in the new > feature with two new processes every time __git_ps() is run. Think of > Windows where fork() is really, *really* expensive. Is this why bash.showDirtyState and friends aren't checked unless the corresponding environment variable is set to a non-empty value? Regardless, it would be nice if the behavior matched the other bash.* variables (only check the bash.* variable if the corresponding environment variable is set, and default to true). The following should fix it: if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] && [ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] && [ "$(git check-ignore .)" ] then ... -Richard > > BTW, you can write '{ foo || bar; }' to bracket a || chain without a > sub-process. > >> +then >> +printf_format="" >> +fi >> + >> if [ $pcmode = yes ]; then >> if [ "${__git_printf_supports_v-}" != yes ]; then >> gitstring=$(printf -- "$printf_format" "$gitstring") > > -- Hannes > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [kernel] completion: silence "fatal: Not a git repository" error
On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano wrote: > John Szakmeister writes: > >> It is possible that a user is trying to run a git command and fail to realize >> that they are not in a git repository or working tree. When trying to >> complete >> an operation, __git_refs would fall to a degenerate case and attempt to use >> "git for-each-ref", which would emit the error. >> >> Let's fix this by shunting the error message coming from "git for-each-ref". > > Hmph, do you mean this one? > > $ cd /var/tmp ;# not a git repository > $ git checkout > > -> > > $ git checkout fatal: Not a git repository (or any of the parent > directories): .git > HEAD > > I agree it is ugly, but would it be an improvement for the end user, > who did not realize that she was not in a directory where "git checkout" > makes sense, not to tell her that she is not in a git repository in > some way? I had thought about that too, but I think--for me--it comes down to two things: 1) We're not intentionally trying to inform the user anywhere else that they are not in a git repo. We simply fail to complete anything, which I think is an established behavior. 2) It mingles with the stuff already on the command line, making it confusing to know what you typed. Then you end up ctrl-c'ing your way out of it and starting over--which is the frustrating part. For me, I thought it better to just be more well-behaved. I've also run across this issue when I legitimately wanted to do something--I wish I could remember what it was--with a remote repo and didn't happen to be in a git working tree. It was frustrating to see this error message then too, for the same reason as above. I use tab completion quite extensively, so spitting things like this out making it difficult to move forward is a problem. Would it be better to check that "$dir" is non-empty and then provide the extra bits of information? We could then avoid giving the user anything in that case. -John -- To unsubscribe from this list: send the line "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-prompt.sh: Hide prompt for ignored pwd
On 2014-10-13 22:32, Jess Austin wrote: > Set __git_ps1 to display nothing when present working directory is > ignored, triggered by either the new environmental variable > GIT_PS1_HIDE_ON_IGNORED_PWD or the new repository configuration > variable bash.hideOnIgnoredPwd (or both). In the absence of these > settings this change has no effect. > > Many people manage e.g. dotfiles in their home directory with git. > This causes the prompt generated by __git_ps1 to refer to that "top > level" repo while working in any descendant directory. That can be > distracting, so this patch helps one shut off that noise. > > Signed-off-by: Jess Austin > --- > On Thu, Oct 9, 2014 at 5:09 PM, Richard Hansen wrote: >> On 2014-10-09 06:27, Jess Austin wrote: >>> Would you want this configured in each repo (i.e. via a line in >>> ".git/config"), >>> or would you prefer something global so that it only need be set in one >>> place? I'm not sure how the latter technique would work, so if that seems >>> better please advise on how to go about that. >> >> A 'git config' variable is fine. The bash.showDirtyState, >> bash.showUntrackedFiles, and bash.showUpstream config variables seem >> like good examples to follow. > > I think this is what you meant. I changed the name of the envvar. Now the > variables are GIT_PS1_HIDE_ON_IGNORED_PWD and bash.hideOnIgnoredPwd. I > admit these are still kind of unwieldy, but maybe now they're more > descriptive? I do prefer the new names. They are long, but how often will someone have to type it? In this case it's better to be descriptive than to be short. (I wonder if adding two letters would improve readability further: GIT_PS1_HIDE_WHEN_PWD_IGNORED and bash.hideWhenPwdIgnored.) To avoid scaring people who might not want this feature enabled, I recommend changing the subject line to something like this: git-prompt.sh: Option to hide prompt for ignored pwd > > Please advise! > > cheers, > Jess > > contrib/completion/git-prompt.sh | 12 > t/t9903-bash-prompt.sh | 42 > > 2 files changed, 54 insertions(+) > > diff --git a/contrib/completion/git-prompt.sh > b/contrib/completion/git-prompt.sh > index c5473dc..d7559ff 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -84,6 +84,11 @@ > # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on > # the colored output of "git status -sb" and are available only when > # using __git_ps1 for PROMPT_COMMAND or precmd. > +# > +# If you would like __git_ps1 to do nothing in the case when the current > +# directory is set up to be ignored by git, then set > +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set > +# bash.hideOnIgnoredPwd to true in the repository configuration. As mentioned in my previous email, I would prefer the code to follow the behavior of the other config variables (the environment variable has to be set *and* the config variable has to be non-false). > > # check whether printf supports -v > __git_printf_supports_v= > @@ -501,6 +506,13 @@ __git_ps1 () > local f="$w$i$s$u" > local gitstring="$c$b${f:+$z$f}$r$p" > > + if [ -n "$(git check-ignore .)" ] && Rather than: [ -n "$(git check-ignore .)" ] I would prefer: git check-ignore -q . For example: if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] && [ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] && git check-ignore -q . then ... -Richard > +( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] || > + [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] ) > + then > + printf_format="" > + fi > + > if [ $pcmode = yes ]; then > if [ "${__git_printf_supports_v-}" != yes ]; then > gitstring=$(printf -- "$printf_format" "$gitstring") > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh > index 9150984..a8ef8a3 100755 > --- a/t/t9903-bash-prompt.sh > +++ b/t/t9903-bash-prompt.sh > @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' ' > git commit -m "another b2" file && > echo 000 >file && > git commit -m "yet another b2" file && > + mkdir ignored_dir && > + echo "ignored_dir/" >> .gitignore && > git checkout master > ' > > @@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' ' > test_cmp expected "$actual" > ' > > +test_expect_success 'prompt - hide on ignored pwd - shell variable unset > with config disabled' ' > + printf " (master)" >expected && > + ( > + cd ignored_dir && > + __git_ps1 >"$actual" > + ) && > + test_cmp expected "$actual" > +' > + > +test_expect_success 'prompt - hide on ignored pwd - shell variable unset > with config enabled' ' > + printf "" >expected && > + test_config bash.hideOnIgnoredPwd true && > + ( > + cd ignored_dir && > +
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
Am 14.10.2014 um 20:34 schrieb Max Kirillov: On Tue, Oct 14, 2014 at 10:26:42AM -0700, Junio C Hamano wrote: And multiple-worktree _is_ about keeping the same repository and history data (i.e. object database, refs, rerere database, reflogs for refs/*) only once, while allowing multiple working trees attached to that single copy. So it appears to me that to create a checkout-to copy of a superproject with a submodule, a checkout-to copy of the superproject would have a submodule, which is a checkout-to copy of the submodule in the superproject. That's right, this linking should be more implicit. Yep. And for the submodule of a submodule too ... ;-) But here are a lot of nuances. For example, it makes sense to have a superproject checkout without submodules being initialized (so that they don't waste space and machine time for working tree, which often is more than repository data). Hmm, I'm not sure if this is a problem. If the GIT_COMMON_DIR does have the submodule repo but it isn't initialized locally, we shouldn't have a problem (except for wasting some disk space if not a single checkout-to superproject initializes this submodule). And if GIT_COMMON_DIR does not have the submodule repo yet, wouldn't it be cloned the moment we init the submodule in the checkout-to? Or would that need extra functionality? > And it may happen so that this checkout is the master repository for superproject checkouts. But this should not prevent users from using initialized submodules in other checkouts. I could live with the restriction that submodule's GIT_COMMON_DIRs always live in their checkout-to superproject's GIT_COMMON_DIR. This would still be an improvement for CI servers that have multiple clones of a super- project, as they would all share their submodule common dirs at least per superproject. Then, a checkout copy of a submodule can be standalone (for example, git and git-html-docs are submodules of msysgit). Or, it can even belong to some other superproject. And in that cases they still should be able to be linked. Maybe such configurations would have to be handled manually to achieve maximum savings. At least I could live with that. Considering all above, and also the thing that I am quite new to submodules (but have to use them currently), I did not intend to create any new UI, only to make backend handle the already existing linked checkouts, which can be made manually. Maybe the way to go is to restrict GIT_COMMON_DIR to superprojects and have a distinct GIT_COMMON_MODULES_DIR? We could even set that to the same location for all submodules of different superprojects to achieve minimum disk footprint (assuming they all have different names across all superprojects and recursion levels). Hmm, so I tend towards adding GIT_COMMON_DIR to local_repo_env until we figured out how to handle this. Without that I fear bad things will happen, at least for a superproject with multiple checkout-to work trees where the same submodule is initialized more than once ... -- To unsubscribe from this list: send the line "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] clone: --dissociate option to mark that reference is only temporary
While use of the --reference option to borrow objects from an existing local repository of the same project is an effective way to reduce traffic when cloning a project over the network, it makes the resulting "borrowing" repository dependent on the "borrowed" repository. After running git clone --reference=P $URL Q the resulting repository Q will be broken if the borrowed repository P disappears. The way to allow the borrowed repository to be removed is to repack the borrowing repository (i.e. run "git repack -a -d" in Q); while power users may know it very well, it is not easily discoverable. Teach a new "--dissociate" option to "git clone" to run this repacking for the user. Signed-off-by: Junio C Hamano --- * This comes from http://thread.gmane.org/gmane.comp.version-control.git/243918/focus=245397 which is one of the low-hanging entries in the leftover-bits list http://git-blame.blogspot.com/p/leftover-bits.html Yes, I must have been really bored to do this ;-) Documentation/git-clone.txt | 11 +-- builtin/clone.c | 25 + t/t5700-clone-reference.sh | 17 + 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 0363d00..f1f2a3f 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -12,7 +12,7 @@ SYNOPSIS 'git clone' [--template=] [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror] [-o ] [-b ] [-u ] [--reference ] - [--separate-git-dir ] + [--dissociate] [--separate-git-dir ] [--depth ] [--[no-]single-branch] [--recursive | --recurse-submodules] [--] [] @@ -98,7 +98,14 @@ objects from the source repository into a pack in the cloned repository. require fewer objects to be copied from the repository being cloned, reducing network and local storage costs. + -*NOTE*: see the NOTE for the `--shared` option. +*NOTE*: see the NOTE for the `--shared` option, and also the +`--dissociate` option. + +--dissociate:: + Borrow the objects from reference repositories specified + with the `--reference` options only to reduce network + transfer and stop borrowing from them after a clone is made + by making necessary local copies of borrowed objects. --quiet:: -q:: diff --git a/builtin/clone.c b/builtin/clone.c index bbd169c..780fbd5 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -48,6 +48,7 @@ static int option_verbosity; static int option_progress = -1; static struct string_list option_config; static struct string_list option_reference; +static int option_dissociate; static int opt_parse_reference(const struct option *opt, const char *arg, int unset) { @@ -93,6 +94,8 @@ static struct option builtin_clone_options[] = { N_("create a shallow clone of that depth")), OPT_BOOL(0, "single-branch", &option_single_branch, N_("clone only one branch, HEAD or --branch")), + OPT_BOOL(0, "dissociate", &option_dissociate, +N_("use --reference only while cloning")), OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"), N_("separate git dir from working tree")), OPT_STRING_LIST('c', "config", &option_config, N_("key=value"), @@ -736,6 +739,21 @@ static void write_refspec_config(const char* src_ref_prefix, strbuf_release(&value); } +static void dissociate_from_references(void) +{ + struct child_process cmd; + + memset(&cmd, 0, sizeof(cmd)); + argv_array_pushl(&cmd.args, "repack", "-a", "-d", NULL); + cmd.git_cmd = 1; + cmd.out = -1; + cmd.no_stdin = 1; + if (run_command(&cmd)) + die(_("cannot repack to clean up")); + if (unlink(git_path("objects/info/alternates")) && errno != ENOENT) + die_errno(_("cannot unlink temporary alternates file")); +} + int cmd_clone(int argc, const char **argv, const char *prefix) { int is_bundle = 0, is_local; @@ -883,6 +901,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_reference.nr) setup_reference(); + else if (option_dissociate) { + warning(_("--dissociate given, but there is no --reference")); + option_dissociate = 0; + } fetch_pattern = value.buf; refspec = parse_fetch_refspec(1, &fetch_pattern); @@ -996,6 +1018,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) transport_unlock_pack(transport); transport_disconnect(transport); + if (option_dissociate) + dissociate_from_references(); + junk_mode = JUNK_LEAVE_REPO; err = checkout(); diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh index 6537911..3e783fc 100755 --- a/t/t5700-clone-reference.sh +++ b/t/t5700-clone-referen
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
On Tue, Oct 14, 2014 at 07:09:45PM +0200, Jens Lehmann wrote: > Until that problem is solved it looks wrong to pass > GIT_COMMON_DIR into submodule recursion, I believe > GIT_COMMON_DIR should be added to the local_repo_env array > (and even if it is passed on later, we might have to > append "/modules/" to make it point to the > correct location). Actually, why there should be an _environment_ variable GIT_COMMON_DIR at all? I mean, gitdir is resolved to some directory (through link or environment), and it contains the shared data directly or referes to it with the commondir link. In which case anyone would want to override that location? I searched though tests but they don't cover this. -- To unsubscribe from this list: send the line "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] [kernel] completion: silence "fatal: Not a git repository" error
John Szakmeister writes: > On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano wrote: > ... >> Hmph, do you mean this one? >> >> $ cd /var/tmp ;# not a git repository >> $ git checkout >> >> -> >> >> $ git checkout fatal: Not a git repository (or any of the parent >> directories): .git >> HEAD >> >> I agree it is ugly, but would it be an improvement for the end user, >> who did not realize that she was not in a directory where "git checkout" >> makes sense, not to tell her that she is not in a git repository in >> some way? > > I had thought about that too, but I think--for me--it comes down to two > things: > > 1) We're not intentionally trying to inform the user anywhere else > that they are not in a git repo. We simply fail to complete anything, > which I think is an established behavior. > 2) It mingles with the stuff already on the command line, making it > confusing to know what you typed. Then you end up ctrl-c'ing your way > out of it and starting over--which is the frustrating part. It is not that I am unsympathetic. It's just it looks to me that the patch is potentially adding one more failed step by hiding the error message to further frustrate the user. $ git checkout ... completes nothing; puzzled but decides not to be worried for now $ git checkout master fatal: not a git repository As you noticed, however, we do not show the ugly error message by design. It is not done consistently, either (happens only when we try to complete refnames). I was just hoping that somebody (not necessarily you) could suggest a way to do better than hide the error message only because it looks ugly (iow, perhaps show it not in the middle of the command line, and do so more consistently). Yes I would imagine it would be a lot harder, but the end user experience _might_ become so much better to make it worthwhile. I dunno. I am not strongly opposed to queuing the patch. -- To unsubscribe from this list: send the line "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] checkout: report upstream correctly even with loosely defined branch.*.merge
When checking out a branch that is set to build on top of another branch (often, a remote-tracking branch), "git checkout" reports how your work relates to the other branch, e.g. Your branch is behind 'origin/master', and can be fast-forwarded. Back when this feature was introduced, this was only done for branches that build on remote-tracking branches, but 5e6e2b48 (Make local branches behave like remote branches when --tracked, 2009-04-01) added support to give the same report for branches that build on other local branches (i.e. branches whose branch.*.remote variables are set to '.'). Unlike the support for the branches building on remote-tracking branches, however, this did not take into account the fact that branch.*.merge configuration is allowed to record a shortened branch name. When branch.*.merge is set to 'master' (not 'refs/heads/master'), i.e. "my branch builds on the local 'master' branch", this caused "git checkout" to report: Your branch is based on 'master', but the upstream is gone. The upstream is our repository and is definitely not gone, so this output is nonsense. The fix is fairly obvious; just like the branch name is DWIMed when "git pull" merges from the 'master' branch without complaint on such a branch, the name of the branch the current branch builds upon needs to be DWIMed the same way. Signed-off-by: Junio C Hamano --- remote.c | 34 +++--- t/t2024-checkout-dwim.sh | 18 ++ 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/remote.c b/remote.c index 0e9459c..ecbe363 100644 --- a/remote.c +++ b/remote.c @@ -1611,6 +1611,27 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, } } +static void set_merge(struct branch *ret) +{ + char *ref; + unsigned char sha1[20]; + int i; + + ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge)); + for (i = 0; i < ret->merge_nr; i++) { + ret->merge[i] = xcalloc(1, sizeof(**ret->merge)); + ret->merge[i]->src = xstrdup(ret->merge_name[i]); + if (!remote_find_tracking(ret->remote, ret->merge[i]) || + strcmp(ret->remote_name, ".")) + continue; + if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]), +sha1, &ref) == 1) + ret->merge[i]->dst = ref; + else + ret->merge[i]->dst = xstrdup(ret->merge_name[i]); + } +} + struct branch *branch_get(const char *name) { struct branch *ret; @@ -1622,17 +1643,8 @@ struct branch *branch_get(const char *name) ret = make_branch(name, 0); if (ret && ret->remote_name) { ret->remote = remote_get(ret->remote_name); - if (ret->merge_nr) { - int i; - ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge)); - for (i = 0; i < ret->merge_nr; i++) { - ret->merge[i] = xcalloc(1, sizeof(**ret->merge)); - ret->merge[i]->src = xstrdup(ret->merge_name[i]); - if (remote_find_tracking(ret->remote, ret->merge[i]) - && !strcmp(ret->remote_name, ".")) - ret->merge[i]->dst = xstrdup(ret->merge_name[i]); - } - } + if (ret->merge_nr) + set_merge(ret); } return ret; } diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index 6ecb559..468a000 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -185,4 +185,22 @@ test_expect_success 'checkout -- succeeds, even if a file with the same test_branch_upstream spam repo_c spam ' +test_expect_success 'loosely defined local base branch is reported correctly' ' + + git checkout master && + git branch strict && + git branch loose && + git commit --allow-empty -m "a bit more" && + + test_config branch.strict.remote . && + test_config branch.loose.remote . && + test_config branch.strict.merge refs/heads/master && + test_config branch.loose.merge master && + + git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect && + git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual && + + test_cmp expect actual +' + test_done -- 2.1.2-492-gf8d07d7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes
On Tue, Oct 14, 2014 at 09:51:22PM +0200, Jens Lehmann wrote: > Am 14.10.2014 um 20:34 schrieb Max Kirillov: >> But here are a lot of nuances. For example, it makes >> sense to have a superproject checkout without submodules >> being initialized (so that they don't waste space and >> machine time for working tree, which often is more than >> repository data). > > Hmm, I'm not sure if this is a problem. If the > GIT_COMMON_DIR does have the submodule repo but it isn't > initialized locally, we shouldn't have a problem (except > for wasting some disk space if not a single checkout-to > superproject initializes this submodule). If initially a repository is clone without submodules, it will not have anything in the GIT_COMMON_DIR. > And if GIT_COMMON_DIR does not have the submodule repo > yet, wouldn't it be cloned the moment we init the > submodule in the checkout-to? Or would that need extra > functionality? I cannot say I like this. Network operations should be caused only by clone and submodules. I think the logic can be simple: it a submodule is not checked-out in the repository "checkout --to" is called from, then it is not checked-out to the new one also. If it is, then checkout calls itself recursively in the submodule and works like being run in standalone repository. >> Then, a checkout copy of a submodule can be standalone >> (for example, git and git-html-docs are submodules of >> msysgit). Or, it can even belong to some other >> superproject. And in that cases they still should be able >> to be linked. > > Maybe such configurations would have to be handled > manually to achieve maximum savings. At least I could live > with that. To make manual handling of the cases, and to skip checking-out a module. I would think about the following interface: $ git checkout --to ... - does not checkout submodules, creates empty directory. $ git checkout --recursive --to ... - if a submodule is checked-out in source repository, recursed there and run "checkout --recursive" again. If a submodule is not checked-out, does not checkout it, creates an empty directory. By the way, I have found your branch recursive_submodule_checkout. Would you like to revive it? Then it can be done with the same option. > Hmm, so I tend towards adding GIT_COMMON_DIR to > local_repo_env until we figured out how to handle this. > Without that I fear bad things will happen, at least for a > superproject with multiple checkout-to work trees where > the same submodule is initialized more than once ... I learned about local_repo_env and agree it should include GIT_COMMON_DIR. Unless it is removed at all... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Oct 2014, #03; Tue, 14)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * bc/asciidoc-pretty-formats-fix (2014-10-08) 1 commit (merged to 'next' on 2014-10-13 at 8208335) + Documentation: fix misrender of pretty-formats in Asciidoctor * da/completion-show-signature (2014-10-07) 1 commit (merged to 'next' on 2014-10-07 at 2467c19) + completion: add --show-signature for log and show * da/include-compat-util-first-in-c (2014-09-15) 1 commit (merged to 'next' on 2014-10-07 at ea5bcb4) + cleanups: ensure that git-compat-util.h is included first Code clean-up. * dt/cache-tree-repair (2014-09-30) 1 commit (merged to 'next' on 2014-10-07 at 923bd93) + t0090: avoid passing empty string to printf %d (this branch is used by jk/prune-mtime.) This fixes a topic that has graduated to 'master'. * mh/lockfile (2014-10-01) 38 commits (merged to 'next' on 2014-10-08 at 39cb6da) + lockfile.h: extract new header file for the functions in lockfile.c + hold_locked_index(): move from lockfile.c to read-cache.c + hold_lock_file_for_append(): restore errno before returning + get_locked_file_path(): new function + lockfile.c: rename static functions + lockfile: rename LOCK_NODEREF to LOCK_NO_DEREF + commit_lock_file_to(): refactor a helper out of commit_lock_file() + trim_last_path_component(): replace last_path_elm() + resolve_symlink(): take a strbuf parameter + resolve_symlink(): use a strbuf for internal scratch space + lockfile: change lock_file::filename into a strbuf + commit_lock_file(): use a strbuf to manage temporary space + try_merge_strategy(): use a statically-allocated lock_file object + try_merge_strategy(): remove redundant lock_file allocation + struct lock_file: declare some fields volatile + lockfile: avoid transitory invalid states + git_config_set_multivar_in_file(): avoid call to rollback_lock_file() + dump_marks(): remove a redundant call to rollback_lock_file() + api-lockfile: document edge cases + commit_lock_file(): rollback lock file on failure to rename + close_lock_file(): if close fails, roll back + commit_lock_file(): die() if called for unlocked lockfile object + commit_lock_file(): inline temporary variable + remove_lock_file(): call rollback_lock_file() + lock_file(): exit early if lockfile cannot be opened + prepare_index(): declare return value to be (const char *) + delete_ref_loose(): don't muck around in the lock_file's filename + cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN + lockfile.c: document the various states of lock_file objects + lock_file(): always initialize and register lock_file object + hold_lock_file_for_append(): release lock on errors + lockfile: unlock file if lockfile permissions cannot be adjusted + rollback_lock_file(): set fd to -1 + rollback_lock_file(): exit early if lock is not active + rollback_lock_file(): do not clear filename redundantly + close_lock_file(): exit (successfully) if file is already closed + api-lockfile: revise and expand the documentation + unable_to_lock_die(): rename function from unable_to_lock_index_die() (this branch is used by mh/lockfile-stdio.) The lockfile API and its users have been cleaned up. * mh/lockfile-stdio (2014-10-01) 3 commits (merged to 'next' on 2014-10-08 at e56cebc) + commit_packed_refs(): reimplement using fdopen_lock_file() + dump_marks(): reimplement using fdopen_lock_file() + fdopen_lock_file(): access a lockfile using stdio (this branch uses mh/lockfile.) * rs/daemon-fixes (2014-10-01) 3 commits (merged to 'next' on 2014-10-07 at 4171e10) + daemon: remove write-only variable maxfd + daemon: fix error message after bind() + daemon: handle gethostbyname() error "git daemon" (with NO_IPV6 build configuration) used to incorrectly use the hostname even when gethostbyname() reported that the given hostname is not found. * rs/mailsplit (2014-10-07) 1 commit (merged to 'next' on 2014-10-08 at 58b053e) + mailsplit: remove unnecessary unlink(2) call * rs/more-uses-of-skip-prefix (2014-10-07) 1 commit (merged to 'next' on 2014-10-08 at cd153c0) + use skip_prefix() to avoid more magic numbers * rs/plug-leak-in-bundle (2014-10-07) 1 commit (merged to 'next' on 2014-10-08 at 5539cd7) + bundle: plug minor memory leak in is_tag_in_date_range() * rs/sha1-array-test (2014-10-01) 2 commits (merged to 'next' on 2014-10-08 at 5960711) + sha1-lookup: handle duplicates in sha1_pos() + sha1-array: add test-sha1-array and basic tests * sk/tag-contains-wo-recursion (2014-09-23) 1 commit (merged to 'next' on 2014-10-08 at e425f54) + t7004: give the test a bit more stack space * so/rebase-doc-fork-point
color box, display box, corrugated box, color card, blister card, color sleeve, hang tag, label
Hi, this is David Wu from Shanghai, China. We are a printing company, we can print color box, corrugated box, label, hang tag etc. Please let me know if you need these. I will send you the website then. Best regards, David Wu -- To unsubscribe from this list: send the line "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 v23 0/25] rs/ref-transaction ("Use ref transactions", part 3)
This series by Ronnie was last seen on-list at [1]. It includes some improvements to the ref-transaction API, improves handling of poorly named refs (which should make it easier to tighten the refname format checks in the future), and is a stepping stone toward later series that use the ref-transaction API more and make it support alternate backends (yay!). The changes since (a merge of 'master' and) v22 are very minor and can be seen below[2]. The more important change is that it's rebased against current 'master'. Review comments from Michael and Junio were very helpful in getting this in shape. Thanks much to both. The series can also be found at git://repo.or.cz/git/jrn.git tags/rs/ref-transaction It is based against current 'master' (670a3c1d, 2014-10-14) and intended for 'next'. Thoughts welcome, as always. Improvements preferred in the form of patches on top of the series. Jonathan Nieder (6): mv test: recreate mod/ directory instead of relying on stale copy branch -d: avoid repeated symref resolution packed-ref cache: forbid dot-components in refnames refs.c: do not permit err == NULL lockfile: remove unable_to_lock_error ref_transaction_commit: bail out on failure to remove a ref Junio C Hamano (1): reflog test: test interaction with detached HEAD Ronnie Sahlberg (18): wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success refs.c: lock_ref_sha1_basic is used for all refs wrapper.c: add a new function unlink_or_msg refs.c: add an err argument to delete_ref_loose refs.c: pass the ref log message to _create/delete/update instead of _commit rename_ref: don't ask read_ref_full where the ref came from refs.c: refuse to lock badly named refs in lock_ref_sha1_basic refs.c: call lock_ref_sha1_basic directly from commit refs.c: pass a list of names to skip to is_refname_available refs.c: ref_transaction_commit: distinguish name conflicts from other errors fetch.c: change s_update_ref to use a ref transaction refs.c: make write_ref_sha1 static refs.c: change resolve_ref_unsafe reading argument to be a flags field branch -d: simplify by using RESOLVE_REF_READING test: put tests for handling of bad ref names in one place refs.c: allow listing and deleting badly named refs for-each-ref: skip and warn about broken ref names remote rm/prune: print a message when writing packed-refs fails branch.c | 6 +- builtin/blame.c | 2 +- builtin/branch.c | 22 ++- builtin/checkout.c | 6 +- builtin/clone.c | 2 +- builtin/commit.c | 6 +- builtin/fetch.c | 34 ++-- builtin/fmt-merge-msg.c | 2 +- builtin/for-each-ref.c | 11 +- builtin/fsck.c | 2 +- builtin/log.c| 3 +- builtin/merge.c | 2 +- builtin/notes.c | 2 +- builtin/receive-pack.c | 9 +- builtin/remote.c | 20 ++- builtin/replace.c| 5 +- builtin/show-branch.c| 7 +- builtin/symbolic-ref.c | 2 +- builtin/tag.c| 4 +- builtin/update-ref.c | 13 +- bundle.c | 2 +- cache.h | 44 +++-- fast-import.c| 8 +- git-compat-util.h| 16 +- http-backend.c | 4 +- lockfile.c | 10 -- lockfile.h | 1 - notes-merge.c| 2 +- reflog-walk.c| 5 +- refs.c | 446 --- refs.h | 44 +++-- remote.c | 11 +- sequencer.c | 8 +- t/t1400-update-ref.sh| 62 +++ t/t1413-reflog-detach.sh | 70 t/t1430-bad-ref-name.sh | 207 ++ t/t3200-branch.sh| 9 + t/t7001-mv.sh| 15 +- t/t9300-fast-import.sh | 30 transport-helper.c | 5 +- transport.c | 5 +- upload-pack.c| 2 +- walker.c | 5 +- wrapper.c| 28 ++- wt-status.c | 2 +- 45 files changed, 850 insertions(+), 351 deletions(-) create mode 100755 t/t1413-reflog-detach.sh create mode 100755 t/t1430-bad-ref-name.sh [1] http://thread.gmane.org/gmane.comp.version-control.git/254501/focus=257771 [2] cache.h | 11 --- git-compat-util.h | 4 +-- refs.c| 96 +-- refs.h| 6 +++- 4 files changed, 64 insertions(+), 53 deletions(-) diff --git a/cache.h b/cache.h index 209e8ba..0501f7d 100644 --- a/cache.h +++ b/cache.h @@ -983,7 +983,8 @@ extern int read_ref(const char *refname, unsigned char *sha1); * packed references), REF_ISSYMREF (if the initial reference was a * symbolic reference), REF_BAD_NAME (if the reference name is ill * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN - * (if the ref is malformed). + * (if the ref is malformed or has a bad name). See refs.h for mo
[PATCH 01/25] mv test: recreate mod/ directory instead of relying on stale copy
From: Jonathan Nieder Date: Wed, 10 Sep 2014 14:01:46 -0700 The tests for 'git mv moves a submodule' functionality often run commands like git mv sub mod/sub to move a submodule into a subdirectory. Just like plain /bin/mv, this is supposed to succeed if the mod/ parent directory exists and fail if it doesn't exist. Usually these tests mkdir the parent directory beforehand, but some instead rely on it being left behind by previous tests. More precisely, when 'git reset --hard' tries to move to a state where mod/sub is not present any more, it would perform the following operations: rmdir("mod/sub") rmdir("mod") The first fails with ENOENT because the test script removed mod/sub with "rm -rf" already, so 'reset --hard' doesn't bother to move on to the second, and the mod/ directory is kept around. Better to explicitly remove and re-create the mod/ directory so later tests don't have to depend on the directory left behind by the earlier ones at all (making it easier to rearrange or skip some tests in the file or to tweak 'reset --hard' behavior without breaking unrelated tests). Noticed while testing a patch that fixes the reset --hard behavior described above. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- t/t7001-mv.sh | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 54d7807..69f11bd 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -350,10 +350,11 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu ' test_expect_success 'git mv moves a submodule with gitfile' ' - rm -rf mod/sub && + rm -rf mod && git reset --hard && git submodule update && entry="$(git ls-files --stage sub | cut -f 1)" && + mkdir mod && ( cd mod && git mv ../sub/ . @@ -372,11 +373,12 @@ test_expect_success 'git mv moves a submodule with gitfile' ' ' test_expect_success 'mv does not complain when no .gitmodules file is found' ' - rm -rf mod/sub && + rm -rf mod && git reset --hard && git submodule update && git rm .gitmodules && entry="$(git ls-files --stage sub | cut -f 1)" && + mkdir mod && git mv sub mod/sub 2>actual.err && ! test -s actual.err && ! test -e sub && @@ -390,11 +392,12 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' ' ' test_expect_success 'mv will error out on a modified .gitmodules file unless staged' ' - rm -rf mod/sub && + rm -rf mod && git reset --hard && git submodule update && git config -f .gitmodules foo.bar true && entry="$(git ls-files --stage sub | cut -f 1)" && + mkdir mod && test_must_fail git mv sub mod/sub 2>actual.err && test -s actual.err && test -e sub && @@ -413,13 +416,14 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta ' test_expect_success 'mv issues a warning when section is not found in .gitmodules' ' - rm -rf mod/sub && + rm -rf mod && git reset --hard && git submodule update && git config -f .gitmodules --remove-section submodule.sub && git add .gitmodules && entry="$(git ls-files --stage sub | cut -f 1)" && echo "warning: Could not find section in .gitmodules where path=sub" >expect.err && + mkdir mod && git mv sub mod/sub 2>actual.err && test_i18ncmp expect.err actual.err && ! test -e sub && @@ -433,9 +437,10 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule ' test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' ' - rm -rf mod/sub && + rm -rf mod && git reset --hard && git submodule update && + mkdir mod && git mv -n sub mod/sub 2>actual.err && test -f sub/.git && git diff-index --exit-code HEAD && -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 02/25] wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success
From: Ronnie Sahlberg Date: Wed, 16 Jul 2014 11:01:18 -0700 Simplify the function warn_if_unremovable slightly. Additionally, change behaviour slightly. If we failed to remove the object because the object does not exist, we can still return success back to the caller since none of the callers depend on "fail if the file did not exist". Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- git-compat-util.h | 7 +-- refs.c| 2 +- wrapper.c | 14 ++ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index fb41118..d67592f 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -777,11 +777,14 @@ void git_qsort(void *base, size_t nmemb, size_t size, /* * Preserves errno, prints a message, but gives no warning for ENOENT. - * Always returns the return value of unlink(2). + * Returns 0 on success, which includes trying to unlink an object that does + * not exist. */ int unlink_or_warn(const char *path); /* - * Likewise for rmdir(2). + * Preserves errno, prints a message, but gives no warning for ENOENT. + * Returns 0 on success, which includes trying to remove a directory that does + * not exist. */ int rmdir_or_warn(const char *path); /* diff --git a/refs.c b/refs.c index a77458f..2dcf6c6 100644 --- a/refs.c +++ b/refs.c @@ -2607,7 +2607,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag) char *loose_filename = get_locked_file_path(lock->lk); int err = unlink_or_warn(loose_filename); free(loose_filename); - if (err && errno != ENOENT) + if (err) return 1; } return 0; diff --git a/wrapper.c b/wrapper.c index 5b77d2a..8d4be66 100644 --- a/wrapper.c +++ b/wrapper.c @@ -466,14 +466,12 @@ int xmkstemp_mode(char *template, int mode) static int warn_if_unremovable(const char *op, const char *file, int rc) { - if (rc < 0) { - int err = errno; - if (ENOENT != err) { - warning("unable to %s %s: %s", - op, file, strerror(errno)); - errno = err; - } - } + int err; + if (!rc || errno == ENOENT) + return 0; + err = errno; + warning("unable to %s %s: %s", op, file, strerror(errno)); + errno = err; return rc; } -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 03/25] refs.c: lock_ref_sha1_basic is used for all refs
From: Ronnie Sahlberg Date: Thu, 2 Oct 2014 07:59:02 -0700 lock_ref_sha1_basic is used to lock refs that sit directly in the .git dir such as HEAD and MERGE_HEAD in addition to the more ordinary refs under "refs/". Remove the note claiming otherwise. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 2dcf6c6..4f2564d 100644 --- a/refs.c +++ b/refs.c @@ -2134,7 +2134,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log) } /* - * Locks a "refs/" ref returning the lock on success and NULL on failure. + * Locks a ref returning the lock on success and NULL on failure. * On failure errno is set to something meaningful. */ static struct ref_lock *lock_ref_sha1_basic(const char *refname, -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 04/25] wrapper.c: add a new function unlink_or_msg
From: Ronnie Sahlberg Date: Wed, 16 Jul 2014 11:20:36 -0700 This behaves like unlink_or_warn except that on failure it writes the message to its 'err' argument, which the caller can display in an appropriate way or ignore. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- git-compat-util.h | 9 + wrapper.c | 14 ++ 2 files changed, 23 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index d67592f..59ecf21 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -326,6 +326,8 @@ static inline char *git_find_last_dir_sep(const char *path) #include "wildmatch.h" +struct strbuf; + /* General helper functions */ extern void vreportf(const char *prefix, const char *err, va_list params); extern void vwritef(int fd, const char *prefix, const char *err, va_list params); @@ -781,6 +783,13 @@ void git_qsort(void *base, size_t nmemb, size_t size, * not exist. */ int unlink_or_warn(const char *path); + /* + * Tries to unlink file. Returns 0 if unlink succeeded + * or the file already didn't exist. Returns -1 and + * appends a message to err suitable for + * 'error("%s", err->buf)' on error. + */ +int unlink_or_msg(const char *file, struct strbuf *err); /* * Preserves errno, prints a message, but gives no warning for ENOENT. * Returns 0 on success, which includes trying to remove a directory that does diff --git a/wrapper.c b/wrapper.c index 8d4be66..007ec0d 100644 --- a/wrapper.c +++ b/wrapper.c @@ -475,6 +475,20 @@ static int warn_if_unremovable(const char *op, const char *file, int rc) return rc; } +int unlink_or_msg(const char *file, struct strbuf *err) +{ + int rc = unlink(file); + + assert(err); + + if (!rc || errno == ENOENT) + return 0; + + strbuf_addf(err, "unable to unlink %s: %s", + file, strerror(errno)); + return -1; +} + int unlink_or_warn(const char *file) { return warn_if_unremovable("unlink", file, unlink(file)); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 05/25] refs.c: add an err argument to delete_ref_loose
From: Ronnie Sahlberg Date: Thu, 15 May 2014 08:25:23 -0700 Add an err argument to delete_ref_loose so that we can pass a descriptive error string back to the caller. Pass the err argument from transaction commit to this function so that transaction users will have a nice error string if the transaction failed due to delete_ref_loose. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 4f2564d..430857b 100644 --- a/refs.c +++ b/refs.c @@ -2597,7 +2597,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) return ret; } -static int delete_ref_loose(struct ref_lock *lock, int flag) +static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) { if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) { /* @@ -2605,9 +2605,9 @@ static int delete_ref_loose(struct ref_lock *lock, int flag) * lockfile name, minus ".lock": */ char *loose_filename = get_locked_file_path(lock->lk); - int err = unlink_or_warn(loose_filename); + int res = unlink_or_msg(loose_filename, err); free(loose_filename); - if (err) + if (res) return 1; } return 0; @@ -3658,7 +3658,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if (update->lock) { - ret |= delete_ref_loose(update->lock, update->type); + ret |= delete_ref_loose(update->lock, update->type, + err); if (!(update->flags & REF_ISPRUNING)) delnames[delnum++] = update->lock->ref_name; } -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 06/25] refs.c: pass the ref log message to _create/delete/update instead of _commit
From: Ronnie Sahlberg Date: Wed, 30 Apr 2014 12:22:42 -0700 Change the ref transaction API so that we pass the reflog message to the create/delete/update functions instead of to ref_transaction_commit. This allows different reflog messages for each ref update in a multi-ref transaction. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- branch.c | 4 ++-- builtin/commit.c | 4 ++-- builtin/receive-pack.c | 5 +++-- builtin/replace.c | 5 +++-- builtin/tag.c | 4 ++-- builtin/update-ref.c | 13 +++-- fast-import.c | 8 refs.c | 34 +- refs.h | 12 ++-- sequencer.c| 4 ++-- walker.c | 5 ++--- 11 files changed, 54 insertions(+), 44 deletions(-) diff --git a/branch.c b/branch.c index 9a2228e..884c62c 100644 --- a/branch.c +++ b/branch.c @@ -285,8 +285,8 @@ void create_branch(const char *head, transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_update(transaction, ref.buf, sha1, - null_sha1, 0, !forcing, &err) || - ref_transaction_commit(transaction, msg, &err)) + null_sha1, 0, !forcing, msg, &err) || + ref_transaction_commit(transaction, &err)) die("%s", err.buf); ref_transaction_free(transaction); strbuf_release(&err); diff --git a/builtin/commit.c b/builtin/commit.c index 81dc622..a7857ab 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1811,8 +1811,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) ref_transaction_update(transaction, "HEAD", sha1, current_head ? current_head->object.sha1 : NULL, - 0, !!current_head, &err) || - ref_transaction_commit(transaction, sb.buf, &err)) { + 0, !!current_head, sb.buf, &err) || + ref_transaction_commit(transaction, &err)) { rollback_index_files(); die("%s", err.buf); } diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f2f6c67..df6c337 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -842,8 +842,9 @@ static const char *update(struct command *cmd, struct shallow_info *si) transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_update(transaction, namespaced_name, - new_sha1, old_sha1, 0, 1, &err) || - ref_transaction_commit(transaction, "push", &err)) { + new_sha1, old_sha1, 0, 1, "push", + &err) || + ref_transaction_commit(transaction, &err)) { ref_transaction_free(transaction); rp_error("%s", err.buf); diff --git a/builtin/replace.c b/builtin/replace.c index 8020db8..85d39b5 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -171,8 +171,9 @@ static int replace_object_sha1(const char *object_ref, transaction = ref_transaction_begin(&err); if (!transaction || - ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) || - ref_transaction_commit(transaction, NULL, &err)) + ref_transaction_update(transaction, ref, repl, prev, + 0, 1, NULL, &err) || + ref_transaction_commit(transaction, &err)) die("%s", err.buf); ref_transaction_free(transaction); diff --git a/builtin/tag.c b/builtin/tag.c index a81b9e4..e633f4e 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -733,8 +733,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix) transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_update(transaction, ref.buf, object, prev, - 0, 1, &err) || - ref_transaction_commit(transaction, NULL, &err)) + 0, 1, NULL, &err) || + ref_transaction_commit(transaction, &err)) die("%s", err.buf); ref_transaction_free(transaction); if (force && !is_null_sha1(prev) && hashcmp(prev, object)) diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 54a48c0..6c9be05 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = { static char line_termination = '\n'; static int update_flags; +static const char *msg; /* * Parse one whitespace- or NUL-terminated, possibly C-quoted argument @@ -198,7 +199,7 @@ static const char *parse_
[PATCH 07/25] rename_ref: don't ask read_ref_full where the ref came from
From: Ronnie Sahlberg Date: Wed, 30 Apr 2014 12:41:04 -0700 We call read_ref_full with a pointer to flags from rename_ref but since we never actually use the returned flags we can just pass NULL here instead. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index f5d7bd7..3c45615 100644 --- a/refs.c +++ b/refs.c @@ -2721,7 +2721,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms goto rollback; } - if (!read_ref_full(newrefname, sha1, 1, &flag) && + if (!read_ref_full(newrefname, sha1, 1, NULL) && delete_ref(newrefname, sha1, REF_NODEREF)) { if (errno==EISDIR) { if (remove_empty_directories(git_path("%s", newrefname))) { -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 09/25] refs.c: call lock_ref_sha1_basic directly from commit
From: Ronnie Sahlberg Date: Thu, 1 May 2014 10:43:39 -0700 Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic directly from the commit function. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c index 9c01623..b591b9c 100644 --- a/refs.c +++ b/refs.c @@ -3632,12 +3632,12 @@ int ref_transaction_commit(struct ref_transaction *transaction, for (i = 0; i < n; i++) { struct ref_update *update = updates[i]; - update->lock = lock_any_ref_for_update(update->refname, - (update->have_old ? - update->old_sha1 : - NULL), - update->flags, - &update->type); + update->lock = lock_ref_sha1_basic(update->refname, + (update->have_old ? + update->old_sha1 : + NULL), + update->flags, + &update->type); if (!update->lock) { if (err) strbuf_addf(err, "Cannot lock the ref '%s'.", -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 08/25] refs.c: refuse to lock badly named refs in lock_ref_sha1_basic
From: Ronnie Sahlberg Date: Thu, 1 May 2014 10:40:10 -0700 Move the check for check_refname_format from lock_any_ref_for_update to lock_ref_sha1_basic. At some later stage we will get rid of lock_any_ref_for_update completely. This has no visible impact to callers except for the inability to lock badly named refs, which is not possible today already for other reasons.(*) Keep lock_any_ref_for_update as a no-op wrapper. It is the public facing version of this interface and keeping it as a separate function will make it easier to experiment with the internal lock_ref_sha1_basic signature. (*) For example, if lock_ref_sha1_basic checks the refname format and refuses to lock badly named refs, it will not be possible to delete such refs because the first step of deletion is to lock the ref. We currently already fail in that case because these refs are not recognized to exist: $ cp .git/refs/heads/master .git/refs/heads/echo...\*\* $ git branch -D .git/refs/heads/echo...\*\* error: branch '.git/refs/heads/echo...**' not found. This has been broken for a while. Later patches in the series will start repairing the handling of badly named refs. Signed-off-by: Ronnie Sahlberg Reviewed-by: Michael Haggerty Signed-off-by: Jonathan Nieder --- refs.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 3c45615..9c01623 100644 --- a/refs.c +++ b/refs.c @@ -2150,6 +2150,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, int missing = 0; int attempts_remaining = 3; + if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { + errno = EINVAL; + return NULL; + } + lock = xcalloc(1, sizeof(struct ref_lock)); lock->lock_fd = -1; @@ -2241,8 +2246,6 @@ struct ref_lock *lock_any_ref_for_update(const char *refname, const unsigned char *old_sha1, int flags, int *type_p) { - if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) - return NULL; return lock_ref_sha1_basic(refname, old_sha1, flags, type_p); } -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 10/25] refs.c: pass a list of names to skip to is_refname_available
From: Ronnie Sahlberg Date: Thu, 1 May 2014 11:16:07 -0700 Change is_refname_available to take a list of strings to exclude when checking for conflicts instead of just one single name. We can already exclude a single name for the sake of renames. This generalizes that support. ref_transaction_commit already tracks a set of refs that are being deleted in an array. This array is then used to exclude refs from being written to the packed-refs file. At some stage we will want to change this array to a struct string_list and then we can pass it to is_refname_available via the call to lock_ref_sha1_basic. That will allow us to perform transactions that perform multiple renames as long as there are no conflicts within the starting or ending state. For example, that would allow a single transaction that contains two renames that are both individually conflicting: m -> n/n n -> m/m No functional change intended yet. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 50 -- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index b591b9c..a007cf3 100644 --- a/refs.c +++ b/refs.c @@ -787,13 +787,13 @@ static void prime_ref_dir(struct ref_dir *dir) } } -static int entry_matches(struct ref_entry *entry, const char *refname) +static int entry_matches(struct ref_entry *entry, const struct string_list *list) { - return refname && !strcmp(entry->name, refname); + return list && string_list_has_string(list, entry->name); } struct nonmatching_ref_data { - const char *skip; + const struct string_list *skip; struct ref_entry *found; }; @@ -817,16 +817,19 @@ static void report_refname_conflict(struct ref_entry *entry, /* * Return true iff a reference named refname could be created without * conflicting with the name of an existing reference in dir. If - * oldrefname is non-NULL, ignore potential conflicts with oldrefname - * (e.g., because oldrefname is scheduled for deletion in the same + * skip is non-NULL, ignore potential conflicts with refs in skip + * (e.g., because they are scheduled for deletion in the same * operation). * * Two reference names conflict if one of them exactly matches the * leading components of the other; e.g., "foo/bar" conflicts with * both "foo" and with "foo/bar/baz" but not with "foo/bar" or * "foo/barbados". + * + * skip must be sorted. */ -static int is_refname_available(const char *refname, const char *oldrefname, +static int is_refname_available(const char *refname, + const struct string_list *skip, struct ref_dir *dir) { const char *slash; @@ -840,12 +843,12 @@ static int is_refname_available(const char *refname, const char *oldrefname, * looking for a conflict with a leaf entry. * * If we find one, we still must make sure it is -* not "oldrefname". +* not in "skip". */ pos = search_ref_dir(dir, refname, slash - refname); if (pos >= 0) { struct ref_entry *entry = dir->entries[pos]; - if (entry_matches(entry, oldrefname)) + if (entry_matches(entry, skip)) return 1; report_refname_conflict(entry, refname); return 0; @@ -878,13 +881,13 @@ static int is_refname_available(const char *refname, const char *oldrefname, /* * We found a directory named "refname". It is a * problem iff it contains any ref that is not -* "oldrefname". +* in "skip". */ struct ref_entry *entry = dir->entries[pos]; struct ref_dir *dir = get_ref_dir(entry); struct nonmatching_ref_data data; - data.skip = oldrefname; + data.skip = skip; sort_ref_dir(dir); if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data)) return 1; @@ -2139,6 +2142,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log) */ static struct ref_lock *lock_ref_sha1_basic(const char *refname, const unsigned char *old_sha1, + const struct string_list *skip, int flags, int *type_p) { char *ref_file; @@ -2188,7 +2192,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, * name is a proper prefix of our refname. */ if (missing && -!is_refname_available(refname, NULL, get_packed_refs(&ref_cache))) { +!is_refname_available(refname, skip, get_packed_refs(&ref_cache)
[PATCH 11/25] refs.c: ref_transaction_commit: distinguish name conflicts from other errors
From: Ronnie Sahlberg Date: Fri, 16 May 2014 14:14:38 -0700 In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either when we lstat the new refname or if the name checking function reports that the same type of conflict happened. In both cases, it means that we can not create the new ref due to a name conflict. Start defining specific return codes for _commit. TRANSACTION_NAME_CONFLICT refers to a failure to create a ref due to a name conflict with another ref. TRANSACTION_GENERIC_ERROR is for all other errors. When "git fetch" is creating refs, name conflicts differ from other errors in that they are likely to be resolved by running "git remote prune ". "git fetch" currently inspects errno to decide whether to give that advice. Once it switches to the transaction API, it can check for TRANSACTION_NAME_CONFLICT instead. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 26 -- refs.h | 9 +++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/refs.c b/refs.c index a007cf3..9d9bbeb 100644 --- a/refs.c +++ b/refs.c @@ -3637,9 +3637,10 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Copy, sort, and reject duplicate refs */ qsort(updates, n, sizeof(*updates), ref_update_compare); - ret = ref_update_reject_duplicates(updates, n, err); - if (ret) + if (ref_update_reject_duplicates(updates, n, err)) { + ret = TRANSACTION_GENERIC_ERROR; goto cleanup; + } /* Acquire all locks while verifying old values */ for (i = 0; i < n; i++) { @@ -3653,10 +3654,12 @@ int ref_transaction_commit(struct ref_transaction *transaction, update->flags, &update->type); if (!update->lock) { + ret = (errno == ENOTDIR) + ? TRANSACTION_NAME_CONFLICT + : TRANSACTION_GENERIC_ERROR; if (err) strbuf_addf(err, "Cannot lock the ref '%s'.", update->refname); - ret = 1; goto cleanup; } } @@ -3666,15 +3669,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if (!is_null_sha1(update->new_sha1)) { - ret = write_ref_sha1(update->lock, update->new_sha1, -update->msg); - update->lock = NULL; /* freed by write_ref_sha1 */ - if (ret) { + if (write_ref_sha1(update->lock, update->new_sha1, + update->msg)) { + update->lock = NULL; /* freed by write_ref_sha1 */ if (err) strbuf_addf(err, "Cannot update the ref '%s'.", update->refname); + ret = TRANSACTION_GENERIC_ERROR; goto cleanup; } + update->lock = NULL; /* freed by write_ref_sha1 */ } } @@ -3683,14 +3687,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if (update->lock) { - ret |= delete_ref_loose(update->lock, update->type, - err); + if (delete_ref_loose(update->lock, update->type, err)) + ret = TRANSACTION_GENERIC_ERROR; + if (!(update->flags & REF_ISPRUNING)) delnames[delnum++] = update->lock->ref_name; } } - ret |= repack_without_refs(delnames, delnum, err); + if (repack_without_refs(delnames, delnum, err)) + ret = TRANSACTION_GENERIC_ERROR; for (i = 0; i < delnum; i++) unlink_or_warn(git_path("logs/%s", delnames[i])); clear_loose_ref_cache(&ref_cache); diff --git a/refs.h b/refs.h index 1a55cab..50b115a 100644 --- a/refs.h +++ b/refs.h @@ -326,9 +326,14 @@ int ref_transaction_delete(struct ref_transaction *transaction, /* * Commit all of the changes that have been queued in transaction, as - * atomically as possible. Return a nonzero value if there is a - * problem. + * atomically as possible. + * + * Returns 0 for success, or one of the below error codes for errors. */ +/* Naming conflict (for example, the ref names A and A/B conflict). */ +#define TRANSACTION_NAME_CONFLICT -1 +/* All other errors. */ +#define TRANSACTION_GENERIC
[PATCH 12/25] fetch.c: change s_update_ref to use a ref transaction
From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 13:49:07 -0700 Change s_update_ref to use a ref transaction for the ref update. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/fetch.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 159fb7e..6ffd023 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -404,23 +404,37 @@ static int s_update_ref(const char *action, { char msg[1024]; char *rla = getenv("GIT_REFLOG_ACTION"); - static struct ref_lock *lock; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; + int ret, df_conflict = 0; if (dry_run) return 0; if (!rla) rla = default_rla.buf; snprintf(msg, sizeof(msg), "%s: %s", rla, action); - lock = lock_any_ref_for_update(ref->name, - check_old ? ref->old_sha1 : NULL, - 0, NULL); - if (!lock) - return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT : - STORE_REF_ERROR_OTHER; - if (write_ref_sha1(lock, ref->new_sha1, msg) < 0) - return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT : - STORE_REF_ERROR_OTHER; + + transaction = ref_transaction_begin(&err); + if (!transaction || + ref_transaction_update(transaction, ref->name, ref->new_sha1, + ref->old_sha1, 0, check_old, msg, &err)) + goto fail; + + ret = ref_transaction_commit(transaction, &err); + if (ret) { + df_conflict = (ret == TRANSACTION_NAME_CONFLICT); + goto fail; + } + + ref_transaction_free(transaction); + strbuf_release(&err); return 0; +fail: + ref_transaction_free(transaction); + error("%s", err.buf); + strbuf_release(&err); + return df_conflict ? STORE_REF_ERROR_DF_CONFLICT + : STORE_REF_ERROR_OTHER; } #define REFCOL_WIDTH 10 -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 13/25] refs.c: make write_ref_sha1 static
From: Ronnie Sahlberg Date: Mon, 28 Apr 2014 15:36:58 -0700 No external users call write_ref_sha1 any more so let's declare it static. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- refs.c | 10 -- refs.h | 3 --- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 9d9bbeb..6b4236a 100644 --- a/refs.c +++ b/refs.c @@ -2706,6 +2706,9 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } +static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, + const char *logmsg); + int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { unsigned char sha1[20], orig_sha1[20]; @@ -2950,8 +2953,11 @@ int is_branch(const char *refname) return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/"); } -/* This function must return a meaningful errno */ -int write_ref_sha1(struct ref_lock *lock, +/* + * Write sha1 into the ref specified by the lock. Make sure that errno + * is sane on error. + */ +static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg) { static char term = '\n'; diff --git a/refs.h b/refs.h index 50b115a..eea1044 100644 --- a/refs.h +++ b/refs.h @@ -197,9 +197,6 @@ extern int commit_ref(struct ref_lock *lock); /** Release any lock taken but not written. **/ extern void unlock_ref(struct ref_lock *lock); -/** Writes sha1 into the ref specified by the lock. **/ -extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg); - /* * Setup reflog before using. Set errno to something meaningful on failure. */ -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 14/25] refs.c: change resolve_ref_unsafe reading argument to be a flags field
From: Ronnie Sahlberg Date: Tue, 15 Jul 2014 12:59:36 -0700 resolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref resolves successfully for writing but not for reading). Change this to be a flags field instead, and pass the new constant RESOLVE_REF_READING when we want this behaviour. While at it, swap two of the arguments in the function to put output arguments at the end. As a nice side effect, this ensures that we can catch callers that were unaware of the new API so they can be audited. Give the wrapper functions resolve_refdup and read_ref_full the same treatment for consistency. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- branch.c| 2 +- builtin/blame.c | 2 +- builtin/branch.c| 9 ++--- builtin/checkout.c | 6 ++-- builtin/clone.c | 2 +- builtin/commit.c| 2 +- builtin/fmt-merge-msg.c | 2 +- builtin/for-each-ref.c | 6 ++-- builtin/fsck.c | 2 +- builtin/log.c | 3 +- builtin/merge.c | 2 +- builtin/notes.c | 2 +- builtin/receive-pack.c | 4 +-- builtin/remote.c| 5 +-- builtin/show-branch.c | 7 ++-- builtin/symbolic-ref.c | 2 +- bundle.c| 2 +- cache.h | 23 ++-- http-backend.c | 4 ++- notes-merge.c | 2 +- reflog-walk.c | 5 +-- refs.c | 93 - remote.c| 11 +++--- sequencer.c | 4 +-- transport-helper.c | 5 ++- transport.c | 5 +-- upload-pack.c | 2 +- wt-status.c | 2 +- 28 files changed, 124 insertions(+), 92 deletions(-) diff --git a/branch.c b/branch.c index 884c62c..4bab55a 100644 --- a/branch.c +++ b/branch.c @@ -170,7 +170,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref, const char *head; unsigned char sha1[20]; - head = resolve_ref_unsafe("HEAD", sha1, 0, NULL); + head = resolve_ref_unsafe("HEAD", 0, sha1, NULL); if (!is_bare_repository() && head && !strcmp(head, ref->buf)) die(_("Cannot force update the current branch.")); } diff --git a/builtin/blame.c b/builtin/blame.c index 3838be2..303e217 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, commit->date = now; parent_tail = &commit->parents; - if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL)) + if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL)) die("no such ref: HEAD"); parent_tail = append_parent(parent_tail, head_sha1); diff --git a/builtin/branch.c b/builtin/branch.c index 6785097..6491bac 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -131,7 +131,8 @@ static int branch_merged(int kind, const char *name, branch->merge[0] && branch->merge[0]->dst && (reference_name = reference_name_to_free = -resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) +resolve_refdup(branch->merge[0]->dst, RESOLVE_REF_READING, + sha1, NULL)) != NULL) reference_rev = lookup_commit_reference(sha1); } if (!reference_rev) @@ -235,7 +236,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(name); name = mkpathdup(fmt, bname.buf); - target = resolve_ref_unsafe(name, sha1, 0, &flags); + target = resolve_ref_unsafe(name, 0, sha1, &flags); if (!target || (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) { error(remote_branch @@ -299,7 +300,7 @@ static char *resolve_symref(const char *src, const char *prefix) int flag; const char *dst; - dst = resolve_ref_unsafe(src, sha1, 0, &flag); + dst = resolve_ref_unsafe(src, 0, sha1, &flag); if (!(dst && (flag & REF_ISSYMREF))) return NULL; if (prefix) @@ -869,7 +870,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) track = git_branch_track; - head = resolve_refdup("HEAD", head_sha1, 0, NULL); + head = resolve_refdup("HEAD", 0, head_sha1, NULL); if (!head) die(_("Failed to resolve HEAD as a valid ref.")); if (!strcmp(head, "HEAD")) diff --git a/builtin/checkout.c b/builtin/checkout.c index b4decd5..5410dac 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -355,7 +355,7 @@ static int checkout_paths(const struct checkout_opts *opts, if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) die(_("unable to write new index file"))
[PATCH 16/25] branch -d: avoid repeated symref resolution
Date: Wed, 10 Sep 2014 18:22:48 -0700 If a repository gets in a broken state with too much symref nesting, it cannot be repaired with "git branch -d": $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense $ git branch -d nonsense error: branch 'nonsense' not found. Worse, "git update-ref --no-deref -d" doesn't work for such repairs either: $ git update-ref -d refs/heads/nonsense error: unable to resolve reference refs/heads/nonsense: Too many levels of symbolic links Fix both by teaching resolve_ref_unsafe a new RESOLVE_REF_NO_RECURSE flag and passing it when appropriate. Callers can still read the value of a symref (for example to print a message about it) with that flag set --- resolve_ref_unsafe will resolve one level of symrefs and stop there. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- builtin/branch.c | 3 ++- cache.h | 6 ++ refs.c| 17 +++-- refs.h| 2 ++ t/t1400-update-ref.sh | 34 ++ t/t3200-branch.sh | 9 + 6 files changed, 68 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6491bac..2ad2d0b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -236,7 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(name); name = mkpathdup(fmt, bname.buf); - target = resolve_ref_unsafe(name, 0, sha1, &flags); + target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE, + sha1, &flags); if (!target || (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) { error(remote_branch diff --git a/cache.h b/cache.h index 7200639..b735f3f 100644 --- a/cache.h +++ b/cache.h @@ -973,6 +973,11 @@ extern int read_ref(const char *refname, unsigned char *sha1); * "writing" to the ref, the return value is the name of the ref * that will actually be created or changed. * + * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one + * level of symbolic reference. The value stored in sha1 for a symbolic + * reference will always be null_sha1 in this case, and the return + * value is the reference that the symref refers to directly. + * * If flags is non-NULL, set the value that it points to the * combination of REF_ISPACKED (if the reference was found among the * packed references), REF_ISSYMREF (if the initial reference was a @@ -985,6 +990,7 @@ extern int read_ref(const char *refname, unsigned char *sha1); * errno is set to something meaningful on error. */ #define RESOLVE_REF_READING 0x01 +#define RESOLVE_REF_NO_RECURSE 0x02 extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags); extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags); diff --git a/refs.c b/refs.c index f8d59ab..4fe263e 100644 --- a/refs.c +++ b/refs.c @@ -1467,6 +1467,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned refname = refname_buffer; if (flags) *flags |= REF_ISSYMREF; + if (resolve_flags & RESOLVE_REF_NO_RECURSE) { + hashclr(sha1); + return refname; + } continue; } } @@ -1523,13 +1527,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned buf = buffer + 4; while (isspace(*buf)) buf++; + refname = strcpy(refname_buffer, buf); + if (resolve_flags & RESOLVE_REF_NO_RECURSE) { + hashclr(sha1); + return refname; + } if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) { if (flags) *flags |= REF_ISBROKEN; errno = EINVAL; return NULL; } - refname = strcpy(refname_buffer, buf); } } @@ -2170,6 +2178,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, if (mustexist) resolve_flags |= RESOLVE_REF_READING; + if (flags & REF_NODEREF && flags & REF_DELETING) + resolve_flags |= RESOLVE_REF_NO_RECURSE; refname = resolve_ref_unsafe(refname, resolve_flags, lock->old_sha1, &type); @@ -3664,13 +3674,16 @@ int ref_transaction_commit(struct ref_transaction *transaction, /* Acquire all locks while verifying old values */ for (i = 0; i < n; i++) {
[PATCH 15/25] reflog test: test interaction with detached HEAD
From: Junio C Hamano Date: Sat, 13 Sep 2014 10:52:25 -0700 A proposed patch produced broken HEAD reflog entries when checking out anything other than a branch. The testsuite still passed, so it took a few days for the bug to be noticed. Add tests checking the content of the reflog after detaching and reattaching HEAD so we don't have to rely on manual testing to catch such problems in the future. [jn: using 'log -g --format=%H' instead of parsing --oneline output, resetting state in each test so they can be safely reordered or skipped] Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- t/t1413-reflog-detach.sh | 70 1 file changed, 70 insertions(+) create mode 100755 t/t1413-reflog-detach.sh diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh new file mode 100755 index 000..c730600 --- /dev/null +++ b/t/t1413-reflog-detach.sh @@ -0,0 +1,70 @@ +#!/bin/sh + +test_description='Test reflog interaction with detached HEAD' +. ./test-lib.sh + +reset_state () { + git checkout master && + cp saved_reflog .git/logs/HEAD +} + +test_expect_success setup ' + test_tick && + git commit --allow-empty -m initial && + git branch side && + test_tick && + git commit --allow-empty -m second && + cat .git/logs/HEAD >saved_reflog +' + +test_expect_success baseline ' + reset_state && + git rev-parse master master^ >expect && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_expect_success 'switch to branch' ' + reset_state && + git rev-parse side master master^ >expect && + git checkout side && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_expect_success 'detach to other' ' + reset_state && + git rev-parse master side master master^ >expect && + git checkout side && + git checkout master^0 && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_expect_success 'detach to self' ' + reset_state && + git rev-parse master master master^ >expect && + git checkout master^0 && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_expect_success 'attach to self' ' + reset_state && + git rev-parse master master master master^ >expect && + git checkout master^0 && + git checkout master && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_expect_success 'attach to other' ' + reset_state && + git rev-parse side master master master^ >expect && + git checkout master^0 && + git checkout side && + git log -g --format=%H >actual && + test_cmp expect actual +' + +test_done -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 18/25] packed-ref cache: forbid dot-components in refnames
Date: Fri, 26 Sep 2014 12:22:22 -0700 Since v1.7.9-rc1~10^2 (write_head_info(): handle "extra refs" locally, 2012-01-06), this trick to keep track of ".have" refs that are only valid on the wire and not on the filesystem is not needed any more. Simplify by removing support for the REFNAME_DOT_COMPONENT flag. This means we'll be slightly stricter with invalid refs found in a packed-refs file or during clone. read_loose_refs() already checks for and skips refnames with .components so it is not affected. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- refs.c | 14 +++--- refs.h | 6 +- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/refs.c b/refs.c index 4fe263e..fe1352a 100644 --- a/refs.c +++ b/refs.c @@ -70,16 +70,8 @@ static int check_refname_component(const char *refname, int flags) out: if (cp == refname) return 0; /* Component has zero length. */ - if (refname[0] == '.') { - if (!(flags & REFNAME_DOT_COMPONENT)) - return -1; /* Component starts with '.'. */ - /* -* Even if leading dots are allowed, don't allow "." -* as a component (".." is prevented by a rule above). -*/ - if (refname[1] == '\0') - return -1; /* Component equals ".". */ - } + if (refname[0] == '.') + return -1; /* Component starts with '.'. */ if (cp - refname >= LOCK_SUFFIX_LEN && !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN)) return -1; /* Refname ends with ".lock". */ @@ -290,7 +282,7 @@ static struct ref_entry *create_ref_entry(const char *refname, struct ref_entry *ref; if (check_name && - check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT)) + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); len = strlen(refname) + 1; ref = xmalloc(sizeof(struct ref_entry) + len); diff --git a/refs.h b/refs.h index 79802d7..c61b8f4 100644 --- a/refs.h +++ b/refs.h @@ -229,7 +229,6 @@ extern int for_each_reflog(each_ref_fn, void *); #define REFNAME_ALLOW_ONELEVEL 1 #define REFNAME_REFSPEC_PATTERN 2 -#define REFNAME_DOT_COMPONENT 4 /* * Return 0 iff refname has the correct format for a refname according @@ -237,10 +236,7 @@ extern int for_each_reflog(each_ref_fn, void *); * If REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then * allow a "*" wildcard character in place of one of the name - * components. No leading or repeated slashes are accepted. If - * REFNAME_DOT_COMPONENT is set in flags, then allow refname - * components to start with "." (but not a whole component equal to - * "." or ".."). + * components. No leading or repeated slashes are accepted. */ extern int check_refname_format(const char *refname, int flags); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 17/25] branch -d: simplify by using RESOLVE_REF_READING
From: Ronnie Sahlberg Date: Thu, 11 Sep 2014 10:34:36 -0700 When "git branch -d" reads the branch it is about to delete, it used to avoid passing the RESOLVE_REF_READING ('treat missing ref as error') flag because a symref pointing to a nonexistent ref would show up as missing instead of as something that could be deleted. To check if a ref is actually missing, we then check - is it a symref? - if not, did it resolve to null_sha1? Now we pass RESOLVE_REF_NO_RECURSE and the correct information is returned for a symref even when it points to a missing ref. Simplify by relying on RESOLVE_REF_READING. No functional change intended. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/branch.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 2ad2d0b..0c7aac0 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -236,10 +236,11 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, free(name); name = mkpathdup(fmt, bname.buf); - target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE, + target = resolve_ref_unsafe(name, + RESOLVE_REF_READING + | RESOLVE_REF_NO_RECURSE, sha1, &flags); - if (!target || - (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) { + if (!target) { error(remote_branch ? _("remote branch '%s' not found.") : _("branch '%s' not found."), bname.buf); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 19/25] test: put tests for handling of bad ref names in one place
From: Ronnie Sahlberg Date: Thu, 25 Sep 2014 15:02:39 -0700 There's no straightforward way to grep for all tests dealing with invalid refs. Put them in a single test script so it is easy to see what functionality has not been exercised with bad ref names yet. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- t/t1400-update-ref.sh | 44 -- t/t1430-bad-ref-name.sh | 84 + t/t9300-fast-import.sh | 30 -- 3 files changed, 84 insertions(+), 74 deletions(-) create mode 100755 t/t1430-bad-ref-name.sh diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 7c8c41a..7b4707b 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -136,14 +136,6 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' ' test_path_is_missing .git/refs/heads/ref-to-bad ' -test_expect_success 'update-ref --no-deref -d can delete reference to broken name' ' - git symbolic-ref refs/heads/badname refs/heads/broken...ref && - test_when_finished "rm -f .git/refs/heads/badname" && - test_path_is_file .git/refs/heads/badname && - git update-ref --no-deref -d refs/heads/badname && - test_path_is_missing .git/refs/heads/badname -' - test_expect_success '(not) create HEAD with old sha1' " test_must_fail git update-ref HEAD $A $B " @@ -408,12 +400,6 @@ test_expect_success 'stdin fails create with no ref' ' grep "fatal: create: missing " err ' -test_expect_success 'stdin fails create with bad ref name' ' - echo "create ~a $m" >stdin && - test_must_fail git update-ref --stdin err && - grep "fatal: invalid ref format: ~a" err -' - test_expect_success 'stdin fails create with no new value' ' echo "create $a" >stdin && test_must_fail git update-ref --stdin err && @@ -432,12 +418,6 @@ test_expect_success 'stdin fails update with no ref' ' grep "fatal: update: missing " err ' -test_expect_success 'stdin fails update with bad ref name' ' - echo "update ~a $m" >stdin && - test_must_fail git update-ref --stdin err && - grep "fatal: invalid ref format: ~a" err -' - test_expect_success 'stdin fails update with no new value' ' echo "update $a" >stdin && test_must_fail git update-ref --stdin err && @@ -456,12 +436,6 @@ test_expect_success 'stdin fails delete with no ref' ' grep "fatal: delete: missing " err ' -test_expect_success 'stdin fails delete with bad ref name' ' - echo "delete ~a $m" >stdin && - test_must_fail git update-ref --stdin err && - grep "fatal: invalid ref format: ~a" err -' - test_expect_success 'stdin fails delete with too many arguments' ' echo "delete $a $m $m" >stdin && test_must_fail git update-ref --stdin err && @@ -734,12 +708,6 @@ test_expect_success 'stdin -z fails create with no ref' ' grep "fatal: create: missing " err ' -test_expect_success 'stdin -z fails create with bad ref name' ' - printf $F "create ~a " "$m" >stdin && - test_must_fail git update-ref -z --stdin err && - grep "fatal: invalid ref format: ~a " err -' - test_expect_success 'stdin -z fails create with no new value' ' printf $F "create $a" >stdin && test_must_fail git update-ref -z --stdin err && @@ -764,12 +732,6 @@ test_expect_success 'stdin -z fails update with too few args' ' grep "fatal: update $a: unexpected end of input when reading " err ' -test_expect_success 'stdin -z fails update with bad ref name' ' - printf $F "update ~a" "$m" "" >stdin && - test_must_fail git update-ref -z --stdin err && - grep "fatal: invalid ref format: ~a" err -' - test_expect_success 'stdin -z emits warning with empty new value' ' git update-ref $a $m && printf $F "update $a" "" "" >stdin && @@ -802,12 +764,6 @@ test_expect_success 'stdin -z fails delete with no ref' ' grep "fatal: delete: missing " err ' -test_expect_success 'stdin -z fails delete with bad ref name' ' - printf $F "delete ~a" "$m" >stdin && - test_must_fail git update-ref -z --stdin err && - grep "fatal: invalid ref format: ~a" err -' - test_expect_success 'stdin -z fails delete with no old value' ' printf $F "delete $a" >stdin && test_must_fail git update-ref -z --stdin err && diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh new file mode 100755 index 000..c7b19b0 --- /dev/null +++ b/t/t1430-bad-ref-name.sh @@ -0,0 +1,84 @@ +#!/bin/sh + +test_description='Test handling of ref names that check-ref-format rejects' +. ./test-lib.sh + +test_expect_success setup ' + test_commit one +' + +test_expect_success 'fast-import: fail on invalid branch name ".badbranchname"' ' + test_when_finished "rm -f .git/objects/pack_* .git/objects/index_*" && + cat >input <<-INPUT_END && + commit .badbranch
[PATCH 20/25] refs.c: allow listing and deleting badly named refs
From: Ronnie Sahlberg Date: Wed, 3 Sep 2014 11:45:43 -0700 We currently do not handle badly named refs well: $ cp .git/refs/heads/master .git/refs/heads/master.@\*@\\. $ git branch fatal: Reference has invalid format: 'refs/heads/master.@*@\.' $ git branch -D master.@\*@\\. error: branch 'master.@*@\.' not found. Users cannot recover from a badly named ref without manually finding and deleting the loose ref file or appropriate line in packed-refs. Making that easier will make it easier to tweak the ref naming rules in the future, for example to forbid shell metacharacters like '`' and '"', without putting people in a state that is hard to get out of. So allow "branch --list" to show these refs and allow "branch -d/-D" and "update-ref -d" to delete them. Other commands (for example to rename refs) will continue to not handle these refs but can be changed in later patches. Details: In resolving functions, refuse to resolve refs that don't pass the git-check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME flag is passed. Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to resolve refs that escape the refs/ directory and do not match the pattern [A-Z_]* (think "HEAD" and "MERGE_HEAD"). In locking functions, refuse to act on badly named refs unless they are being deleted and either are in the refs/ directory or match [A-Z_]*. Just like other invalid refs, flag resolved, badly named refs with the REF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them in all iteration functions except for for_each_rawref. Flag badly named refs (but not symrefs pointing to badly named refs) with a REF_BAD_NAME flag to make it easier for future callers to notice and handle them specially. For example, in a later patch for-each-ref will use this flag to detect refs whose names can confuse callers parsing for-each-ref output. In the transaction API, refuse to create or update badly named refs, but allow deleting them (unless they try to escape refs/ and don't match [A-Z_]*). Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/branch.c| 9 +-- cache.h | 17 +- refs.c | 148 ++-- refs.h | 12 +++- t/t1430-bad-ref-name.sh | 125 +++- 5 files changed, 273 insertions(+), 38 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 0c7aac0..7e113d6 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, name = mkpathdup(fmt, bname.buf); target = resolve_ref_unsafe(name, RESOLVE_REF_READING - | RESOLVE_REF_NO_RECURSE, + | RESOLVE_REF_NO_RECURSE + | RESOLVE_REF_ALLOW_BAD_NAME, sha1, &flags); if (!target) { error(remote_branch @@ -248,7 +249,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, continue; } - if (!(flags & REF_ISSYMREF) && + if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) && check_branch_commit(bname.buf, name, sha1, head_rev, kinds, force)) { ret = 1; @@ -268,8 +269,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, ? _("Deleted remote branch %s (was %s).\n") : _("Deleted branch %s (was %s).\n"), bname.buf, - (flags & REF_ISSYMREF) - ? target + (flags & REF_ISBROKEN) ? "broken" + : (flags & REF_ISSYMREF) ? target : find_unique_abbrev(sha1, DEFAULT_ABBREV)); } delete_branch_config(bname.buf); diff --git a/cache.h b/cache.h index b735f3f..0501f7d 100644 --- a/cache.h +++ b/cache.h @@ -981,16 +981,29 @@ extern int read_ref(const char *refname, unsigned char *sha1); * If flags is non-NULL, set the value that it points to the * combination of REF_ISPACKED (if the reference was found among the * packed references), REF_ISSYMREF (if the initial reference was a - * symbolic reference) and REF_ISBROKEN (if the ref is malformed). + * symbolic reference), REF_BAD_NAME (if the reference name is ill + * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN + * (if the ref is malformed or has a bad name). See refs.h for more detail + * on each flag. * * If ref is not a properly-formatted, normalized reference, return
[PATCH 22/25] remote rm/prune: print a message when writing packed-refs fails
From: Ronnie Sahlberg Date: Thu, 11 Sep 2014 08:42:57 -0700 Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to repack_without_refs, 2014-06-20), repack_without_refs forgot to provide an error message when commit_packed_refs fails. Even today, it only provides a message for callers that pass a non-NULL err parameter. Internal callers in refs.c pass non-NULL err but "git remote" does not. That means that "git remote rm" and "git remote prune" can fail without printing a message about why. Fix them by passing in a non-NULL err parameter and printing the returned message. This is the last caller to a ref handling function passing err == NULL. A later patch can drop support for err == NULL, avoiding such problems in the future. Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder Reviewed-by: Michael Haggerty --- builtin/remote.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 42a533a..7f28f92 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -749,13 +749,16 @@ static int mv(int argc, const char **argv) static int remove_branches(struct string_list *branches) { + struct strbuf err = STRBUF_INIT; const char **branch_names; int i, result = 0; branch_names = xmalloc(branches->nr * sizeof(*branch_names)); for (i = 0; i < branches->nr; i++) branch_names[i] = branches->items[i].string; - result |= repack_without_refs(branch_names, branches->nr, NULL); + if (repack_without_refs(branch_names, branches->nr, &err)) + result |= error("%s", err.buf); + strbuf_release(&err); free(branch_names); for (i = 0; i < branches->nr; i++) { @@ -1332,9 +1335,13 @@ static int prune_remote(const char *remote, int dry_run) delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs)); for (i = 0; i < states.stale.nr; i++) delete_refs[i] = states.stale.items[i].util; - if (!dry_run) - result |= repack_without_refs(delete_refs, - states.stale.nr, NULL); + if (!dry_run) { + struct strbuf err = STRBUF_INIT; + if (repack_without_refs(delete_refs, states.stale.nr, + &err)) + result |= error("%s", err.buf); + strbuf_release(&err); + } free(delete_refs); } -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 23/25] refs.c: do not permit err == NULL
Date: Thu, 28 Aug 2014 16:42:37 -0700 Some functions that take a strbuf argument to append an error treat !err as an indication that the message should be suppressed (e.g., ref_update_reject_duplicates). Others write the message to stderr on !err (e.g., repack_without_refs). Others crash (e.g., ref_transaction_update). Some of these behaviors are for historical reasons and others were accidents. Luckily no callers pass err == NULL any more. Simplify by consistently requiring the strbuf argument. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- refs.c | 46 +++--- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index e7000f2..097fb4b 100644 --- a/refs.c +++ b/refs.c @@ -2646,6 +2646,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) struct string_list_item *ref_to_delete; int i, ret, removed = 0; + assert(err); + /* Look for a packed ref */ for (i = 0; i < n; i++) if (get_packed_ref(refnames[i])) @@ -2656,13 +2658,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) return 0; /* no refname exists in packed refs */ if (lock_packed_refs(0)) { - if (err) { - unable_to_lock_message(git_path("packed-refs"), errno, - err); - return -1; - } - unable_to_lock_error(git_path("packed-refs"), errno); - return error("cannot delete '%s' from packed refs", refnames[i]); + unable_to_lock_message(git_path("packed-refs"), errno, err); + return -1; } packed = get_packed_refs(&ref_cache); @@ -2688,7 +2685,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) /* Write what remains */ ret = commit_packed_refs(); - if (ret && err) + if (ret) strbuf_addf(err, "unable to overwrite old ref-pack file: %s", strerror(errno)); return ret; @@ -2696,6 +2693,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err) { + assert(err); + if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) { /* * loose. The loose file name is the same as the @@ -3551,6 +3550,8 @@ struct ref_transaction { struct ref_transaction *ref_transaction_begin(struct strbuf *err) { + assert(err); + return xcalloc(1, sizeof(struct ref_transaction)); } @@ -3590,6 +3591,8 @@ int ref_transaction_update(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: update called for transaction that is not open"); @@ -3622,6 +3625,8 @@ int ref_transaction_create(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: create called for transaction that is not open"); @@ -3653,6 +3658,8 @@ int ref_transaction_delete(struct ref_transaction *transaction, { struct ref_update *update; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: delete called for transaction that is not open"); @@ -3715,13 +3722,14 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, struct strbuf *err) { int i; + + assert(err); + for (i = 1; i < n; i++) if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) { - const char *str = - "Multiple updates for ref '%s' not allowed."; - if (err) - strbuf_addf(err, str, updates[i]->refname); - + strbuf_addf(err, + "Multiple updates for ref '%s' not allowed.", + updates[i]->refname); return 1; } return 0; @@ -3735,6 +3743,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, int n = transaction->nr; struct ref_update **updates = transaction->updates; + assert(err); + if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: commit called for transaction that is not open"); @@ -3771,9 +3781,8 @@ int ref_transaction_commit(struct ref_transaction *transaction, ret = (errno == ENOTDIR) ? TRANSACTION_NAME_CONFLICT : TRANSACTION_GENERIC_ERROR; -
[PATCH 21/25] for-each-ref: skip and warn about broken ref names
From: Ronnie Sahlberg Date: Fri, 5 Sep 2014 14:35:17 -0700 Print a warning message for any bad ref names we find in the repo and skip them so callers don't have to deal with parsing them. It might be useful in the future to have a flag where we would not skip these refs for those callers that want to and are prepared (for example by using a --format argument with %0 as a delimiter after the ref name). Signed-off-by: Ronnie Sahlberg Signed-off-by: Jonathan Nieder --- builtin/for-each-ref.c | 5 + 1 file changed, 5 insertions(+) diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 492265d..3ee22b9 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -839,6 +839,11 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f struct refinfo *ref; int cnt; + if (flag & REF_BAD_NAME) { + warning("ignoring ref with broken name %s", refname); + return 0; + } + if (*cb->grab_pattern) { const char **pattern; int namelen = strlen(refname); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 25/25] ref_transaction_commit: bail out on failure to remove a ref
Date: Thu, 28 Aug 2014 17:01:35 -0700 When removal of a loose or packed ref fails, bail out instead of trying to finish the transaction. This way, a single error message can be printed (instead of multiple messages being concatenated by mistake) and the operator can try to solve the underlying problem before there is a chance to muck things up even more. In particular, when git fails to remove a ref, git goes on to try to delete the reflog. Exiting early lets us keep the reflog. When git succeeds in deleting a ref A and fails to remove a ref B, it goes on to try to delete both reflogs. It would be better to just remove the reflog for A, but that would be a more invasive change. Failing early means we keep both reflogs, which puts the operator in a good position to understand the problem and recover. A long term goal is to avoid these problems altogether and roll back the transaction on failure. That kind of transactionality will have to wait for a later series (the plan for which is to make all destructive work happen in a single update of the packed-refs file). Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- Thanks for reading. refs.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 097fb4b..0368ed4 100644 --- a/refs.c +++ b/refs.c @@ -3809,16 +3809,20 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if (update->lock) { - if (delete_ref_loose(update->lock, update->type, err)) + if (delete_ref_loose(update->lock, update->type, err)) { ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } if (!(update->flags & REF_ISPRUNING)) delnames[delnum++] = update->lock->ref_name; } } - if (repack_without_refs(delnames, delnum, err)) + if (repack_without_refs(delnames, delnum, err)) { ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } for (i = 0; i < delnum; i++) unlink_or_warn(git_path("logs/%s", delnames[i])); clear_loose_ref_cache(&ref_cache); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "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 24/25] lockfile: remove unable_to_lock_error
Date: Thu, 28 Aug 2014 16:41:34 -0700 The former caller uses unable_to_lock_message now. Signed-off-by: Jonathan Nieder Reviewed-by: Ronnie Sahlberg --- lockfile.c | 10 -- lockfile.h | 1 - 2 files changed, 11 deletions(-) diff --git a/lockfile.c b/lockfile.c index d098ade..4f16ee7 100644 --- a/lockfile.c +++ b/lockfile.c @@ -162,16 +162,6 @@ void unable_to_lock_message(const char *path, int err, struct strbuf *buf) absolute_path(path), strerror(err)); } -int unable_to_lock_error(const char *path, int err) -{ - struct strbuf buf = STRBUF_INIT; - - unable_to_lock_message(path, err, &buf); - error("%s", buf.buf); - strbuf_release(&buf); - return -1; -} - NORETURN void unable_to_lock_die(const char *path, int err) { struct strbuf buf = STRBUF_INIT; diff --git a/lockfile.h b/lockfile.h index dc066d1..cd2ec95 100644 --- a/lockfile.h +++ b/lockfile.h @@ -71,7 +71,6 @@ struct lock_file { #define LOCK_DIE_ON_ERROR 1 #define LOCK_NO_DEREF 2 -extern int unable_to_lock_error(const char *path, int err); extern void unable_to_lock_message(const char *path, int err, struct strbuf *buf); extern NORETURN void unable_to_lock_die(const char *path, int err); -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
meld output check is broken
Hello, It seems that support of "--output" by meld is performed by grepping its help output. However with meld 3.12 I get $ meld --help Usage: meld [OPTION...] Help Options: -h, --help Show help options --help-all Show all help options --help-gtk Show GTK+ Options --help-gapplicationShow GApplication options Application Options: --display=DISPLAY X display to use Yet it still supports --output and can show nice middle instead of << HEAD etc. markers. Thank you, Andrey -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Allow building Git with Asciidoctor
On Tue, Oct 14, 2014 at 10:08:19AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote: > > > > Specifically I'm not excited about getting into a state where we have to > > maintain both an asciidoc.conf file _and_ ruby extensions for > > asciidoctor. I don't mind if somebody wants to step up and keep the > > asciidoctor bits in sync with the asciidoc.conf, but I feel like one of > > them needs to be considered the "master". > > My so-far-unstated inclination, since seeing the patch to fix the > unbalanced example block separators from Brian (which was outside > and before this four-patch series), has been to keep our Makefile in > Documentation/ aware only of AsciiDoc while maintaining *.txt files > in a state so that AsciiDoctor could also be used to process them, > if people want to futz with their copies of Documentation/Makefile. > > I do not mind to have the machinery to run AsciiDoctor too much in > my tree. It may make it easier for those who use it to spot places > in *.txt that need (in)compatibility workarounds between the two > formatters than keeping it outside. I'd be happy if you simply picked up patch 3 and left out patch 4. It gets us most of the way there, which is good enough for most things. It's even possible to handle the litdd attribute on the command line, so the only thing we'd really lose is the linkgit links. Alternately, I'm happy to be responsible for maintaining the extensions.rb file. The asciidoc.conf file has not had a substantive (non-comment) change since 2012, and it has not had a change that would require an update to the extensions since 2010. I don't anticipate that keeping it up-to-date will require a significant amount of work. We can even drop it into contrib if you think that's a better place. It's really up to you which you'd prefer. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v4 0/1] Use absolute paths of lockfiles
Hi Michael: Good Morning. :) I see mh/lockfile and mh/lockfile-stdio were graduated to "master". So. does this patch continue? On top of mh/lockfile-stdio? Thank you. ^_^ Yue Lin Ho -- View this message in context: http://git.661346.n2.nabble.com/What-s-cooking-in-git-git-Aug-2014-02-Fri-8-tp7616651p7619900.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-prompt.sh: Option to hide prompt for ignored pwd
Optionally set __git_ps1 to display nothing when present working directory is ignored, triggered by the new environmental variable GIT_PS1_HIDE_IF_PWD_IGNORED. This environmental variable may be overridden on any repository by setting bash.hideIfPwdIgnored to "false". In the absence of GIT_PS1_HIDE_IF_PWD_IGNORED this change has no effect. Many people manage e.g. dotfiles in their home directory with git. This causes the prompt generated by __git_ps1 to refer to that "top level" repo while working in any descendant directory. That can be distracting, so this patch helps one shut off that noise. Signed-off-by: Jess Austin --- On Tue, Oct 14, 2014 at 2:08 PM, Richard Hansen wrote: > On 2014-10-14 14:47, Johannes Sixt wrote: >> Ahem, no. Please do not punish users who are not interested in the new >> feature with two new processes every time __git_ps() is run. Think of >> Windows where fork() is really, *really* expensive. > Regardless, it would be nice if the behavior matched the other bash.* > variables (only check the bash.* variable if the corresponding > environment variable is set, and default to true). The following should > fix it: > > if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] && >[ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] && >[ "$(git check-ignore .)" ] > then Thanks for helping me understand this! I think I have it correct now. On Tue, Oct 14, 2014 at 2:21 PM, Richard Hansen wrote: > I do prefer the new names. They are long, but how often will someone > have to type it? In this case it's better to be descriptive than to be > short. (I wonder if adding two letters would improve readability > further: GIT_PS1_HIDE_WHEN_PWD_IGNORED and bash.hideWhenPwdIgnored.) We got those two letters back with GIT_PS1_HIDE_IF_PWD_IGNORED and bash.hideIfPwdIgnored. > To avoid scaring people who might not want this feature enabled, I > recommend changing the subject line to something like this: > > git-prompt.sh: Option to hide prompt for ignored pwd Good idea! contrib/completion/git-prompt.sh | 12 t/t9903-bash-prompt.sh | 42 2 files changed, 54 insertions(+) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index c5473dc..151218b 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -84,6 +84,11 @@ # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on # the colored output of "git status -sb" and are available only when # using __git_ps1 for PROMPT_COMMAND or precmd. +# +# If you would like __git_ps1 to do nothing in the case when the current +# directory is set up to be ignored by git, then set +# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the +# repository level by setting bash.hideIfPwdIgnored to "false". # check whether printf supports -v __git_printf_supports_v= @@ -501,6 +506,13 @@ __git_ps1 () local f="$w$i$s$u" local gitstring="$c$b${f:+$z$f}$r$p" + if [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED}" ] && + [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] && + git check-ignore -q . + then + printf_format="" + fi + if [ $pcmode = yes ]; then if [ "${__git_printf_supports_v-}" != yes ]; then gitstring=$(printf -- "$printf_format" "$gitstring") diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 9150984..88a75cf 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' ' git commit -m "another b2" file && echo 000 >file && git commit -m "yet another b2" file && + mkdir ignored_dir && + echo "ignored_dir/" >> .gitignore && git checkout master ' @@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' ' test_cmp expected "$actual" ' +test_expect_success 'prompt - hide if pwd ignored - shell variable unset with config disabled' ' + printf " (master)" >expected && + test_config bash.hideIfPwdIgnored false && + ( + cd ignored_dir && + __git_ps1 >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - hide if pwd ignored - shell variable unset with config unset' ' + printf " (master)" >expected && + ( + cd ignored_dir && + __git_ps1 >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt - hide if pwd ignored - shell variable set with config disabled' ' + printf " (master)" >expected && + test_config bash.hideIfPwdIgnored false && + ( + cd ignored_dir && + GIT_PS1_HIDE_IF_PWD_IGNORED=y && + __git_ps1 >"$actual" + ) && + test_cmp expected "$actual" +' + +test_expect_success 'prompt -
Expected bug with reset --hard
git version 1.9.1 & version 2.1.2 I had some changes in a tracked file that I wanted to discard. Instead of using checkout, I instead used `git reset --hard HEAD` to reset. Git returned the message `HEAD is now at ` I then went `git status` and it showed me that I still have changes to commit I expected there to be no changes in my working directory as a result of doing a `git reset --hard` I did a little more fiddling around and still yielded the same results. - `$ touch file.md` - `$ git add file.md` - `$ git commit -m 'empty file'` - `$ echo 'text' > file.md` At this point git reported that I had uncommited changes for both file.md and the troublesome file in question (jquery.datatables.js) `$ git reset --hard HEAD` Git now no longer reports and untracked changes for file.md but still for jquery.datatables.js `$ git reset --hard HEAD~1` Git still reports untracked changes for jquery.datatables.js I have included a copy of the diff of the jquery.datatables.js diff --git a/vendor/assets/javascripts/jquery.datatables.js b/vendor/assets/javascripts/jquery.datatables.js index b9044f1..4aab04b 100644 --- a/vendor/assets/javascripts/jquery.datatables.js +++ b/vendor/assets/javascripts/jquery.datatables.js @@ -1,11 +1,11 @@ -/*! DataTables 1.10.1 - * ©2008-2014 SpryMedia Ltd - datatables.net/license +/*! DataTables 1.10.3 + * ©2008-2014 SpryMedia Ltd - datatables.net/license */ /** * @summary DataTables * @description Paginate, search and order HTML tables - * @version 1.10.1 + * @version 1.10.3 * @filejquery.dataTables.js * @author SpryMedia Ltd (www.sprymedia.co.uk) * @contact www.sprymedia.co.uk/contact @@ -113,7 +113,7 @@ // U+2009 is thin space and U+202F is narrow no-break space, both used in many // standards as thousands separators - var _re_formatted_numeric = /[',$£€¥%\u2009\u202F]/g; + var _re_formatted_numeric = /[',$£€¥%\u2009\u202F]/g; var _empty = function ( d ) { @@ -133,7 +133,7 @@ if ( ! _re_dic[ decimalPoint ] ) { _re_dic[ decimalPoint ] = new RegExp( _fnEscapeRegex( decimalPoint ), 'g' ); } - return typeof num === 'string' ? + return typeof num === 'string' && decimalPoint !== '.' ? num.replace( /\./g, '' ).replace( _re_dic[ decimalPoint ], '.' ) : num; }; @@ -310,7 +310,6 @@ newKey = key.replace( match[0], match[2].toLowerCase() ); map[ newKey ] = key; - //console.log( key, match ); if ( match[1] === 'o' ) { _fnHungarianMap( o[key] ); @@ -673,6 +672,12 @@ return _fnSetObjectDataFn( mDataSrc )( rowData, val, meta ); }; + // Indicate if DataTables should read DOM data as an object or array + // Used in _fnGetRowElements + if ( typeof mDataSrc !== 'number' ) { + oSettings._rowReadObject = true; + } + /* Feature sorting overrides column specific when off */ if ( !oSettings.oFeatures.bSort ) { @@ -1498,19 +1503,22 @@ function _fnGetRowElements( settings, row ) { var - d = [], tds = [], td = row.firstChild, name, col, o, i=0, contents, - columns = settings.aoColumns; + columns = settings.aoColumns, + objectRead = settings._rowReadObject; - var attr = function ( str, data, td ) { + var d = objectRead ? {} : []; + + var attr = function ( str, td ) { if ( typeof str === 'string' ) { var idx = str.indexOf('@'); if ( idx !== -1 ) { - var src = str.substring( idx+1 ); - o[ '@'+src ] = td.getAttribute( src ); + var attr = str.substring( idx+1 ); + var setter = _fnSetObjectDataFn( str ); + setter( d, td.getAttribute( attr ) ); } } }; @@ -1520,18 +1528,26 @@ contents = $.trim(cell.innerHTML); if ( col && col._bAttrSrc ) { - o = { - display: contents - }; + var setter = _fnSetOb
Re: Expected bug with reset --hard
I'd like to rescind my bug report. A bit more further investigation revealed that git was detecting the file with changes as jquery.datatables.js but the file in my directory was reported as being named jquery.dataTables.js . I'm currently working on OS X 10.9, and this issue is probably related more to the case-preservation of the file system rather than git itself. On 15 October 2014 14:34, Nicholas Chmielewski wrote: > git version 1.9.1 & version 2.1.2 > > I had some changes in a tracked file that I wanted to discard. > Instead of using checkout, I instead used `git reset --hard HEAD` to reset. > > Git returned the message `HEAD is now at ` > I then went `git status` and it showed me that I still have changes to commit > > I expected there to be no changes in my working directory as a result of > doing a `git reset --hard` > > I did a little more fiddling around and still yielded the same results. > > - `$ touch file.md` > - `$ git add file.md` > - `$ git commit -m 'empty file'` > - `$ echo 'text' > file.md` > > At this point git reported that I had uncommited changes for both file.md and > the troublesome file in question (jquery.datatables.js) > > `$ git reset --hard HEAD` > > Git now no longer reports and untracked changes for file.md but still > for jquery.datatables.js > > `$ git reset --hard HEAD~1` > > Git still reports untracked changes for jquery.datatables.js > > I have included a copy of the diff of the jquery.datatables.js -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] mergetool: don't require a work tree for --tool-help
On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote: > David Aguilar writes: > > > From: Charles Bailey > > > > Signed-off-by: Charles Bailey > > Signed-off-by: David Aguilar > > --- > > Changes since v2: > > > > This now uses the new git_dir_init function. > > > > git-mergetool.sh | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/git-mergetool.sh b/git-mergetool.sh > > index 96a61ba..cddb533 100755 > > --- a/git-mergetool.sh > > +++ b/git-mergetool.sh > > @@ -10,11 +10,11 @@ > > > > USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to > > merge] ...' > > SUBDIRECTORY_OK=Yes > > +NONGIT_OK=Yes > > OPTIONS_SPEC= > > TOOL_MODE=merge > > . git-sh-setup > > . git-mergetool--lib > > -require_work_tree > > > > # Returns true if the mode reflects a symlink > > is_symlink () { > > @@ -378,6 +378,9 @@ prompt_after_failed_merge () { > > done > > } > > > > +require_work_tree > > +git_dir_init > > This is somewhat curious. Shouldn't the order of these swapped? Yes. I'll send a replacement patch for 2/3 only. -- 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] mergetool: add an option for writing to a temporary directory
On Mon, Oct 13, 2014 at 12:24:41PM -0700, Junio C Hamano wrote: > David Aguilar writes: > > > Teach mergetool to write files in a temporary directory when > > 'mergetool.writeToTemp' is true. > > > > This is helpful for tools such as Eclipse which cannot cope with > > multiple copies of the same file in the worktree. > > With this can we drop the "change the temporary file name" patch by > Robin Rosenberg? > > http://thread.gmane.org/gmane.comp.version-control.git/255457/focus=255599 > > Message-Id: <1408607240-11369-1-git-send-email-robin.rosenb...@dewire.com> I would think so but I'm biased ;-) -- 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 v3 2/3] mergetool: don't require a work tree for --tool-help
On Tue, Oct 14, 2014 at 11:35:11PM -0700, David Aguilar wrote: > On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote: > > David Aguilar writes: > > > > > From: Charles Bailey > > > > > > Signed-off-by: Charles Bailey > > > Signed-off-by: David Aguilar > > > --- > > > Changes since v2: > > > > > > This now uses the new git_dir_init function. > > > > > > git-mergetool.sh | 5 - > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/git-mergetool.sh b/git-mergetool.sh > > > index 96a61ba..cddb533 100755 > > > --- a/git-mergetool.sh > > > +++ b/git-mergetool.sh > > > @@ -10,11 +10,11 @@ > > > > > > USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to > > > merge] ...' > > > SUBDIRECTORY_OK=Yes > > > +NONGIT_OK=Yes > > > OPTIONS_SPEC= > > > TOOL_MODE=merge > > > . git-sh-setup > > > . git-mergetool--lib > > > -require_work_tree > > > > > > # Returns true if the mode reflects a symlink > > > is_symlink () { > > > @@ -378,6 +378,9 @@ prompt_after_failed_merge () { > > > done > > > } > > > > > > +require_work_tree > > > +git_dir_init > > > > This is somewhat curious. Shouldn't the order of these swapped? > > Yes. I'll send a replacement patch for 2/3 only. Nevermind, I noticed you already fixed this up in pu. Thank you, -- 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