Re: [RFC/GSOC] Git Beginner | Warnings for potentially destructive commands
On Friday 25 March 2016 11:08 PM, Junio C Hamano wrote: > Sidhant Sharma writes: > >> $ ggit rebase >> >> [WARNING] You are about to rebase your commits in onto the >> $BASE_BRANCH, which will essentially replay the work done in $TOPIC_BRANCH >> since last merge onto $BASE_BRANCH. >> For instance, >> Current state: >> >> o---o---A---B $BASE_BRANCH >> \ >> X---Y $TOPIC_BRANCH >> >> State after rebasing: >> >> o---o---A---B---X'---Y' $BASE_BRANCH >> \ >> X---Y $TOPIC_BRANCH >> >> where X' and Y' are the commits making changes identical to those made by X >> and >> Y respectively. > The topology may be correct, but the branch labels are both wrong, > no? The tip of the base branch will stay at B, and the tip of the > topic will point at Y'. Thanks for pointing that out, will correct that. >> Rebasing is not usually problematic except in cases when you are rebasing >> commits that do not exist in your repository. > This cannot be correct, as you fundamentally cannot work on (not > limited to rebasing) commits that do not exist in your repository. > Actually I meant to refer to the commits that exist outside the local repo (eg. on the remote), like it says in the 'The Perils of Rebasing' section of the Git Branching and Rebasing documentation [1]. I'll rephrase it to make it clearer. >> $ ggit reset --hard >> >> Resetting to >> [WARNING] You are about to hard reset the current HEAD (master) by >> commit(s). > If I were on B and did "git reset --hard Y", i.e. > > o---o---A---B $CURRENT_BRANCH > \ >X---Y $CURRENT_BRANCH_AFTER_RESETTING > > does the phrasing "about to reset by commit(s):" make any sense? > >> This will take you back to commit , and discard all >> changes make thereafter. For instance, >> Current state: >> >> o---o---A---B---C---D---E $CURRENT_BRANCH >> >> After resetting 3 commits: >> >> o---o---A---B $CURRENT_BRANCH > The above two examples make me wonder if these should be static > text. "ggit rebase" and "ggit reset" have full information of the > concrete branch names, commit object names and the actual topology > of the history, so it should be able to give a description more > tailored to the user's situation. Instead of giving a fictional > drawing with "For instance, Current state:", it should be able to > draw the actual before-and-after picture based on where the end-user > actually is. I see _some_ attempts (e.g. with "", mention of > "(master)" and $BASE_BRANCH, you may have meant that they will be > replaced with actual values), but I suspect that telling some truth > (i.e. use of the real branch names) while showing pictures that do > not match the reality (i.e. if the topology and the description are > done as fixed text) would only confuse the users. On Sunday 27 March 2016 01:06 PM, Jacob Keller wrote: > On Sat, Mar 26, 2016 at 8:12 AM, Matthieu Moy > wrote: >> Jacob Keller writes: >> >>> If possible, I would suggest aiming for generating the actual topology >>> that the user is seeing, customized so that it gives relevenat >>> information, rather than static examples. >> Using the real topology in a useful way is actually pretty hard. It's >> quite easy to throw the output of "git log --graph --oneline ..." to the >> user, but as soon as the rebase deals with more than a handfull of >> commits, we'd want to simplify the history to show something >> understandable to the user (which by definition should be a beginner if >> he uses ggit), like replacing long sequences of commits with "..." or >> so. That is hard to get right. >> > Yes, in which case we should go Junio's route of not using anything > from the real topology. > I now understand why using the actual names with a hypothetical topology would only confuse the user. I'll update the diagrams with hypothetical names. Also, other than these commands, what others should I include in the list? I think we may also have warnings for the following: * git checkout -- * git rm --cached * git stash drop [] * git stash clear * All plumbing commands? Comments? Thanks and regards, Sidhant Sharma [1]: https://git-scm.com/book/en/v2/Git-Branching-Rebasing#The-Perils-of-Rebasing -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: Problem with git-http-backend.exe as iis cgi
Hi,
I put together a first patch for the issue.
Mit freundlichen Grüßen / With kind regards
Florian Manschwetus
E-Mail: manschwe...@cs-software-gmbh.de
Tel.: +49-(0)611-8908534
CS Software Concepts and Solutions GmbH
Geschäftsführer / Managing director: Dr. Werner Alexi
Amtsgericht Wiesbaden HRB 10004 (Commercial registry)
Schiersteiner Straße 31
D-65187 Wiesbaden
Germany
Tel.: 0611/8908555
-Ursprüngliche Nachricht-
Von: Konstantin Khomoutov [mailto:kostix+...@007spb.ru]
Gesendet: Donnerstag, 10. März 2016 13:55
An: Florian Manschwetus
Cc: git@vger.kernel.org
Betreff: Re: Problem with git-http-backend.exe as iis cgi
On Thu, 10 Mar 2016 07:28:50 +
Florian Manschwetus wrote:
> I tried to setup git-http-backend with iis, as iis provides proper
> impersonation for cgi under windows, which leads to have the
> filesystem access performed with the logon user, therefore the
> webserver doesn't need generic access to the files. I stumbled across
> a problem, ending up with post requests hanging forever. After some
> investigation I managed to get it work by wrapping the http-backend
> into a bash script, giving a lot of control about the environmental
> things, I was unable to solve within IIS configuration. The
> workaround, I use currently, is to use "/bin/head -c ${CONTENT_LENGTH}
> | ./git-http-backend.exe", which directly shows the issue. Git
> http-backend should check if CONTENT_LENGTH is set to something
> reasonable (e.g. >0) and should in this case read only CONTENT_LENGTH
> bytes from stdin, instead of reading till EOF what I suspect it is
> doing currently.
The rfc [1] states in its section 4.2:
| A request-body is supplied with the request if the CONTENT_LENGTH is
| not NULL. The server MUST make at least that many bytes available for
| the script to read. The server MAY signal an end-of-file condition
| after CONTENT_LENGTH bytes have been read or it MAY supply extension
| data. Therefore, the script MUST NOT attempt to read more than
| CONTENT_LENGTH bytes, even if more data is available. However, it is
| not obliged to read any of the data.
So yes, if Git currently reads until EOF, it's an error.
The correct way would be:
1) Check to see if the CONTENT_LENGTH variable is available in the
environment. If no, read nothing.
2) Otherwise read as many bytes it specifies, and no more.
1. https://www.ietf.org/rfc/rfc3875
http-backend-content-length.patch
Description: http-backend-content-length.patch
Re: [PATCH 4/7] submodule update --init: correct path handling in recursive submodules
Stefan Beller writes: > This fixes the test introduced by the parent commit. > > Signed-off-by: Stefan Beller Unlike 2/7, this is kinda on the thin side in the explanation department, it looks. > --- > git-submodule.sh| 5 +++-- > t/t7406-submodule-update.sh | 2 +- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/git-submodule.sh b/git-submodule.sh > index 2838069..a7c8599 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -474,7 +474,7 @@ cmd_init() > die_if_unmatched "$mode" > name=$(git submodule--helper name "$sm_path") || exit > > - displaypath=$(relative_path "$sm_path") > + displaypath=$(relative_path "$prefix$sm_path") > > # Copy url setting when it is not set yet > if test -z "$(git config "submodule.$name.url")" > @@ -826,8 +826,9 @@ Maybe you want to use 'update --init'?")" > if test -n "$recursive" > then > ( > - prefix="$prefix$sm_path/" > + prefix="$(relative_path $prefix$sm_path)/" Same here as 2/7 (see the above hunk which does this correctly). > clear_local_git_env > + wt_prefix= > cd "$sm_path" && > eval cmd_update > ) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] submodule update --init: test path handling in recursive submodules
Stefan Beller writes: > @@ -95,6 +99,35 @@ test_expect_success 'submodule update from subdirectory' ' > ) > ' > > +supersha1=$(cd super && git rev-parse HEAD) "supersha1=$(git -C super rev-parse HEAD)" perhaps? > +cat+Submodule path '../super': checked out '${supersha1}' > +Submodule 'merging' (${pwd}/merging) registered for path '../super/merging' > +Submodule 'none' (${pwd}/none) registered for path '../super/none' > +Submodule 'rebasing' (${pwd}/rebasing) registered for path > '../super/rebasing' > +Submodule 'submodule' (${pwd}/submodule) registered for path > '../super/submodule' > +Submodule path '../super/merging': checked out '${mergingsha1}' > +Submodule path '../super/none': checked out '${nonesha1}' > +Submodule path '../super/rebasing': checked out '${rebasingsha1}' > +Submodule path '../super/submodule': checked out '${submodulesha1}' > +EOF Why all these {curly braces}? -- To unsubscribe from this list: send the line "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/7] submodule foreach: correct path computation in recursive submodules
Stefan Beller writes: > The test which is fixed by this patch would report > Entering 'nested1/nested2/../nested3' > instead of the expected > Entering '../nested1/nested2/nested3' > > because the prefix is put unconditionally in front and after that a > computed display path with is affected by `wt_prefix`. This is wrong as > any relative path computation would need to be at the front. By emptying > the `wt_prefix` in recursive submodules and adding the information of any > relative path into the `prefix` this is fixed. > > Signed-off-by: Stefan Beller > --- Nicely explained and executed. FWIW, it is fine to have a fix and new tests to demonstrate the fix to pre-existing breakages in a single step. It is easier to review when we can see the body of the test (as opposed to just the change s/expect_failure/expect_success/) in the same patch as the change to the code for a focused and small fix like these patches 1 & 2; it is easy to partially revert the patch for such a focused and small fix when a reviewer wants to verify that the new tests fail without the code change. > git-submodule.sh | 3 ++- > t/t7407-submodule-foreach.sh | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/git-submodule.sh b/git-submodule.sh > index 43c68de..2838069 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -417,10 +417,11 @@ cmd_foreach() > say "$(eval_gettext "Entering '\$prefix\$displaypath'")" > name=$(git submodule--helper name "$sm_path") > ( > - prefix="$prefix$sm_path/" > + prefix="$(relative_path $prefix$sm_path)/" Make sure that "$prefix$sm_path" is given as a single string to relative_path. I'd probably write this like so: - prefix="$prefix$sm_path/" + prefix=$(relative_path "$prefix$sm_path")/ 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: [GSoC] A late proposal: a modern send-email
2016-03-29 0:49 GMT+08:00 Ævar Arnfjörð Bjarmason : > On Sat, Mar 26, 2016 at 3:13 AM, 惠轶群 wrote: >> 2016-03-26 2:16 GMT+08:00 Junio C Hamano : >>> 惠轶群 writes: >>> # Purpose The current implementation of send-email is based on perl and has only a tui, it has two problems: - user must install a ton of dependencies before submit a single patch. - tui and parameter are both not quite friendly to new users. >>> >>> Is "a ton of dependencies" true? "apt-cache show git-email" >>> suggests otherwise. Is "a ton of dependencies" truly a problem? >>> "apt-get install" would resolve the dependencies for you. >> >> There are three perl packages needed to send patch through gmail: >> - perl-mime-tools >> - perl-net-smtp-ssl >> - perl-authen-sasl >> >> Yes, not too many, but is it better none of them? >> >> What's more, when I try to send mails, I was first disrupted by >> "no perl-mime-tools" then by "no perl-net-smtp-ssl or perl-authen-sasl". >> Then I think, why not just a mailto link? > > I think your proposal should clarify a bit who these users are that > find it too difficult to install these perl module dependencies. Users > on OSX & Windows I would assume, because in the case of Linux distros > getting these is the equivalent of an apt-get command away. In fact, I'm not familiar with the build for OSX or Windows. > If installing these dependencies is hard for users perhaps a better > thing to focus on is altering the binary builds on Git for platforms > that don't have package systems to include these dependencies. Why `mailto` not a good choice? I'm confusing. > In this case it would mean shipping a statically linked OpenSSL since > that's what these perl SSL packages eventually depend on. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/17] read-cache: invalidate untracked cache data when reading WAMA
On Sat, Mar 19, 2016 at 8:04 AM, David Turner wrote:
> @@ -1407,10 +1472,24 @@ static int read_watchman_ext(struct index_state
> *istate, const void *data,
> ewah_each_bit(bitmap, mark_no_watchman, istate);
> ewah_free(bitmap);
>
> - /*
> -* TODO: update the untracked cache from the untracked data in this
> -* extension.
> -*/
> + if (istate->untracked && istate->untracked->root) {
> + int i;
> + const char *untracked;
> +
> + untracked = data + len + 8 + bitmap_size;
> + for (i = 0; i < untracked_nr; ++i) {
> + int len = strlen(untracked);
> +
> string_list_append(&istate->untracked->invalid_untracked,
> + untracked);
> + untracked += len + 1;
> + }
> +
> + for_each_string_list(&istate->untracked->invalid_untracked,
> +mark_untracked_invalid, istate->untracked);
I think it's a bit early to invalidate untracked cache here. We can do
that in refresh_by_watchman() in 10/17, where ce_mark_uptodate() to
prevent lstat() is also done.
> +
> + if (untracked_nr)
> + istate->cache_changed |= WATCHMAN_CHANGED;
> + }
> return 0;
> }
--
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 v2] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
2016-03-29 1:55 GMT+08:00 Jeff King :
> On Mon, Mar 28, 2016 at 11:56:10PM +0800, Hui Yiqun wrote:
>
>> According to strbuf.h, strbuf_detach is the sole supported method
>> to unwrap a memory buffer from its strbuf shell.
>>
>> So we should not return the pointer of strbuf.buf directly.
>>
>> What's more, some memory leakages are solved.
>
> There is something else going on here, which makes this case different
> than some others. Note that the function returns a const string:
>
>> diff --git a/path.c b/path.c
>> index 969b494..b07e5a7 100644
>> --- a/path.c
>> +++ b/path.c
>> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
>
> By convention, that means that the result is not owned by the caller,
> and should not be freed. We implement that by:
>
>> {
>> static struct strbuf validated_path = STRBUF_INIT;
>> static struct strbuf used_path = STRBUF_INIT;
>
> ...using static variables which persist after the call returns. So this
> function retains ownership of the memory, and it remains valid until the
> next call to enter_repo().
Sorry for my carelessness. I didn't notice it.
> There's no leak when we return NULL, because the function retains
> control of the memory (though it will hold onto it until the end of the
> program if nobody calls enter_repo() again). And thus we shouldn't use
> strbuf_detach(), which loses that reference to the memory (and since the
> callers don't take ownership, it actually _creates_ a leak).
>
> We could release the memory when returning, but I don't think it's a big
> deal to do so.
>
> -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 v3] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
2016-03-29 1:58 GMT+08:00 Junio C Hamano :
> Hui Yiqun writes:
>
>> According to strbuf.h, strbuf_detach is the sole supported method
>> to unwrap a memory buffer from its strbuf shell.
>> ...
>> diff --git a/path.c b/path.c
>> index 969b494..9801617 100644
>> --- a/path.c
>> +++ b/path.c
>> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
>> {
>> static struct strbuf validated_path = STRBUF_INIT;
>> static struct strbuf used_path = STRBUF_INIT;
>> ...
>> +return_null:
>> + free(dbuf);
>> + strbuf_release(&used_path);
>> + strbuf_release(&validated_path);
>> return NULL;
>> }
>
> I see these strbuf's are "static" storage class, so that they do not
> have to get freed.
I see, 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 v2 06/17] index-helper: add --detach
On Sat, Mar 19, 2016 at 8:04 AM, David Turner wrote:
> @@ -237,6 +239,7 @@ int main(int argc, char **argv)
> N_("exit if not used after some seconds")),
> OPT_BOOL(0, "strict", &to_verify,
> "verify shared memory after creating"),
N_() here (my bad)
> + OPT_BOOL(0, "detach", &detach, "detach the process"),
ditto
> OPT_END()
> };
>
> @@ -258,6 +261,10 @@ int main(int argc, char **argv)
> fd = setup_pipe(pipe_path.buf);
> if (fd < 0)
> die_errno("Could not set up index-helper.pipe");
_() here
> +
> + if (detach && daemonize(&daemonized))
> + die_errno("unable to detach");
and here
> +
> loop(fd, idle_in_seconds);
> return 0;
> }
> --
> 2.4.2.767.g62658d5-twtrsrc
>
--
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 v2 03/17] index-helper: new daemon for caching index and related stuff
On Sat, Mar 19, 2016 at 8:04 AM, David Turner wrote:
> From: Nguyễn Thái Ngọc Duy
>
> Instead of reading the index from disk and worrying about disk
> corruption, the index is cached in memory (memory bit-flips happen
> too, but hopefully less often). The result is faster read. Read time
> is reduced by 70%.
>
> The biggest gain is not having to verify the trailing SHA-1, which
> takes lots of time especially on large index files. But this also
> opens doors for further optimiztions:
>
> - we could create an in-memory format that's essentially the memory
>dump of the index to eliminate most of parsing/allocation
>overhead. The mmap'd memory can be used straight away. Experiment
>[1] shows we could reduce read time by 88%.
This reference [1] is missing (even in my old version). I believe it's
http://thread.gmane.org/gmane.comp.version-control.git/247268/focus=248771,
comparing 256.442ms in that mail with v4 number, 2245.113ms in 0/8
mail from the same thread.
> Git can poke the daemon via named pipes to tell it to refresh the
> index cache, or to keep it alive some more minutes. It can't give any
> real index data directly to the daemon. Real data goes to disk first,
> then the daemon reads and verifies it from there. Poking only happens
> for $GIT_DIR/index, not temporary index files.
I think we should go with unix socket on *nix platform instead of
named pipe. UNIX named pipe only allows one communication channel at a
time. Windows named pipe is different and allows multiple clients,
which is the same as unix socket.
> $GIT_DIR/index-helper.pipe is the named pipe for daemon process. The
> daemon reads from the pipe and executes commands. Commands that need
> replies from the daemon will have to open their own pipe, since a
> named pipe should only have one reader. Unix domain sockets don't
> have this problem, but are less portable.
Hm..NO_UNIX_SOCKETS is only set for Windows in config.mak.uname and
Windows will need to be specially tailored anyway, I think unix socket
would be more elegant.
> +static void share_index(struct index_state *istate, struct shm *is)
> +{
> + void *new_mmap;
> + if (istate->mmap_size <= 20 ||
> + hashcmp(istate->sha1,
> + (unsigned char *)istate->mmap + istate->mmap_size - 20) ||
> + !hashcmp(istate->sha1, is->sha1) ||
> + git_shm_map(O_CREAT | O_EXCL | O_RDWR, 0700, istate->mmap_size,
> + &new_mmap, PROT_READ | PROT_WRITE, MAP_SHARED,
> + "git-index-%s", sha1_to_hex(istate->sha1)) < 0)
> + return;
> +
> + release_index_shm(is);
> + is->size = istate->mmap_size;
> + is->shm = new_mmap;
> + hashcpy(is->sha1, istate->sha1);
> + memcpy(new_mmap, istate->mmap, istate->mmap_size - 20);
> +
> + /*
> +* The trailing hash must be written last after everything is
> +* written. It's the indication that the shared memory is now
> +* ready.
> +*/
> + hashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);
You commented here [1] a long time ago about memory barrier. I'm not
entirely sure if compilers dare to reorder function calls, but when
hashcpy is inlined and memcpy is builtin, I suppose that's possible...
[1] http://article.gmane.org/gmane.comp.version-control.git/280729
> +}
--
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 v3 1/2] refs: add a new function set_worktree_head_symref
On Mon, 2016-03-28 at 10:48 -0700, Junio C Hamano wrote:
> Kazuki Yamaguchi writes:
>
> > Add a new function set_worktree_head_symref, to update HEAD symref
> > for
> > the specified worktree.
> >
> > To update HEAD of a linked working tree,
> > create_symref("worktrees/$work_tree/HEAD", "refs/heads/$branch",
> > msg)
> > could be used. However when it comes to updating HEAD of the main
> > working tree, it is unusable because it uses $GIT_DIR for
> > worktree-specific symrefs (HEAD).
> >
> > The new function takes git_dir (real directory) as an argument, and
> > updates HEAD of the working tree. This function will be used when
> > renaming a branch.
> >
> > Signed-off-by: Kazuki Yamaguchi
> > ---
>
> I suspect that this helper function should be in the common layer
> (refs.c) to be redirected to specific backend(s).
>
> David & Michael, what do you think?
HEAD is a per-worktree ref, so it's always handled by the files
backend. So this looks fine to me.
> > +/*
> > + * Update HEAD of the specified gitdir.
> > + * Similar to create_symref("relative-git-dir/HEAD", target,
> > NULL), but this is
> > + * able to update the main working tree's HEAD wherever $GIT_DIR
> > points to.
nit: "able to update the main working tree's HEAD regardless of where
GIT_DIR points to" would be clearer.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
weird diff output?
On Mon, Mar 28, 2016 at 4:28 PM, Stefan Beller wrote: > cat > expect < +Entering '../nested1' > +Entering '../nested1/nested2' > +Entering '../nested1/nested2/nested3' > +Entering '../nested1/nested2/nested3/submodule' > +Entering '../sub1' > +Entering '../sub2' > +Entering '../sub3' > +EOF > + > +test_expect_failure 'test messages from "foreach --recursive" from > subdirectory' ' > + ( > + cd clone2 && > + mkdir untracked && > + cd untracked && > + git submodule foreach --recursive >../../actual > + ) && > + test_i18ncmp expect actual > +' > + > +cat > expect < nested1-nested1 > nested2-nested2 > nested3-nested3 Complete tangent here. The diff above looks like + + + + + is it possible to get diff output that would look more like + + + + + + instead? This is one of those huge readability issues with diff formatting that seems like both are completely correct, but the second way is much easier in general to read what was added. I don't understand why diff algorithms result in the former instead of the latter, and am curious if anyone knows whether this has ever been thought about or solved by someone. I've tried using various diffing algorithms (histogram, etc) and they always produce the same result above, and never what I would prefer. Regards, Jake -- To unsubscribe from this list: send the line "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: Gitk "External diff" broken when using worktree
I've done that, and here are the two arguments that it gives to the diff tool: >From the main repository (in which case the it works): .git/.gitk-tmp.CCxPmN/1/[1980e260494cbd225d482b5d962e77bdcdb2321c] RemoteTransaction.C .git/.gitk-tmp.CCxPmN/1/[2759bf6053e73cb5f7c11c646aee206242db2cd4] RemoteTransaction.C >From the auxiliary worktree (when the diff doesn't work): /home/daryl.vandenbrink/git-work/mdf_products/.git/worktrees/mdf2/.gitk-tmp.ZEdGpi/1/[1980e260494cbd225d482b5d962e77bdcdb2321c] RemoteTransaction.C /home/daryl.vandenbrink/git-work/mdf_products/.git/worktrees/mdf2/.gitk-tmp.ZEdGpi/1/[2759bf6053e73cb5f7c11c646aee206242db2cd4] RemoteTransaction.C I hope that helps. On 24 March 2016 at 19:52, Duy Nguyen wrote: > On Thu, Mar 24, 2016 at 9:55 AM, Daryl Van Den Brink > wrote: >> Hi, >> >> I'm using git 2.7.3 on cygwin, and have been taking advantage of the >> new "git worktree" feature. I noticed that when I launch gitk from one >> of the attached working directories, its "external diff" feature >> doesn't seem to work. Nothing shows up in the diff tool at all. >> However, it works if you launch gitk from the main repository. >> >> To reproduce: >> 1. Create a new working tree with "git worktree add" >> 2. From that new worktree, launch gitk. >> 3. Right-click in a file in the bottom right pane and click "External diff" >> 4. No useful diff appears. > > Works for me (on linux with 'master' branch). Maybe gitk selected > invalid tempdir on cygwin. You can try replace your external diff > program with a script or something that prints the whole command line. > That should reveal if gitk given paths are correct or not (or if gitk > fails even before that) > -- > Duy -- Daryl van den Brink Software Engineer Maptek | 31 Flemington Street, Glenside, SA 5065, Australia Tel: +61-8 8338 9222 | Dir: +61-8 8338 9222 | Fax: +61-8 8338 9229 daryl.vandenbr...@maptek.com.au | www.maptek.com Keep up to date with Maptek in our Forge newsletter. -- To unsubscribe from this list: send the line "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 6/7] submodule status: test path handling in recursive submodules
This replicates the previous test except that it executes from a sub directory. Document the expected outcome, which currently fails by having too many '../' prefixed. Signed-off-by: Stefan Beller --- t/t7407-submodule-foreach.sh | 19 +++ 1 file changed, 19 insertions(+) diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 808c6c6..5c57151 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -263,6 +263,25 @@ test_expect_success 'test "status --recursive"' ' ' cat > expect < ../../actual + ) && + test_cmp expect actual +' + +cat > expect
[PATCH 5/7] t7407: make expectation as clear as possible
Not everyone (including me) grasps the sed expression in a split second as they would grasp the 4 lines printed as is. Signed-off-by: Stefan Beller --- t/t7407-submodule-foreach.sh | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 776b349..808c6c6 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -262,8 +262,12 @@ test_expect_success 'test "status --recursive"' ' test_cmp expect actual ' -sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2 -mv -f expect2 expect +cat > expect
[PATCH 7/7] submodule status: fix path handling in recursive submodules
This fixes the test which was introduced in the parent commit. Signed-off-by: Stefan Beller --- git-submodule.sh | 1 + t/t7407-submodule-foreach.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index a7c8599..7503b27 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1161,6 +1161,7 @@ cmd_status() ( prefix="$displaypath/" clear_local_git_env + wt_prefix= cd "$sm_path" && eval cmd_status ) || diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 5c57151..91f9ca9 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -272,7 +272,7 @@ cat > expect
[PATCH 4/7] submodule update --init: correct path handling in recursive submodules
This fixes the test introduced by the parent commit.
Signed-off-by: Stefan Beller
---
git-submodule.sh| 5 +++--
t/t7406-submodule-update.sh | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2838069..a7c8599 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -474,7 +474,7 @@ cmd_init()
die_if_unmatched "$mode"
name=$(git submodule--helper name "$sm_path") || exit
- displaypath=$(relative_path "$sm_path")
+ displaypath=$(relative_path "$prefix$sm_path")
# Copy url setting when it is not set yet
if test -z "$(git config "submodule.$name.url")"
@@ -826,8 +826,9 @@ Maybe you want to use 'update --init'?")"
if test -n "$recursive"
then
(
- prefix="$prefix$sm_path/"
+ prefix="$(relative_path $prefix$sm_path)/"
clear_local_git_env
+ wt_prefix=
cd "$sm_path" &&
eval cmd_update
)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index c1b9ffa..3bd1552 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -118,7 +118,7 @@ Submodule path '../super/rebasing': checked out
'${rebasingsha1}'
Submodule path '../super/submodule': checked out '${submodulesha1}'
EOF
-test_expect_failure 'submodule update --init --recursive from subdirectory' '
+test_expect_success 'submodule update --init --recursive from subdirectory' '
git -C recursivesuper/super reset --hard HEAD^ &&
(cd recursivesuper &&
mkdir tmp &&
--
2.8.0.rc4.23.gd22361a.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] submodule foreach: correct path computation in recursive submodules
The test which is fixed by this patch would report Entering 'nested1/nested2/../nested3' instead of the expected Entering '../nested1/nested2/nested3' because the prefix is put unconditionally in front and after that a computed display path with is affected by `wt_prefix`. This is wrong as any relative path computation would need to be at the front. By emptying the `wt_prefix` in recursive submodules and adding the information of any relative path into the `prefix` this is fixed. Signed-off-by: Stefan Beller --- git-submodule.sh | 3 ++- t/t7407-submodule-foreach.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 43c68de..2838069 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -417,10 +417,11 @@ cmd_foreach() say "$(eval_gettext "Entering '\$prefix\$displaypath'")" name=$(git submodule--helper name "$sm_path") ( - prefix="$prefix$sm_path/" + prefix="$(relative_path $prefix$sm_path)/" clear_local_git_env cd "$sm_path" && sm_path=$(relative_path "$sm_path") && + wt_prefix= # we make $path available to scripts ... path=$sm_path && if test $# -eq 1 diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index f868636..776b349 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -187,7 +187,7 @@ Entering '../sub2' Entering '../sub3' EOF -test_expect_failure 'test messages from "foreach --recursive" from subdirectory' ' +test_expect_success 'test messages from "foreach --recursive" from subdirectory' ' ( cd clone2 && mkdir untracked && -- 2.8.0.rc4.23.gd22361a.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] submodule foreach: test path handling in recursive submodules
This replicates the previous test with the difference of executing from a sub directory. By executing from a sub directory all we would expect all paths to be prefixed by '../'. Document this as failure in this patch, to be fixed in a later patch. Signed-off-by: Stefan Beller --- t/t7407-submodule-foreach.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 7ca10b8..f868636 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -178,6 +178,26 @@ test_expect_success 'test messages from "foreach --recursive"' ' ' cat > expect <../../actual + ) && + test_i18ncmp expect actual +' + +cat > expect
[PATCH 0/7] Fix path bugs in submodule commands executed from sub dir [WAS: submodule--helper clone: lose the extra prefix option]
Junio wrote: > I suspect that the fix in your 1&2 may be demonstratable without > forcing an early failure by switching to "git -C". So for now I present test coverage and their minimal fixes. This series follows a "tick-tock" pattern except for patch5, which I wrote quickly as I was annoying by the bells and whistles. I expect test code to be dumb, not tricking ourselves by "smart" code there. The "tick" patches introduce failing tests. They need to fail to demonstrate the bugs exist, which are fixed in the "tock" patches, which are doing nothing fancy but just a one or two line correction of the path handling code. This applies to 2.8. As this is taking a completely different turn than I expected in "[PATCHv3 0/5] submodule helper: cleanup prefix passing", I made this a new series. (It also doesn't do cleanup any more, but just fixes bugs.) Thanks, Stefan Stefan Beller (7): submodule foreach: test path handling in recursive submodules submodule foreach: correct path computation in recursive submodules submodule update --init: test path handling in recursive submodules submodule update --init: correct path handling in recursive submodules t7407: make expectation as clear as possible submodule status: test path handling in recursive submodules submodule status: fix path handling in recursive submodules git-submodule.sh | 9 ++--- t/t7406-submodule-update.sh | 33 +++ t/t7407-submodule-foreach.sh | 47 ++-- 3 files changed, 84 insertions(+), 5 deletions(-) -- 2.8.0.rc4.23.gd22361a.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] submodule update --init: test path handling in recursive submodules
This demonstrates a failure to handle paths correctly.
Instead of getting the expected
Submodule 'submodule' (${pwd}/submodule) registered for path
'../super/submodule'
the `super` directory is omitted and you get
Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule'
instead.
Signed-off-by: Stefan Beller
---
t/t7406-submodule-update.sh | 33 +
1 file changed, 33 insertions(+)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 68ea31d..c1b9ffa 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -63,6 +63,10 @@ test_expect_success 'setup a submodule tree' '
git submodule add ../none none &&
test_tick &&
git commit -m "none"
+ ) &&
+ git clone . recursivesuper &&
+ ( cd recursivesuper
+git submodule add ../super super
)
'
@@ -95,6 +99,35 @@ test_expect_success 'submodule update from subdirectory' '
)
'
+supersha1=$(cd super && git rev-parse HEAD)
+mergingsha1=$(cd super/merging && git rev-parse HEAD)
+nonesha1=$(cd super/none && git rev-parse HEAD)
+rebasingsha1=$(cd super/rebasing && git rev-parse HEAD)
+submodulesha1=$(cd super/submodule && git rev-parse HEAD)
+pwd=$(pwd)
+
+cat ../../actual
+ ) &&
+ test_cmp expect actual
+'
+
apos="'";
test_expect_success 'submodule update does not fetch already present commits' '
(cd submodule &&
--
2.8.0.rc4.23.gd22361a.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning about conflict markers - undocumented 'diff --check' feature & suggestion
Ori Avtalion writes: > A bug report and a suggestion: > > `git diff --check` has been warning about conflict markers since 2008: > https://marc.info/?l=git&m=122398500726634&w=2 > > This is an undocumented feature. The current documentation for the > flag only mentions "whitespace errors". Thanks for digging (even though I do not think the message you quoted has much to do with this). 04954043 (diff --check: detect leftover conflict markers, 2008-06-26) was the change that added this check to the "check whitespace breakage" codepath, which forgot to update the documentation. And when 4f830390 (Documentation: git diff --check respects core.whitespace, 2011-06-22) rewrote the paragraph, it again didn't remember to update it to match the reality. Care to try a 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 v2 07/11] rerere: do use multiple variants
This enables the multiple-variant support for real. Multiple
conflicts of the same shape can have differences in contexts where
they appear, interfering the replaying of recorded resolution of one
conflict to another, and in such a case, their resolutions are
recorded as different variants under the same conflict ID.
We still need to adjust garbage collection codepaths for this
change, but the basic "replay" functionality is functional with
this change.
Signed-off-by: Junio C Hamano
---
* Mostly unchanged except for fixing the same glitch as 04/11.
rerere.c | 98 ++-
t/t4200-rerere.sh | 2 +-
2 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/rerere.c b/rerere.c
index 0cf857b..353227c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -74,7 +74,9 @@ static void assign_variant(struct rerere_id *id)
variant = id->variant;
if (variant < 0) {
- variant = 0; /* for now */
+ for (variant = 0; variant < rr_dir->status_nr; variant++)
+ if (!rr_dir->status[variant])
+ break;
}
fit_variant(rr_dir, variant);
id->variant = variant;
@@ -177,15 +179,6 @@ static int has_rerere_resolution(const struct rerere_id
*id)
return ((id->collection->status[variant] & both) == both);
}
-static int has_rerere_preimage(const struct rerere_id *id)
-{
- int variant = id->variant;
-
- if (variant < 0)
- return 0;
- return (id->collection->status[variant] & RR_HAS_PREIMAGE);
-}
-
static struct rerere_id *new_rerere_id_hex(char *hex)
{
struct rerere_id *id = xmalloc(sizeof(*id));
@@ -818,6 +811,13 @@ static void update_paths(struct string_list *update)
rollback_lock_file(&index_lock);
}
+static void remove_variant(struct rerere_id *id)
+{
+ unlink_or_warn(rerere_path(id, "postimage"));
+ unlink_or_warn(rerere_path(id, "preimage"));
+ id->collection->status[id->variant] = 0;
+}
+
/*
* The path indicated by rr_item may still have conflict for which we
* have a recorded resolution, in which case replay it and optionally
@@ -830,32 +830,46 @@ static void do_rerere_one_path(struct string_list_item
*rr_item,
{
const char *path = rr_item->string;
struct rerere_id *id = rr_item->util;
+ struct rerere_dir *rr_dir = id->collection;
int variant;
- if (id->variant < 0)
- assign_variant(id);
variant = id->variant;
- if (!has_rerere_preimage(id)) {
+ /* Has the user resolved it already? */
+ if (variant >= 0) {
+ if (!handle_file(path, NULL, NULL)) {
+ copy_file(rerere_path(id, "postimage"), path, 0666);
+ id->collection->status[variant] |= RR_HAS_POSTIMAGE;
+ fprintf(stderr, "Recorded resolution for '%s'.\n",
path);
+ free_rerere_id(rr_item);
+ rr_item->util = NULL;
+ return;
+ }
/*
-* We are the first to encounter this conflict. Ask
-* handle_file() to write the normalized contents to
-* the "preimage" file.
+* There may be other variants that can cleanly
+* replay. Try them and update the variant number for
+* this one.
*/
- handle_file(path, NULL, rerere_path(id, "preimage"));
- if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
- const char *path = rerere_path(id, "postimage");
- if (unlink(path))
- die_errno("cannot unlink stray '%s'", path);
- id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
- }
- id->collection->status[variant] |= RR_HAS_PREIMAGE;
- fprintf(stderr, "Recorded preimage for '%s'\n", path);
- return;
- } else if (has_rerere_resolution(id)) {
- /* Is there a recorded resolution we could attempt to apply? */
- if (merge(id, path))
- return; /* failed to replay */
+ }
+
+ /* Does any existing resolution apply cleanly? */
+ for (variant = 0; variant < rr_dir->status_nr; variant++) {
+ const int both = RR_HAS_PREIMAGE | RR_HAS_POSTIMAGE;
+ struct rerere_id vid = *id;
+
+ if ((rr_dir->status[variant] & both) != both)
+ continue;
+
+ vid.variant = variant;
+ if (merge(&vid, path))
+ continue; /* failed to replay */
+
+ /*
+* If there already is a different variant that applies
+* cleanly, there is no point maintaining our own variant.
+*/
+
[PATCH v2 11/11] rerere: adjust 'forget' to multi-variant world order
Because conflicts with the same contents inside conflict blocks
enclosed by "<<<" and ">>>" can now have multiple variants
to help three-way merge to adjust to the differences outside the
conflict blocks, "rerere forget $path" needs to be taught that there
may be multiple recorded resolutions that share the same conflict
hash (which groups the conflicts with "the same contents inside
conflict blocks"), among which there are some that would not be
relevant to the conflict we are looking at. These "other variants"
that happen to share the same conflict hash should not be cleared,
and the variant that would apply to the current conflict may not be
the zero-th one (which is the only one that is cleared by the
current code).
After finding the conflict hash, iterate over the existing variants
and try to resolve the conflict using each of them to find the one
that "cleanly" resolves the current conflict. That is the one we
want to forget and record the preimage for, so that the user can
record the corrected resolution.
Signed-off-by: Junio C Hamano
---
* New in v2. I _think_ we should refactor the code further in
order to avoid rewriting "thisimage", "thisimage.1", etc. on the
filesystem (instead, teach handle_path() to output to an in-core
buffer and keep it around during this loop), and doing the same
handle_cache() over and over again, but for now this should do.
rerere.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/rerere.c b/rerere.c
index e636d4b..1693866 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1038,7 +1038,33 @@ static int rerere_forget_one_path(const char *path,
struct string_list *rr)
/* Nuke the recorded resolution for the conflict */
id = new_rerere_id(sha1);
- id->variant = 0; /* for now */
+
+ for (id->variant = 0;
+id->variant < id->collection->status_nr;
+id->variant++) {
+ mmfile_t cur = { NULL, 0 };
+ mmbuffer_t result = {NULL, 0};
+ int cleanly_resolved;
+
+ if (!has_rerere_resolution(id))
+ continue;
+
+ handle_cache(path, sha1, rerere_path(id, "thisimage"));
+ if (read_mmfile(&cur, rerere_path(id, "thisimage"))) {
+ free(cur.ptr);
+ return error("Failed to update conflicted state in
'%s'",
+path);
+ }
+ cleanly_resolved = !try_merge(id, path, &cur, &result);
+ free(result.ptr);
+ free(cur.ptr);
+ if (cleanly_resolved)
+ break;
+ }
+
+ if (id->collection->status_nr <= id->variant)
+ return error("no remembered resolution for '%s'", path);
+
filename = rerere_path(id, "postimage");
if (unlink(filename))
return (errno == ENOENT
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Warning about conflict markers - undocumented 'diff --check' feature & suggestion
A bug report and a suggestion: `git diff --check` has been warning about conflict markers since 2008: https://marc.info/?l=git&m=122398500726634&w=2 This is an undocumented feature. The current documentation for the flag only mentions "whitespace errors". This check will also be useful in `git add`, to prevent accidental staging of conflict markers. Perhaps it could be included, if not by default, then at least with a configuration setting. Thoughts? Is this perhaps an existing feature that I'm missing? :) -Ori -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/11] rerere: allow multiple variants to exist
The shape of the conflict in a path determines the conflict ID. The
preimage and postimage pair that was recorded for the conflict ID
previously may or may not replay well for the conflict we just saw.
Currently, we punt when the previous resolution does not cleanly
replay, but ideally we should then be able to record the currently
conflicted path by assigning a new 'variant', and then record the
resolution the user is going to make.
Introduce a mechanism to have more than one variant for a given
conflict ID; we do not actually assign any variant other than 0th
variant yet at this step.
Signed-off-by: Junio C Hamano
---
rerere.c | 127 ---
rerere.h | 1 +
2 files changed, 106 insertions(+), 22 deletions(-)
diff --git a/rerere.c b/rerere.c
index 33b1348..0cf857b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -30,14 +30,17 @@ static int rerere_dir_alloc;
#define RR_HAS_PREIMAGE 2
static struct rerere_dir {
unsigned char sha1[20];
- unsigned char status;
+ int status_alloc, status_nr;
+ unsigned char *status;
} **rerere_dir;
static void free_rerere_dirs(void)
{
int i;
- for (i = 0; i < rerere_dir_nr; i++)
+ for (i = 0; i < rerere_dir_nr; i++) {
+ free(rerere_dir[i]->status);
free(rerere_dir[i]);
+ }
free(rerere_dir);
rerere_dir_nr = rerere_dir_alloc = 0;
rerere_dir = NULL;
@@ -53,17 +56,59 @@ static const char *rerere_id_hex(const struct rerere_id *id)
return sha1_to_hex(id->collection->sha1);
}
+static void fit_variant(struct rerere_dir *rr_dir, int variant)
+{
+ variant++;
+ ALLOC_GROW(rr_dir->status, variant, rr_dir->status_alloc);
+ if (rr_dir->status_nr < variant) {
+ memset(rr_dir->status + rr_dir->status_nr,
+ '\0', variant - rr_dir->status_nr);
+ rr_dir->status_nr = variant;
+ }
+}
+
+static void assign_variant(struct rerere_id *id)
+{
+ int variant;
+ struct rerere_dir *rr_dir = id->collection;
+
+ variant = id->variant;
+ if (variant < 0) {
+ variant = 0; /* for now */
+ }
+ fit_variant(rr_dir, variant);
+ id->variant = variant;
+}
+
const char *rerere_path(const struct rerere_id *id, const char *file)
{
if (!file)
return git_path("rr-cache/%s", rerere_id_hex(id));
- return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+ if (id->variant <= 0)
+ return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+
+ return git_path("rr-cache/%s/%s.%d",
+ rerere_id_hex(id), file, id->variant);
}
-static int is_rr_file(const char *name, const char *filename)
+static int is_rr_file(const char *name, const char *filename, int *variant)
{
- return !strcmp(name, filename);
+ const char *suffix;
+ char *ep;
+
+ if (!strcmp(name, filename)) {
+ *variant = 0;
+ return 1;
+ }
+ if (!skip_prefix(name, filename, &suffix) || *suffix != '.')
+ return 0;
+
+ errno = 0;
+ *variant = strtol(suffix + 1, &ep, 10);
+ if (errno || *ep)
+ return 0;
+ return 1;
}
static void scan_rerere_dir(struct rerere_dir *rr_dir)
@@ -74,10 +119,15 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
if (!dir)
return;
while ((de = readdir(dir)) != NULL) {
- if (is_rr_file(de->d_name, "postimage"))
- rr_dir->status |= RR_HAS_POSTIMAGE;
- else if (is_rr_file(de->d_name, "preimage"))
- rr_dir->status |= RR_HAS_PREIMAGE;
+ int variant;
+
+ if (is_rr_file(de->d_name, "postimage", &variant)) {
+ fit_variant(rr_dir, variant);
+ rr_dir->status[variant] |= RR_HAS_POSTIMAGE;
+ } else if (is_rr_file(de->d_name, "preimage", &variant)) {
+ fit_variant(rr_dir, variant);
+ rr_dir->status[variant] |= RR_HAS_PREIMAGE;
+ }
}
closedir(dir);
}
@@ -100,7 +150,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
if (pos < 0) {
rr_dir = xmalloc(sizeof(*rr_dir));
hashcpy(rr_dir->sha1, sha1);
- rr_dir->status = 0;
+ rr_dir->status = NULL;
+ rr_dir->status_nr = 0;
+ rr_dir->status_alloc = 0;
pos = -1 - pos;
/* Make sure the array is big enough ... */
@@ -118,19 +170,27 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
static int has_rerere_resolution(const struct rerere_id *id)
{
const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+ int variant = id->variant;
- return ((id->collection->status & both) == both);
+
[PATCH v2 10/11] rerere: split code to call ll_merge() further
The merge() helper function is given an existing rerere ID (i.e. the
name of the .git/rr-cache/* subdirectory, and the variant number)
that identifies one pair, try to see if the
conflicted state in the given path can be resolved by using the pair,
and if this succeeds, then update the conflicted path with the
result in the working tree.
To implement rerere_forget() in the multiple variant world, we'd
need a helper to do the "see if a pair cleanly
resolves a conflicted state we have in-core" part, without actually
touching any file in the working tree, in order to identify which
variant(s) to remove. Split the logic to do so into a separate
helper function try_merge() out of merge().
Signed-off-by: Junio C Hamano
---
* New in v2.
rerere.c | 47 +++
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/rerere.c b/rerere.c
index 2b870e0..e636d4b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -622,6 +622,33 @@ int rerere_remaining(struct string_list *merge_rr)
}
/*
+ * Try using the given conflict resolution "ID" to see
+ * if that recorded conflict resolves cleanly what we
+ * got in the "cur".
+ */
+static int try_merge(const struct rerere_id *id, const char *path,
+mmfile_t *cur, mmbuffer_t *result)
+{
+ int ret;
+ mmfile_t base = {NULL, 0}, other = {NULL, 0};
+
+ if (read_mmfile(&base, rerere_path(id, "preimage")) ||
+ read_mmfile(&other, rerere_path(id, "postimage")))
+ ret = 1;
+ else
+ /*
+* A three-way merge. Note that this honors user-customizable
+* low-level merge driver settings.
+*/
+ ret = ll_merge(result, path, &base, NULL, cur, "", &other, "",
NULL);
+
+ free(base.ptr);
+ free(other.ptr);
+
+ return ret;
+}
+
+/*
* Find the conflict identified by "id"; the change between its
* "preimage" (i.e. a previous contents with conflict markers) and its
* "postimage" (i.e. the corresponding contents with conflicts
@@ -635,30 +662,20 @@ static int merge(const struct rerere_id *id, const char
*path)
{
FILE *f;
int ret;
- mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
+ mmfile_t cur = {NULL, 0};
mmbuffer_t result = {NULL, 0};
/*
* Normalize the conflicts in path and write it out to
* "thisimage" temporary file.
*/
- if (handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) {
- ret = 1;
- goto out;
- }
-
- if (read_mmfile(&cur, rerere_path(id, "thisimage")) ||
- read_mmfile(&base, rerere_path(id, "preimage")) ||
- read_mmfile(&other, rerere_path(id, "postimage"))) {
+ if ((handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) ||
+ read_mmfile(&cur, rerere_path(id, "thisimage"))) {
ret = 1;
goto out;
}
- /*
-* A three-way merge. Note that this honors user-customizable
-* low-level merge driver settings.
-*/
- ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", NULL);
+ ret = try_merge(id, path, &cur, &result);
if (ret)
goto out;
@@ -684,8 +701,6 @@ static int merge(const struct rerere_id *id, const char
*path)
out:
free(cur.ptr);
- free(base.ptr);
- free(other.ptr);
free(result.ptr);
return ret;
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/11] rerere: gc and clear
Adjust "git rerere gc" and "git rerere clear" to the new world order
with rerere database with multiple variants for the same shape of
conflicts.
Signed-off-by: Junio C Hamano
---
* New in v2; a preview appeared on $gmane/288454
rerere.c | 87 ++-
t/t4200-rerere.sh | 82 ++-
2 files changed, 123 insertions(+), 46 deletions(-)
diff --git a/rerere.c b/rerere.c
index 353227c..3d4dd33 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1081,29 +1081,16 @@ int rerere_forget(struct pathspec *pathspec)
* Garbage collection support
*/
-/*
- * Note that this is not reentrant but is used only one-at-a-time
- * so it does not matter right now.
- */
-static struct rerere_id *dirname_to_id(const char *name)
-{
- static struct rerere_id id;
- id.collection = find_rerere_dir(name);
- return &id;
-}
-
-static time_t rerere_created_at(const char *dir_name)
+static time_t rerere_created_at(struct rerere_id *id)
{
struct stat st;
- struct rerere_id *id = dirname_to_id(dir_name);
return stat(rerere_path(id, "preimage"), &st) ? (time_t) 0 :
st.st_mtime;
}
-static time_t rerere_last_used_at(const char *dir_name)
+static time_t rerere_last_used_at(struct rerere_id *id)
{
struct stat st;
- struct rerere_id *id = dirname_to_id(dir_name);
return stat(rerere_path(id, "postimage"), &st) ? (time_t) 0 :
st.st_mtime;
}
@@ -1113,15 +1100,28 @@ static time_t rerere_last_used_at(const char *dir_name)
*/
static void unlink_rr_item(struct rerere_id *id)
{
- unlink(rerere_path(id, "thisimage"));
- unlink(rerere_path(id, "preimage"));
- unlink(rerere_path(id, "postimage"));
- /*
-* NEEDSWORK: what if this rmdir() fails? Wouldn't we then
-* assume that we already have preimage recorded in
-* do_plain_rerere()?
-*/
- rmdir(rerere_path(id, NULL));
+ unlink_or_warn(rerere_path(id, "thisimage"));
+ remove_variant(id);
+ id->collection->status[id->variant] = 0;
+}
+
+static void prune_one(struct rerere_id *id, time_t now,
+ int cutoff_resolve, int cutoff_noresolve)
+{
+ time_t then;
+ int cutoff;
+
+ then = rerere_last_used_at(id);
+ if (then)
+ cutoff = cutoff_resolve;
+ else {
+ then = rerere_created_at(id);
+ if (!then)
+ return;
+ cutoff = cutoff_noresolve;
+ }
+ if (then < now - cutoff * 86400)
+ unlink_rr_item(id);
}
void rerere_gc(struct string_list *rr)
@@ -1129,8 +1129,8 @@ void rerere_gc(struct string_list *rr)
struct string_list to_remove = STRING_LIST_INIT_DUP;
DIR *dir;
struct dirent *e;
- int i, cutoff;
- time_t now = time(NULL), then;
+ int i;
+ time_t now = time(NULL);
int cutoff_noresolve = 15;
int cutoff_resolve = 60;
@@ -1142,25 +1142,32 @@ void rerere_gc(struct string_list *rr)
die_errno("unable to open rr-cache directory");
/* Collect stale conflict IDs ... */
while ((e = readdir(dir))) {
+ struct rerere_dir *rr_dir;
+ struct rerere_id id;
+ int now_empty;
+
if (is_dot_or_dotdot(e->d_name))
continue;
-
- then = rerere_last_used_at(e->d_name);
- if (then) {
- cutoff = cutoff_resolve;
- } else {
- then = rerere_created_at(e->d_name);
- if (!then)
- continue;
- cutoff = cutoff_noresolve;
+ rr_dir = find_rerere_dir(e->d_name);
+ if (!rr_dir)
+ continue; /* or should we remove e->d_name? */
+
+ now_empty = 1;
+ for (id.variant = 0, id.collection = rr_dir;
+id.variant < id.collection->status_nr;
+id.variant++) {
+ prune_one(&id, now, cutoff_resolve, cutoff_noresolve);
+ if (id.collection->status[id.variant])
+ now_empty = 0;
}
- if (then < now - cutoff * 86400)
+ if (now_empty)
string_list_append(&to_remove, e->d_name);
}
closedir(dir);
- /* ... and then remove them one-by-one */
+
+ /* ... and then remove the empty directories */
for (i = 0; i < to_remove.nr; i++)
- unlink_rr_item(dirname_to_id(to_remove.items[i].string));
+ rmdir(git_path("rr-cache/%s", to_remove.items[i].string));
string_list_clear(&to_remove, 0);
}
@@ -1177,8 +1184,10 @@ void rerere_clear(struct string_list *merge_rr)
for (i = 0; i < merge_rr->nr; i++) {
st
[PATCH v2 03/11] rerere: handle leftover rr-cache/$ID directory and postimage files
If by some accident there is only $GIT_DIR/rr-cache/$ID directory
existed, we wouldn't have recorded a preimage for a conflict that
is newly encountered, which would mean after a manual resolution,
we wouldn't have recorded it by storing the postimage, because the
logic used to be "if there is no rr-cache/$ID directory, then we are
the first so record the preimage". Instead, record preimage if we
do not have one.
In addition, if there is only $GIT_DIR/rr-cache/$ID/postimage
without corresponding preimage, we would have tried to call into
merge() and punted.
These would have been a situation frustratingly hard to recover
from.
Signed-off-by: Junio C Hamano
---
* A test that saved away both postimage and preimage has been
updated to only save postimage, as we do not look at the other
one.
rerere.c | 42 +-
t/t4200-rerere.sh | 17 -
2 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/rerere.c b/rerere.c
index fbdade8..a1e2963 100644
--- a/rerere.c
+++ b/rerere.c
@@ -117,7 +117,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
static int has_rerere_resolution(const struct rerere_id *id)
{
- return (id->collection->status & RR_HAS_POSTIMAGE);
+ const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+
+ return ((id->collection->status & both) == both);
}
static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -806,24 +808,30 @@ static int do_plain_rerere(struct string_list *rr, int fd)
string_list_insert(rr, path)->util = id;
/*
-* If the directory does not exist, create
-* it. mkdir_in_gitdir() will fail with
-* EEXIST if there already is one.
-*
-* NEEDSWORK: make sure "gc" does not remove
-* preimage without removing the directory.
+* Ensure that the directory exists.
+* mkdir_in_gitdir() will fail with EEXIST if there
+* already is one.
*/
- if (mkdir_in_gitdir(rerere_path(id, NULL)))
- continue;
+ if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
+ errno != EEXIST)
+ continue; /* NEEDSWORK: perhaps we should die? */
- /*
-* We are the first to encounter this
-* conflict. Ask handle_file() to write the
-* normalized contents to the "preimage" file.
-*/
- handle_file(path, NULL, rerere_path(id, "preimage"));
- id->collection->status |= RR_HAS_PREIMAGE;
- fprintf(stderr, "Recorded preimage for '%s'\n", path);
+ if (id->collection->status & RR_HAS_PREIMAGE) {
+ ;
+ } else {
+ /*
+* We are the first to encounter this
+* conflict. Ask handle_file() to write the
+* normalized contents to the "preimage" file.
+*
+* NEEDSWORK: what should happen if we had a
+* leftover postimage that is totally
+* unrelated? Perhaps we should unlink it?
+*/
+ handle_file(path, NULL, rerere_path(id, "preimage"));
+ id->collection->status |= RR_HAS_PREIMAGE;
+ fprintf(stderr, "Recorded preimage for '%s'\n", path);
+ }
}
for (i = 0; i < rr->nr; i++)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index ed9c91e..c428011 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -184,12 +184,27 @@ test_expect_success 'rerere updates postimage timestamp' '
'
test_expect_success 'rerere clear' '
- rm $rr/postimage &&
+ mv $rr/postimage .git/post-saved &&
echo "$sha1 a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
git rerere clear &&
! test -d $rr
'
+test_expect_success 'leftover directory' '
+ git reset --hard &&
+ mkdir -p $rr &&
+ test_must_fail git merge first &&
+ test -f $rr/preimage
+'
+
+test_expect_success 'missing preimage' '
+ git reset --hard &&
+ mkdir -p $rr &&
+ cp .git/post-saved $rr/postimage &&
+ test_must_fail git merge first &&
+ test -f $rr/preimage
+'
+
test_expect_success 'set up for garbage collection tests' '
mkdir -p $rr &&
echo Hello >$rr/preimage &&
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/11] rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id
This will help fixing bootstrap corner-case issues, e.g. having an
empty $GIT_DIR/rr-cache/$ID directory would fail to record a
preimage, in later changes in this series.
Signed-off-by: Junio C Hamano
---
* Unchanged.
rerere.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/rerere.c b/rerere.c
index a5d8a06..fbdade8 100644
--- a/rerere.c
+++ b/rerere.c
@@ -26,8 +26,11 @@ static char *merge_rr_path;
static int rerere_dir_nr;
static int rerere_dir_alloc;
+#define RR_HAS_POSTIMAGE 1
+#define RR_HAS_PREIMAGE 2
static struct rerere_dir {
unsigned char sha1[20];
+ unsigned char status;
} **rerere_dir;
static void free_rerere_dirs(void)
@@ -58,6 +61,27 @@ const char *rerere_path(const struct rerere_id *id, const
char *file)
return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
}
+static int is_rr_file(const char *name, const char *filename)
+{
+ return !strcmp(name, filename);
+}
+
+static void scan_rerere_dir(struct rerere_dir *rr_dir)
+{
+ struct dirent *de;
+ DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->sha1)));
+
+ if (!dir)
+ return;
+ while ((de = readdir(dir)) != NULL) {
+ if (is_rr_file(de->d_name, "postimage"))
+ rr_dir->status |= RR_HAS_POSTIMAGE;
+ else if (is_rr_file(de->d_name, "preimage"))
+ rr_dir->status |= RR_HAS_PREIMAGE;
+ }
+ closedir(dir);
+}
+
static const unsigned char *rerere_dir_sha1(size_t i, void *table)
{
struct rerere_dir **rr_dir = table;
@@ -76,6 +100,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
if (pos < 0) {
rr_dir = xmalloc(sizeof(*rr_dir));
hashcpy(rr_dir->sha1, sha1);
+ rr_dir->status = 0;
pos = -1 - pos;
/* Make sure the array is big enough ... */
@@ -85,15 +110,14 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
memmove(rerere_dir + pos + 1, rerere_dir + pos,
(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
rerere_dir[pos] = rr_dir;
+ scan_rerere_dir(rr_dir);
}
return rerere_dir[pos];
}
static int has_rerere_resolution(const struct rerere_id *id)
{
- struct stat st;
-
- return !stat(rerere_path(id, "postimage"), &st);
+ return (id->collection->status & RR_HAS_POSTIMAGE);
}
static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -737,6 +761,7 @@ static void do_rerere_one_path(struct string_list_item
*rr_item,
} else if (!handle_file(path, NULL, NULL)) {
/* The user has resolved it. */
copy_file(rerere_path(id, "postimage"), path, 0666);
+ id->collection->status |= RR_HAS_POSTIMAGE;
fprintf(stderr, "Recorded resolution for '%s'.\n", path);
} else {
return;
@@ -797,6 +822,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
* normalized contents to the "preimage" file.
*/
handle_file(path, NULL, rerere_path(id, "preimage"));
+ id->collection->status |= RR_HAS_PREIMAGE;
fprintf(stderr, "Recorded preimage for '%s'\n", path);
}
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/11] rerere: delay the recording of preimage
We record the preimage only when there is no directory to record the
conflict we encountered, i.e. when $GIT_DIR/rr-cache/$ID does not
exist. As the plan is to allow multiple pairs
as variants for the same conflict ID eventually, this logic needs to
go.
As the first step in that direction, stop the "did we create the
directory? Then we record the preimage" logic. Instead, we record
if a preimage does not exist when we saw a conflict in a path. Also
make sure that we remove a stale postimage, which most likely is
totally unrelated to the resolution of this new conflict, when we
create a new preimage under $ID when $GIT_DIR/rr-cache/$ID already
exists.
In later patches, we will further update this logic to be "do we
have pair that cleanly resolve the current
conflicts? If not, record a new preimage as a new variant", but
that does not happen at this stage yet.
Signed-off-by: Junio C Hamano
---
* The previous one had a handful of confusions between which of
RR_HAS_{PRE,POST}IMAGE bit to clear, which was pointed out by
J6t. The corrections to them appear here and a few patches
that follow this one in this round.
rerere.c | 52 +---
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/rerere.c b/rerere.c
index a1e2963..33b1348 100644
--- a/rerere.c
+++ b/rerere.c
@@ -122,6 +122,11 @@ static int has_rerere_resolution(const struct rerere_id
*id)
return ((id->collection->status & both) == both);
}
+static int has_rerere_preimage(const struct rerere_id *id)
+{
+ return (id->collection->status & RR_HAS_PREIMAGE);
+}
+
static struct rerere_id *new_rerere_id_hex(char *hex)
{
struct rerere_id *id = xmalloc(sizeof(*id));
@@ -749,8 +754,24 @@ static void do_rerere_one_path(struct string_list_item
*rr_item,
const char *path = rr_item->string;
const struct rerere_id *id = rr_item->util;
- /* Is there a recorded resolution we could attempt to apply? */
- if (has_rerere_resolution(id)) {
+ if (!has_rerere_preimage(id)) {
+ /*
+* We are the first to encounter this conflict. Ask
+* handle_file() to write the normalized contents to
+* the "preimage" file.
+*/
+ handle_file(path, NULL, rerere_path(id, "preimage"));
+ if (id->collection->status & RR_HAS_POSTIMAGE) {
+ const char *path = rerere_path(id, "postimage");
+ if (unlink(path))
+ die_errno("cannot unlink stray '%s'", path);
+ id->collection->status &= ~RR_HAS_POSTIMAGE;
+ }
+ id->collection->status |= RR_HAS_PREIMAGE;
+ fprintf(stderr, "Recorded preimage for '%s'\n", path);
+ return;
+ } else if (has_rerere_resolution(id)) {
+ /* Is there a recorded resolution we could attempt to apply? */
if (merge(id, path))
return; /* failed to replay */
@@ -807,31 +828,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)
id = new_rerere_id(sha1);
string_list_insert(rr, path)->util = id;
- /*
-* Ensure that the directory exists.
-* mkdir_in_gitdir() will fail with EEXIST if there
-* already is one.
-*/
- if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
- errno != EEXIST)
- continue; /* NEEDSWORK: perhaps we should die? */
-
- if (id->collection->status & RR_HAS_PREIMAGE) {
- ;
- } else {
- /*
-* We are the first to encounter this
-* conflict. Ask handle_file() to write the
-* normalized contents to the "preimage" file.
-*
-* NEEDSWORK: what should happen if we had a
-* leftover postimage that is totally
-* unrelated? Perhaps we should unlink it?
-*/
- handle_file(path, NULL, rerere_path(id, "preimage"));
- id->collection->status |= RR_HAS_PREIMAGE;
- fprintf(stderr, "Recorded preimage for '%s'\n", path);
- }
+ /* Ensure that the directory exists. */
+ mkdir_in_gitdir(rerere_path(id, NULL));
}
for (i = 0; i < rr->nr; i++)
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/11] t4200: rerere a merge with two identical conflicts
When the context of multiple identical conflicts are different, two
seemingly the same conflict resolution cannot be safely applied.
In such a case, at least we should be able to record these two
resolutions separately in the rerere database, and reuse them when
we see the same conflict later.
Signed-off-by: Junio C Hamano
---
* Unchanged
t/t4200-rerere.sh | 74 +++
1 file changed, 74 insertions(+)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index c428011..6fcc6d4 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -406,4 +406,78 @@ test_expect_success 'rerere -h' '
test_i18ngrep [Uu]sage help
'
+concat_insert () {
+ last=$1
+ shift
+ cat early && printf "%s\n" "$@" && cat late "$last"
+}
+
+test_expect_failure 'multiple identical conflicts' '
+ git reset --hard &&
+
+ test_seq 1 6 >early &&
+ >late &&
+ test_seq 11 15 >short &&
+ test_seq 111 120 >long &&
+ concat_insert short >file1 &&
+ concat_insert long >file2 &&
+ git add file1 file2 &&
+ git commit -m base &&
+ git tag base &&
+ git checkout -b six.1 &&
+ concat_insert short 6.1 >file1 &&
+ concat_insert long 6.1 >file2 &&
+ git add file1 file2 &&
+ git commit -m 6.1 &&
+ git checkout -b six.2 HEAD^ &&
+ concat_insert short 6.2 >file1 &&
+ concat_insert long 6.2 >file2 &&
+ git add file1 file2 &&
+ git commit -m 6.2 &&
+
+ # At this point, six.1 and six.2
+ # - derive from common ancestor that has two files
+ # 1...6 7 11..15 (file1) and 1...6 7 111..120 (file2)
+ # - six.1 replaces these 7s with 6.1
+ # - six.2 replaces these 7s with 6.2
+
+ test_must_fail git merge six.1 &&
+
+ # Check that rerere knows that file1 and file2 have conflicts
+
+ printf "%s\n" file1 file2 >expect &&
+ git ls-files -u | sed -e "s/^.* //" | sort -u >actual &&
+ test_cmp expect actual &&
+
+ git rerere status | sort >actual &&
+ test_cmp expect actual &&
+
+ # Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+ concat_insert short 6.1 6.2 >file1 &&
+ concat_insert long 6.1 6.2 >file2 &&
+
+ git rerere remaining >actual &&
+ test_cmp expect actual &&
+
+ # We resolved file1 and file2
+ git rerere &&
+ >expect &&
+ git rerere remaining >actual &&
+ test_cmp expect actual &&
+
+ # Now we should be able to resolve them both
+ git reset --hard &&
+ test_must_fail git merge six.1 &&
+ git rerere &&
+
+ >expect &&
+ git rerere remaining >actual &&
+ test_cmp expect actual &&
+
+ concat_insert short 6.1 6.2 >file1.expect &&
+ concat_insert long 6.1 6.2 >file2.expect &&
+ test_cmp file1.expect file1 &&
+ test_cmp file2.expect file2
+'
+
test_done
--
2.8.0-215-g046a488
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/11] rerere: move code related to "forget" together
"rerere forget" is the only user of handle_cache() helper, which in
turn is the only user of rerere_io that reads from an in-core buffer
whose getline method is implemented as rerere_mem_getline(). Gather
them together.
Signed-off-by: Junio C Hamano
---
* New in v2.
rerere.c | 194 +++
1 file changed, 97 insertions(+), 97 deletions(-)
diff --git a/rerere.c b/rerere.c
index 3d4dd33..2b870e0 100644
--- a/rerere.c
+++ b/rerere.c
@@ -517,103 +517,6 @@ static int handle_file(const char *path, unsigned char
*sha1, const char *output
}
/*
- * Subclass of rerere_io that reads from an in-core buffer that is a
- * strbuf
- */
-struct rerere_io_mem {
- struct rerere_io io;
- struct strbuf input;
-};
-
-/*
- * ... and its getline() method implementation
- */
-static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)
-{
- struct rerere_io_mem *io = (struct rerere_io_mem *)io_;
- char *ep;
- size_t len;
-
- strbuf_release(sb);
- if (!io->input.len)
- return -1;
- ep = memchr(io->input.buf, '\n', io->input.len);
- if (!ep)
- ep = io->input.buf + io->input.len;
- else if (*ep == '\n')
- ep++;
- len = ep - io->input.buf;
- strbuf_add(sb, io->input.buf, len);
- strbuf_remove(&io->input, 0, len);
- return 0;
-}
-
-static int handle_cache(const char *path, unsigned char *sha1, const char
*output)
-{
- mmfile_t mmfile[3] = {{NULL}};
- mmbuffer_t result = {NULL, 0};
- const struct cache_entry *ce;
- int pos, len, i, hunk_no;
- struct rerere_io_mem io;
- int marker_size = ll_merge_marker_size(path);
-
- /*
-* Reproduce the conflicted merge in-core
-*/
- len = strlen(path);
- pos = cache_name_pos(path, len);
- if (0 <= pos)
- return -1;
- pos = -pos - 1;
-
- while (pos < active_nr) {
- enum object_type type;
- unsigned long size;
-
- ce = active_cache[pos++];
- if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
- break;
- i = ce_stage(ce) - 1;
- if (!mmfile[i].ptr) {
- mmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);
- mmfile[i].size = size;
- }
- }
- for (i = 0; i < 3; i++)
- if (!mmfile[i].ptr && !mmfile[i].size)
- mmfile[i].ptr = xstrdup("");
-
- /*
-* NEEDSWORK: handle conflicts from merges with
-* merge.renormalize set, too
-*/
- ll_merge(&result, path, &mmfile[0], NULL,
-&mmfile[1], "ours",
-&mmfile[2], "theirs", NULL);
- for (i = 0; i < 3; i++)
- free(mmfile[i].ptr);
-
- memset(&io, 0, sizeof(io));
- io.io.getline = rerere_mem_getline;
- if (output)
- io.io.output = fopen(output, "w");
- else
- io.io.output = NULL;
- strbuf_init(&io.input, 0);
- strbuf_attach(&io.input, result.ptr, result.size, result.size);
-
- /*
-* Grab the conflict ID and optionally write the original
-* contents with conflict markers out.
-*/
- hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
- strbuf_release(&io.input);
- if (io.io.output)
- fclose(io.io.output);
- return hunk_no;
-}
-
-/*
* Look at a cache entry at "i" and see if it is not conflicting,
* conflicting and we are willing to handle, or conflicting and
* we are unable to handle, and return the determination in *type.
@@ -1005,6 +908,103 @@ int rerere(int flags)
return status;
}
+/*
+ * Subclass of rerere_io that reads from an in-core buffer that is a
+ * strbuf
+ */
+struct rerere_io_mem {
+ struct rerere_io io;
+ struct strbuf input;
+};
+
+/*
+ * ... and its getline() method implementation
+ */
+static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)
+{
+ struct rerere_io_mem *io = (struct rerere_io_mem *)io_;
+ char *ep;
+ size_t len;
+
+ strbuf_release(sb);
+ if (!io->input.len)
+ return -1;
+ ep = memchr(io->input.buf, '\n', io->input.len);
+ if (!ep)
+ ep = io->input.buf + io->input.len;
+ else if (*ep == '\n')
+ ep++;
+ len = ep - io->input.buf;
+ strbuf_add(sb, io->input.buf, len);
+ strbuf_remove(&io->input, 0, len);
+ return 0;
+}
+
+static int handle_cache(const char *path, unsigned char *sha1, const char
*output)
+{
+ mmfile_t mmfile[3] = {{NULL}};
+ mmbuffer_t result = {NULL, 0};
+ const struct cache_entry *ce;
+ int pos, len, i, hunk_no;
+ struct rerere_io_mem io;
+ int marker_size = ll_merge_marker_size(path);
+
+
[ANNOUNCE] Git v2.8.0
The latest feature release Git v2.8.0 is now available at the usual places. It is comprised of 532 non-merge commits since v2.7.0, contributed by 74 people, 22 of which are new faces. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/ The following public repositories all have a copy of the 'v2.8.0' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git New contributors whose contributions weren't in v2.7.0 are as follows. Welcome to the Git development community! 마누엘, Adam Dinwoodie, Andrew Wheeler, Changwoo Ryu, Christoph Egger, Christoph Hoopmann, Dan Aloni, Dave Ware, David A. Wheeler, Dickson Wong, Felipe Gonçalves Assis, GyuYong Jung, Jon Griffiths, Kazutoshi Satoda, Lars Vogel, Martin Amdisen, Matthew Kraai, Paul Wagland, Rob Mayoff, Romain Picard, Vasco Almeida, and Victor Leschuk. Returning contributors who helped this release are as follows. Thanks for your continued support. Alexander Kuleshov, Alexander Shopov, Alex Henrie, Audric Schiltknecht, brian m. carlson, Carlos Martín Nieto, Christian Couder, David A. Greene, David Turner, Dennis Kaarsemaker, Dimitriy Ryazantcev, Edmundo Carmona Antoranz, Elia Pinto, Eric Sunshine, Eric Wong, Guillermo S. Romero, Jacob Keller, Jean-Noel Avila, Jeff King, Jiang Xin, Johannes Schindelin, Johannes Sixt, John Keeping, Jonathan Nieder, Junio C Hamano, Karsten Blees, Karthik Nayak, Knut Franke, Lars Schneider, Matthieu Moy, Matt McCutchen, Michael J Gruber, Mike Hommey, Nguyễn Thái Ngọc Duy, Øyvind A. Holm, Patrick Steinhardt, Pat Thoyts, Peter Krefting, Ralf Thielow, Ray Chen, Sebastian Schuberth, Shawn O. Pearce, Stefan Beller, Stephen P. Smith, SZEDER Gábor, Thomas Ackermann, Thomas Braun, Thomas Gummerer, Tobias Klauser, Torsten Bögershausen, Trần Ngọc Quân, and Will Palmer. Git 2.8 Release Notes = Backward compatibility note --- The rsync:// transport has been removed. Updates since v2.7 -- UI, Workflows & Features * It turns out "git clone" over rsync transport has been broken when the source repository has packed references for a long time, and nobody noticed nor complained about it. * "push" learned that its "--delete" option can be shortened to "-d", just like "branch --delete" and "branch -d" are the same thing. * "git blame" learned to produce the progress eye-candy when it takes too much time before emitting the first line of the result. * "git grep" can now be configured (or told from the command line) how many threads to use when searching in the working tree files. * Some "git notes" operations, e.g. "git log --notes=", should be able to read notes from any tree-ish that is shaped like a notes tree, but the notes infrastructure required that the argument must be a ref under refs/notes/. Loosen it to require a valid ref only when the operation would update the notes (in which case we must have a place to store the updated notes tree, iow, a ref). * "git grep" by default does not fall back to its "--no-index" behavior outside a directory under Git's control (otherwise the user may by mistake end up running a huge recursive search); with a new configuration (set in $HOME/.gitconfig--by definition this cannot be set in the config file per project), this safety can be disabled. * "git pull --rebase" has been extended to allow invoking "rebase -i". * "git p4" learned to cope with the type of a file getting changed. * "git format-patch" learned to notice format.outputDirectory configuration variable. This allows "-o " option to be omitted on the command line if you always use the same directory in your workflow. * "interpret-trailers" has been taught to optionally update a file in place, instead of always writing the result to the standard output. * Many commands that read files that are expected to contain text that is generated (or can be edited) by the end user to control their behavior (e.g. "git grep -f ") have been updated to be more tolerant to lines that are terminated with CRLF (they used to treat such a line to contain payload that ends with CR, which is usually not what the users expect). * "git notes merge" used to limit the source of the merged notes tree to somewhere under refs/notes/ hierarchy, which was too limiting when inventing a workflow to exchange notes with remote repositories using remote-tracking notes trees (located in e.g. refs/remote-notes/ or somesuch). * "git ls-files" learned a new "--eol" option to help diagnose end-of-line problems. * "ls-remo
[PATCH v2 00/11] saving and replaying multiple variants with rerere
Take two after several months. That is what you get when you end up running a busy project, instead of staying to be an individual contributor ;-). The previous round starts at the message this is a response to and is found at http://thread.gmane.org/gmane.comp.version-control.git/277878 The previous round completed the basic "record and replay", while leaving "gc" and "forget" as known-to-be-stale bits. This round attempts to complete them. Junio C Hamano (11): rerere: split conflict ID further rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id rerere: handle leftover rr-cache/$ID directory and postimage files rerere: delay the recording of preimage rerere: allow multiple variants to exist t4200: rerere a merge with two identical conflicts rerere: do use multiple variants rerere: gc and clear rerere: move code related to "forget" together rerere: split code to call ll_merge() further rerere: adjust 'forget' to multi-variant world order rerere.c | 626 +- rerere.h | 4 +- t/t4200-rerere.sh | 159 +- 3 files changed, 593 insertions(+), 196 deletions(-) -- 2.8.0-215-g046a488 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
A note from the maintainer
Welcome to the Git development community.
This message is written by the maintainer and talks about how Git
project is managed, and how you can work with it.
* Mailing list and the community
The development is primarily done on the Git mailing list. Help
requests, feature proposals, bug reports and patches should be sent to
the list address . You don't have to be
subscribed to send messages. The convention on the list is to keep
everybody involved on Cc:, so it is unnecessary to say "Please Cc: me,
I am not subscribed".
Before sending patches, please read Documentation/SubmittingPatches
and Documentation/CodingGuidelines to familiarize yourself with the
project convention.
If you sent a patch and you did not hear any response from anybody for
several days, it could be that your patch was totally uninteresting,
but it also is possible that it was simply lost in the noise. Please
do not hesitate to send a reminder message in such a case. Messages
getting lost in the noise may be a sign that those who can evaluate
your patch don't have enough mental/time bandwidth to process them
right at the moment, and it often helps to wait until the list traffic
becomes calmer before sending such a reminder.
The list archive is available at a few public sites:
http://news.gmane.org/gmane.comp.version-control.git/
http://marc.info/?l=git
http://www.spinics.net/lists/git/
For those who prefer to read it over NNTP:
nntp://news.gmane.org/gmane.comp.version-control.git
When you point at a message in a mailing list archive, using
gmane is often the easiest to follow by readers, like this:
http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217
as it also allows people who subscribe to the mailing list as gmane
newsgroup to "jump to" the article.
Some members of the development community can sometimes be found on
the #git and #git-devel IRC channels on Freenode. Their logs are
available at:
http://colabti.org/irclogger/irclogger_log/git
http://colabti.org/irclogger/irclogger_log/git-devel
There is a volunteer-run newsletter to serve our community ("Git Rev
News" http://git.github.io/rev_news/rev_news.html).
Git is a member project of software freedom conservancy, a non-profit
organization (https://sfconservancy.org/). To reach a committee of
liaisons to the conservancy, contact them at .
* Reporting bugs
When you think git does not behave as you expect, please do not stop
your bug report with just "git does not work". "I used git in this
way, but it did not work" is not much better, neither is "I used git
in this way, and X happend, which is broken". It often is that git is
correct to cause X happen in such a case, and it is your expectation
that is broken. People would not know what other result Y you expected
to see instead of X, if you left it unsaid.
Please remember to always state
- what you wanted to achieve;
- what you did (the version of git and the command sequence to reproduce
the behavior);
- what you saw happen (X above);
- what you expected to see (Y above); and
- how the last two are different.
See http://www.chiark.greenend.org.uk/~sgtatham/bugs.html for further
hints.
If you think you found a security-sensitive issue and want to disclose
it to us without announcing it to wider public, please contact us at
our security mailing list .
* Repositories and documentation.
My public git.git repositories are at:
git://git.kernel.org/pub/scm/git/git.git/
https://kernel.googlesource.com/pub/scm/git/git
git://repo.or.cz/alt-git.git/
https://github.com/git/git/
git://git.sourceforge.jp/gitroot/git-core/git.git/
git://git-core.git.sourceforge.net/gitroot/git-core/git-core/
A few web interfaces are found at:
http://git.kernel.org/cgit/git/git.git
https://kernel.googlesource.com/pub/scm/git/git
http://repo.or.cz/w/alt-git.git
Preformatted documentation from the tip of the "master" branch can be
found in:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
Also GitHub shows the manual pages formatted in HTML (with a
formatting backend different from the one that is used to create the
above) at:
http://git-scm.com/docs/git
* How various branches are used.
There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".
The "master" branch is meant to contain what are very well tested and
ready to be used in a production setting. Every now and then, a
"feature release" is cut from the tip of this branch. They used to be
named with three dotted decimal digits (e.g. "1.8.5"), but recently we
switched the versioning scheme and "feature releases" are named with
three-dotted decimal digits that ends with ".0" (e.g. "1.9.0").
The last such release was 2.8.0 done on Mar 28th, 2016. You can expect
that the tip of the "master"
[PATCH v2 01/11] rerere: split conflict ID further
The plan is to keep assigning the backward compatible conflict ID
based on the hash of the (normalized) text of conflicts, keep using
that conflict ID as the directory name under $GIT_DIR/rr-cache/, but
allow each conflicted path to use a separate "variant" to record
resolutions, i.e. having more than one pairs
under $GIT_DIR/rr-cache/$ID/ directory. As the first step in that
direction, separate the shared "conflict ID" out of the rerere_id
structure.
The plan is to keep information per $ID in rerere_dir, that can be
shared among rerere_id that is per conflicted path.
When we are done with rerere(), which can be directly called from
other programs like "git apply", "git commit" and "git merge", the
shared rerere_dir structures can be freed entirely, so they are not
reference-counted and they are not freed when we release rerere_id's
that reference them.
Signed-off-by: Junio C Hamano
---
* This is designed to be applied to a slightly newer codebase than
the previous round, and a few removed lines that used to call
strcpy() in the preimage shows up as using xsnprintf() in the
patch. We no longer use a simple string anyway, so the postimages
are the same between rerolls.
rerere.c | 61 -
rerere.h | 3 ++-
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/rerere.c b/rerere.c
index 3d0fa8f..a5d8a06 100644
--- a/rerere.c
+++ b/rerere.c
@@ -8,6 +8,7 @@
#include "ll-merge.h"
#include "attr.h"
#include "pathspec.h"
+#include "sha1-lookup.h"
#define RESOLVED 0
#define PUNTED 1
@@ -22,6 +23,23 @@ static int rerere_autoupdate;
static char *merge_rr_path;
+static int rerere_dir_nr;
+static int rerere_dir_alloc;
+
+static struct rerere_dir {
+ unsigned char sha1[20];
+} **rerere_dir;
+
+static void free_rerere_dirs(void)
+{
+ int i;
+ for (i = 0; i < rerere_dir_nr; i++)
+ free(rerere_dir[i]);
+ free(rerere_dir);
+ rerere_dir_nr = rerere_dir_alloc = 0;
+ rerere_dir = NULL;
+}
+
static void free_rerere_id(struct string_list_item *item)
{
free(item->util);
@@ -29,7 +47,7 @@ static void free_rerere_id(struct string_list_item *item)
static const char *rerere_id_hex(const struct rerere_id *id)
{
- return id->hex;
+ return sha1_to_hex(id->collection->sha1);
}
const char *rerere_path(const struct rerere_id *id, const char *file)
@@ -40,6 +58,37 @@ const char *rerere_path(const struct rerere_id *id, const
char *file)
return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
}
+static const unsigned char *rerere_dir_sha1(size_t i, void *table)
+{
+ struct rerere_dir **rr_dir = table;
+ return rr_dir[i]->sha1;
+}
+
+static struct rerere_dir *find_rerere_dir(const char *hex)
+{
+ unsigned char sha1[20];
+ struct rerere_dir *rr_dir;
+ int pos;
+
+ if (get_sha1_hex(hex, sha1))
+ return NULL; /* BUG */
+ pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr, rerere_dir_sha1);
+ if (pos < 0) {
+ rr_dir = xmalloc(sizeof(*rr_dir));
+ hashcpy(rr_dir->sha1, sha1);
+ pos = -1 - pos;
+
+ /* Make sure the array is big enough ... */
+ ALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);
+ /* ... and add it in. */
+ rerere_dir_nr++;
+ memmove(rerere_dir + pos + 1, rerere_dir + pos,
+ (rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
+ rerere_dir[pos] = rr_dir;
+ }
+ return rerere_dir[pos];
+}
+
static int has_rerere_resolution(const struct rerere_id *id)
{
struct stat st;
@@ -50,7 +99,7 @@ static int has_rerere_resolution(const struct rerere_id *id)
static struct rerere_id *new_rerere_id_hex(char *hex)
{
struct rerere_id *id = xmalloc(sizeof(*id));
- xsnprintf(id->hex, sizeof(id->hex), "%s", hex);
+ id->collection = find_rerere_dir(hex);
return id;
}
@@ -810,12 +859,14 @@ int setup_rerere(struct string_list *merge_rr, int flags)
int rerere(int flags)
{
struct string_list merge_rr = STRING_LIST_INIT_DUP;
- int fd;
+ int fd, status;
fd = setup_rerere(&merge_rr, flags);
if (fd < 0)
return 0;
- return do_plain_rerere(&merge_rr, fd);
+ status = do_plain_rerere(&merge_rr, fd);
+ free_rerere_dirs();
+ return status;
}
static int rerere_forget_one_path(const char *path, struct string_list *rr)
@@ -900,7 +951,7 @@ int rerere_forget(struct pathspec *pathspec)
static struct rerere_id *dirname_to_id(const char *name)
{
static struct rerere_id id;
- xsnprintf(id.hex, sizeof(id.hex), "%s", name);
+ id.collection = find_rerere_dir(name);
return &id;
}
diff --git a/rerere.h b/rerere.h
index ce545d0..faf5a23 100644
--- a/rerere.h
+++ b/rerere.h
@@ -15,8 +15,9 @@ struct pathspec;
*/
Re: pre-push hook does not get input on non-fast-forward pushes
On Mon, 28 Mar 2016 14:44:20 -0700 Junio C Hamano wrote: > Stefan Tauner writes: > > The pre-push hook is not the only thing that may prevent you from > pushing a ref update. As you noticed, non-fast-forward check may > trigger and decide that a ref is not going to be pushed, and that > may happen before we call the pre-push hook. > > Information about what is to be pushed is provided on the hook's > standard input with lines of the form ... > > So when the pre-push is called, the refs that will not fast-forward > may not be among the "what is to be pushed", hence not reported. > > We _could_ add something like this to the documentation, but we do > not necessarily want to promise that the order of checks to stay > "internal checks like non-ff check first before pre-push hook", so > this update may not be an improvement. The current text conveys > enough information without making such a promise, but you need to > read it carefully. I understand but that behavior is quite surprising and the wording not that clear (if one is not aware of the behavior already) IMHO. If the fast forward check is done beforehand and no refs remain, why is the script called at all? Or put otherwise, why isn't git aborting before that? That would seem way more logical... consistency is a good argument to call the hook with no refs anyway... but the abort/filter order is remarkable and should be documented in some way IMHO. > > Documentation/githooks.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index 9ef2469..605ba4d 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. > If a ref is to be > deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > -supplied as it was originally given. > +supplied as it was originally given. A request to update remote ref that has > +already been rejected for other reasons (e.g. failing to pass a fast-forward > +test) does not appear in the input. > > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent LGTM... if you don't want to promise anything then maybe something like "may not appear" instead of "does not appear" is better. As long as the reason for no input is stated more explicitly than currently I am happy. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "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: pre-push hook does not get input on non-fast-forward pushes
On Mon, Mar 28, 2016 at 2:44 PM, Junio C Hamano wrote: > Stefan Tauner writes: > >> I noticed that without an additional --force the pre-push hook does not >> get any input on stdin if a push would result in non-fast-forward >> uploads. This is not a problem per se (although I don't get the >> rationale) but it is undocumented and the latter left me puzzled. >> >> (Please keep me in CC since I am not subscribed, thanks) > > Thanks (no need to say "please cc me", as that is the norm here). > > The pre-push hook is not the only thing that may prevent you from > pushing a ref update. As you noticed, non-fast-forward check may > trigger and decide that a ref is not going to be pushed, and that > may happen before we call the pre-push hook. > > Information about what is to be pushed is provided on the hook's > standard input with lines of the form ... > > So when the pre-push is called, the refs that will not fast-forward > may not be among the "what is to be pushed", hence not reported. > > We _could_ add something like this to the documentation, but we do > not necessarily want to promise that the order of checks to stay > "internal checks like non-ff check first before pre-push hook", so > this update may not be an improvement. The current text conveys > enough information without making such a promise, but you need to > read it carefully. > > So I dunno. > > Documentation/githooks.txt | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index 9ef2469..605ba4d 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. > If a ref is to be > deleted, the `` will be supplied as `(delete)` and the ` SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > -supplied as it was originally given. > +supplied as it was originally given. A request to update remote ref that has > +already been rejected for other reasons (e.g. failing to pass a fast-forward > +test) does not appear in the input. If we don't want to give promises, s/does not/ may not/ ? Although that sounds like it is non-deterministic (some occur, some don't, maybe it depends on $random_reason) > > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pre-push hook does not get input on non-fast-forward pushes
Stefan Tauner writes: > I noticed that without an additional --force the pre-push hook does not > get any input on stdin if a push would result in non-fast-forward > uploads. This is not a problem per se (although I don't get the > rationale) but it is undocumented and the latter left me puzzled. > > (Please keep me in CC since I am not subscribed, thanks) Thanks (no need to say "please cc me", as that is the norm here). The pre-push hook is not the only thing that may prevent you from pushing a ref update. As you noticed, non-fast-forward check may trigger and decide that a ref is not going to be pushed, and that may happen before we call the pre-push hook. Information about what is to be pushed is provided on the hook's standard input with lines of the form ... So when the pre-push is called, the refs that will not fast-forward may not be among the "what is to be pushed", hence not reported. We _could_ add something like this to the documentation, but we do not necessarily want to promise that the order of checks to stay "internal checks like non-ff check first before pre-push hook", so this update may not be an improvement. The current text conveys enough information without making such a promise, but you need to read it carefully. So I dunno. Documentation/githooks.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 9ef2469..605ba4d 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -201,7 +201,9 @@ does not yet exist the `` will be 40 `0`. If a ref is to be deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be -supplied as it was originally given. +supplied as it was originally given. A request to update remote ref that has +already been rejected for other reasons (e.g. failing to pass a fast-forward +test) does not appear in the input. If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
pre-push hook does not get input on non-fast-forward pushes
Hi, I noticed that without an additional --force the pre-push hook does not get any input on stdin if a push would result in non-fast-forward uploads. This is not a problem per se (although I don't get the rationale) but it is undocumented and the latter left me puzzled. (Please keep me in CC since I am not subscribed, thanks) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- To unsubscribe from this list: send the line "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] t/perf: "make clean" from the top-level to clean results
On Mon, Mar 28, 2016 at 01:16:50PM -0700, Junio C Hamano wrote: > Running "make clean" from the top-level after running perf tests > left t/perf/test-results/ directory and tons of files in it. At > least "make distclean" should turn things back to pristine state. > > Signed-off-by: Junio C Hamano > --- > > * Perhaps I am missing some reason why this was deliberately left >out when we added t/perf/Makefile that does have the clean >target? Cc'ing the suspects found by "shortlog t/perf". I don't think I've ever touched the "clean" code path. This change is fine by me. I have noticed that the contents of t/perf/build can pile up and consume quite a lot of space, as they are all full builds of git. They're a little more expensive to reproduce than some other things, but they're inherently still a cache. I think your patch is doing the right thing. -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
[RFC/PATCH] t/perf: "make clean" from the top-level to clean results
Running "make clean" from the top-level after running perf tests left t/perf/test-results/ directory and tons of files in it. At least "make distclean" should turn things back to pristine state. Signed-off-by: Junio C Hamano --- * Perhaps I am missing some reason why this was deliberately left out when we added t/perf/Makefile that does have the clean target? Cc'ing the suspects found by "shortlog t/perf". t/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/t/Makefile b/t/Makefile index 18e2b28..e0aef9a 100644 --- a/t/Makefile +++ b/t/Makefile @@ -48,6 +48,7 @@ pre-clean: clean-except-prove-cache: $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)' $(RM) -r valgrind/bin + cd perf && $(MAKE) clean clean: clean-except-prove-cache $(RM) .prove -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] git rev-parse --git-common-dir executed from a sub dir of the main worktree is wrong
Mike Rappazzo writes: > I found a case where it seems that the result of `git rev-parse > --git-common-dir` is incorrect. If you execute the command from > within a subdirectory in the main worktree, it returns the path from > the root of the worktree to the current dir + "/.git". (As a > refresher, running this command from the root of the worktree returns > ".git"). > > I wrote a quick test to demonstrate the problem: > > > +test_expect_success 'git-common-dir inside sub-dir' ' > + ( > + mkdir -p path/to/child && > + cd path/to/child && > + echo "$(git rev-parse --show-toplevel)/.git" >expected && ... or "$(git rev-parse --show-cdup).git >expect i.e. to use relative path, just like the case where you start from the top. > + git rev-parse --git-common-dir >actual && > + test_cmp expected actual > + ) > +' > + > > I suggest that we change the result of this call to _always_ return an > absolute path. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug] git rev-parse --git-common-dir executed from a sub dir of the main worktree is wrong
I found a case where it seems that the result of `git rev-parse --git-common-dir` is incorrect. If you execute the command from within a subdirectory in the main worktree, it returns the path from the root of the worktree to the current dir + "/.git". (As a refresher, running this command from the root of the worktree returns ".git"). I wrote a quick test to demonstrate the problem: +test_expect_success 'git-common-dir inside sub-dir' ' + ( + mkdir -p path/to/child && + cd path/to/child && + echo "$(git rev-parse --show-toplevel)/.git" >expected && + git rev-parse --git-common-dir >actual && + test_cmp expected actual + ) +' + I suggest that we change the result of this call to _always_ return an absolute path. I would be willing to code this change, but I didn't want to start anything that may be considered backwards-incompatible. This seems related to [1]http://thread.gmane.org/gmane.comp.version-control.git/286038 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git show -m with a parent number
Jeff King writes: > On Sun, Mar 27, 2016 at 08:46:02PM +, Anatoly Borodin wrote: > >> is there a good reason for `git show -m` to not accept the number of a >> parent of a merge commit? I can run `git show --first-parent COMMIT`, >> but need to write `git diff COMMIT^2 COMMIT` every time I want to diff >> with the second parent! > > I think it could, but nobody has yet found it useful enough to > implement. For "git show" that does not traverse the history, there may be occasions where showing a diff with the second parent might make sense (e.g. the user merged two histories in a wrong direction in the past and it is too late to rewrite). I think there are two reasons behind the current state. Because it does not make any sense for "git log -m 2" to show diff with its second parent while traversing many commits, the "git log" codepath is not prepared to show diff with only nth parent. Because "git show" started its life merely as a thin wrapper around "git log", "git show" does not use such an underlying machinery to show diff with only nth parent that does not exist. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git show -m with a parent number
On Sun, Mar 27, 2016 at 08:46:02PM +, Anatoly Borodin wrote: > is there a good reason for `git show -m` to not accept the number of a > parent of a merge commit? I can run `git show --first-parent COMMIT`, > but need to write `git diff COMMIT^2 COMMIT` every time I want to diff > with the second parent! I think it could, but nobody has yet found it useful enough to implement. I am not sure of the workflow it would help. Personally, I use "-m" in two situations: 1. To puzzle out issues where "-c" or "--cc" do not show what I expected. 2. With "--first-parent", to follow the linear history of a mainline branch, showing merged topic branches as a single diff. For the first one, showing all diffs is what I want. For the second, it only makes sense to for the first parent case, as following other parents would zig-zag through history. But perhaps you have some other use case in mind. In cases like these, I think it is a good idea to implement the feature, and run with it for a while, seeing how it can be used. And then if it proves useful, post the patch to the list describing your experiences. -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 v9 1/3] parse-options.c: make OPTION__COUNTUP consider negative values
On Mon, Mar 28, 2016 at 2:42 PM, Pranit Bauva wrote: >> A couple of new tests to t0040-parse-options.sh would be great to >> ensure that starting from a negative value works as advertised, i.e. >> at least that '--option' jumps to 1 and '--no-option' resets to 0. > > I think adding tests to t0040-parse-options.sh cannot reflect the > behavior introduced by this patch. > This is because setting the initial value of the variable (which is > going to be modified by the argument) is set in test-parse-options.c . > A possible solution will be to modify the test-parse-options.c and > initialize the required variable (verbose or quiet) to "unspecified" > value. But then this will leave one part of the untested ie. when the > initial value of the variable is 0. I could do one thing to test these > both behaviors which is to set verbose initially to -1 and leave quiet > = 0. Since verbose and quiet are both consumers of OPT_COUNTUP, this > can test both the behaviors. > > I tried searching for alternatives but could not find any. Is there > something else which you had thought before that would test this > behavior? > > If not, then we are left with 2 options, either modify > test-parse-options.c or just hand test it whenever there seems to be a > problematic case. Modifying test-parse-options.c, if needed, was implied by SZEDER's suggestion to add tests for this new behavior. test-parse-options.c exists strictly for testing option parsing, so don't feel constrained about modifying or extending it in order to test the new count-up behavior if the existing implementation doesn't directly support what you want it to do. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] rebase-i: print an abbreviated hash when stop for editing
Ralf Thielow writes:
> The message that is shown when rebase-i stops for editing prints
> the full hash of the commit where it stopped which makes the message
> overflow to the next line on smaller terminal windows. Print an
> abbreviated hash instead.
>
> Signed-off-by: Ralf Thielow
> ---
Thanks, will queue.
> git-rebase--interactive.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 4cde685..9ea3075 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -548,7 +548,8 @@ do_next () {
>
> mark_action_done
> do_pick $sha1 "$rest"
> - warn "Stopped at $sha1... $rest"
> + sha1_abbrev=$(git rev-parse --short $sha1)
> + warn "Stopped at $sha1_abbrev... $rest"
> exit_with_patch $sha1 0
> ;;
> squash|s|fixup|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 v9 1/3] parse-options.c: make OPTION__COUNTUP consider negative values
> A couple of new tests to t0040-parse-options.sh would be great to > ensure that starting from a negative value works as advertised, i.e. > at least that '--option' jumps to 1 and '--no-option' resets to 0. I think adding tests to t0040-parse-options.sh cannot reflect the behavior introduced by this patch. This is because setting the initial value of the variable (which is going to be modified by the argument) is set in test-parse-options.c . A possible solution will be to modify the test-parse-options.c and initialize the required variable (verbose or quiet) to "unspecified" value. But then this will leave one part of the untested ie. when the initial value of the variable is 0. I could do one thing to test these both behaviors which is to set verbose initially to -1 and leave quiet = 0. Since verbose and quiet are both consumers of OPT_COUNTUP, this can test both the behaviors. I tried searching for alternatives but could not find any. Is there something else which you had thought before that would test this behavior? If not, then we are left with 2 options, either modify test-parse-options.c or just hand test it whenever there seems to be a problematic case. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
Hui Yiqun writes:
> According to strbuf.h, strbuf_detach is the sole supported method
> to unwrap a memory buffer from its strbuf shell.
> ...
> diff --git a/path.c b/path.c
> index 969b494..9801617 100644
> --- a/path.c
> +++ b/path.c
> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
> {
> static struct strbuf validated_path = STRBUF_INIT;
> static struct strbuf used_path = STRBUF_INIT;
> ...
> +return_null:
> + free(dbuf);
> + strbuf_release(&used_path);
> + strbuf_release(&validated_path);
> return NULL;
> }
I see these strbuf's are "static" storage class, so that they do not
have to get freed.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
On Mon, Mar 28, 2016 at 11:56:10PM +0800, Hui Yiqun wrote:
> According to strbuf.h, strbuf_detach is the sole supported method
> to unwrap a memory buffer from its strbuf shell.
>
> So we should not return the pointer of strbuf.buf directly.
>
> What's more, some memory leakages are solved.
There is something else going on here, which makes this case different
than some others. Note that the function returns a const string:
> diff --git a/path.c b/path.c
> index 969b494..b07e5a7 100644
> --- a/path.c
> +++ b/path.c
> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
By convention, that means that the result is not owned by the caller,
and should not be freed. We implement that by:
> {
> static struct strbuf validated_path = STRBUF_INIT;
> static struct strbuf used_path = STRBUF_INIT;
...using static variables which persist after the call returns. So this
function retains ownership of the memory, and it remains valid until the
next call to enter_repo().
There's no leak when we return NULL, because the function retains
control of the memory (though it will hold onto it until the end of the
program if nobody calls enter_repo() again). And thus we shouldn't use
strbuf_detach(), which loses that reference to the memory (and since the
callers don't take ownership, it actually _creates_ a leak).
We could release the memory when returning, but I don't think it's a big
deal to do so.
-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 v3 1/2] refs: add a new function set_worktree_head_symref
Kazuki Yamaguchi writes:
> Add a new function set_worktree_head_symref, to update HEAD symref for
> the specified worktree.
>
> To update HEAD of a linked working tree,
> create_symref("worktrees/$work_tree/HEAD", "refs/heads/$branch", msg)
> could be used. However when it comes to updating HEAD of the main
> working tree, it is unusable because it uses $GIT_DIR for
> worktree-specific symrefs (HEAD).
>
> The new function takes git_dir (real directory) as an argument, and
> updates HEAD of the working tree. This function will be used when
> renaming a branch.
>
> Signed-off-by: Kazuki Yamaguchi
> ---
I suspect that this helper function should be in the common layer
(refs.c) to be redirected to specific backend(s).
David & Michael, what do you think?
> refs.h | 8
> refs/files-backend.c | 35 +++
> 2 files changed, 43 insertions(+)
>
> diff --git a/refs.h b/refs.h
> index 2f3decb432cf..f11154cf5faf 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -306,6 +306,14 @@ extern int rename_ref(const char *oldref, const char
> *newref, const char *logmsg
>
> extern int create_symref(const char *refname, const char *target, const char
> *logmsg);
>
> +/*
> + * Update HEAD of the specified gitdir.
> + * Similar to create_symref("relative-git-dir/HEAD", target, NULL), but this
> is
> + * able to update the main working tree's HEAD wherever $GIT_DIR points to.
> + * Return 0 if successful, non-zero otherwise.
> + * */
> +extern int set_worktree_head_symref(const char *gitdir, const char *target);
> +
> enum action_on_err {
> UPDATE_REFS_MSG_ON_ERR,
> UPDATE_REFS_DIE_ON_ERR,
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 81f68f846b69..ec237efec35d 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2894,6 +2894,41 @@ int create_symref(const char *refname, const char
> *target, const char *logmsg)
> return ret;
> }
>
> +int set_worktree_head_symref(const char *gitdir, const char *target)
> +{
> + static struct lock_file head_lock;
> + struct ref_lock *lock;
> + struct strbuf err = STRBUF_INIT;
> + struct strbuf head_path = STRBUF_INIT;
> + const char *head_rel;
> + int ret;
> +
> + strbuf_addf(&head_path, "%s/HEAD", absolute_path(gitdir));
> + if (hold_lock_file_for_update(&head_lock, head_path.buf,
> + LOCK_NO_DEREF) < 0) {
> + error("%s", err.buf);
> + strbuf_release(&err);
> + strbuf_release(&head_path);
> + return -1;
> + }
> +
> + /* head_rel will be "HEAD" for the main tree, "worktrees/wt/HEAD" for
> +linked trees */
> + head_rel = remove_leading_path(head_path.buf,
> +absolute_path(get_git_common_dir()));
> + /* to make use of create_symref_locked(), initialize ref_lock */
> + lock = xcalloc(1, sizeof(struct ref_lock));
> + lock->lk = &head_lock;
> + lock->ref_name = xstrdup(head_rel);
> + lock->orig_ref_name = xstrdup(head_rel);
> +
> + ret = create_symref_locked(lock, head_rel, target, NULL);
> +
> + unlock_ref(lock); /* will free lock */
> + strbuf_release(&head_path);
> + return ret;
> +}
> +
> int reflog_exists(const char *refname)
> {
> struct stat st;
--
To unsubscribe from this list: send the line "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] worktree: add: introduce --checkout option
Ray Zhang writes: > By adding this option which defaults to true, we can use the > corresponding --no-checkout to make some customizations before > the checkout, like sparse checkout, etc. > > Helped-by: Eric Sunshine > Helped-by: Junio C Hamano > Reviewed-by: Eric Sunshine > Signed-off-by: Ray Zhang > --- Thanks. Will queue. > diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh > index cbfa41e..472b811 100755 > --- a/t/t2025-worktree-add.sh > +++ b/t/t2025-worktree-add.sh > @@ -213,4 +213,17 @@ test_expect_success 'local clone from linked checkout' ' > ( cd here-clone && git fsck ) > ' > > +test_expect_success '"add" worktree with --no-checkout' ' > + git worktree add --no-checkout -b swamp swamp && > + ls swamp >actual && > + test_line_count = 0 actual && The remainder of the test (both existing and added parts) seems to only care about init.t; running "ls swamp" feels somewhat inconsistent. We could replace the two lines with ! test -e swamp/init.t to address this, but I do not think it is worth a patch churn. > + git -C swamp reset --hard && > + test_cmp init.t swamp/init.t > +' > +test_expect_success '"add" worktree with --checkout' ' > + git worktree add --checkout -b swmap2 swamp2 && > + test_cmp init.t swamp2/init.t > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3/GSoC 2/5] path.c: implement xdg_runtime_dir()
惠轶群 writes:
> For example:
>
> const char *enter_repo(const char *path, int strict)
> {
> static struct strbuf validated_path = STRBUF_INIT;
> static struct strbuf used_path = STRBUF_INIT;
>
> if (!path)
> return NULL; // no need to release, right?
> ...
> }
Correct.
--
To unsubscribe from this list: send the line "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: [PATCHv3 5/5] submodule--helper clone: lose the extra prefix option
Junio C Hamano writes: > I am talking about a case where > > cd repo > cd untracked > git submodule --recurse-submodules --read-from=file > > wants to run , using information stored in repo/untracked/file, > and work on submodules repo/sub and repo/sub/subsub. The reference > to the original location somehow needs to be carried through if we > wanted to allow this kind of thing. Let us step back a bit and try a simplified scenario to help us think "prefix" through from the first principle. Imagine there is a project P like this: /leading/path/Q/P/.git/ /leading/path/Q/P/one/file /leading/path/Q/P/one/msg and we do cd /leading/path/Q/P/one edit file git commit -F msg file This example illustrates that there are two quite different uses of "prefix". It is used to serve as a name of the path that is used in the history of the project (i.e. "file" is prefixed with "one/" to form "one/file" that is the full path recorded in the history). It also is used to name an entity on the filesystem that does not have anything to do with any path recorded in the history (i.e. the message is read from "one/msg", even though the argument given to "-F" is "msg"). This distinction may be hard to realize until we start doing some unusual thing. Suppose there is a directory Q/R just next to P and we did this: cd /leading/path/Q/R cp ../P/one/file ../P/one/msg . edit file msg GIT_WORK_TREE=/leading/path/Q/P GIT_DIR=$GIT_WORK_TREE/.git export GIT_WORK_TREE GIT_DIR git commit -F msg file What should happen (we are trying to think things through from the first principle, so this is not asking about the implemented behaviour)? As this "file" refers to /leading/path/Q/R/file that is outside of the working tree of the project, the reference to it by "git commit" does not make any sense. The reference to "msg", however, does make sense--there is no reason why the contents of the message read by "-F" option have to be inside the working tree. So it probably is sensible to fail this command. It is the same story if you replace "file" with ".", i.e. "git commit -F msg .". The reference to "current directory" the latter makes to name set of paths to be recorded in the history, made outside the working tree, makes the command a nonsense. Side note: I suspect that the current implementation of "commit" actually does wrong things to these two uses of the prefix in that (1) it would not find "msg" and (2) it would happily use an empty string as the "prefix" without failing. The former is something we may want to fix, but I suspect this is not a low-hanging fruit. The latter is taking the only sensible fallback position and I do not feel a strong need to change it to error out. Without "file" parameter, however, it may make sense to allow making the commit. After all, the command is not talking about any paths at the point, and telling us to work on the entire working tree. Up to this point, the scenario is simplified by not having any submodule. Now, imagine the case where Q is the top-level superproject of P, i.e. /leading/path/Q/.git/ /leading/path/Q/P/.git/ /leading/path/Q/P/one/file /leading/path/Q/P/one/msg /leading/path/Q/R/file /leading/path/Q/R/msg When "submodule --recurse-submodules" does the work of in a submodule, as far as the submodule repository that gets recursed into (i.e. P) is concerned, the situation is exactly like the above example: a command that is meant to work on project P is launched from ../R that is outside its working tree. So a parameter like "--read-from=file" in the original example may make sense, while taking any pathspec to be applied inside the visited submodules does not, and instead we should declare that the work of for each visited submodule is to be done on the entire working tree of the submodule. In other words, the correct value of "prefix" for its two different uses are different. The "prefix" for the paths recorded in the history (i.e. those that correspond to "file" in the above example) has to be an empty string to denote that the work of applies to the entire tree, while the "prefix" for the filesystem entities that does not have anything to do with the paths in the recorded history (i.e. those that correspond to "msg" in the above example) would be pointing outside the working tree (e.g. "../R"). Now, "git submodule" uses $wt_prefix to grab the return value of "git rev-parse --show-prefix" upfront, and it _is_ possible to use the variable to refer to the "file" in the original location in "git submodule --recurse-submodules --read-from=file" if we wanted to, if its mechanism is taught to correctly maintain the variable relative to the root of the working tree of submodules as it visits them. But even if we did so, we shouldn'
Re: [PATCH v2] branch -d: refuse deleting a branch which is currently checked out
On Mon, Mar 28, 2016 at 3:22 AM, Kazuki Yamaguchi wrote:
> When a branch is checked out by current working tree, deleting the
> branch is forbidden. However when the branch is checked out only by
> other working trees, deleting is allowed.
It's not quite clear from this description that it is bad for deletion
to succeed in the second case. Perhaps:
s/deleting is allowed/deletion incorrectly succeeds/
would make it more clear.
> Use find_shared_symref() to check if the branch is in use, not just
> comparing with the current working tree's HEAD.
This version of the patch is nicer. Thanks. See a couple minor
comments below which may or may not be worth a re-roll (you decide).
> Signed-off-by: Kazuki Yamaguchi
> ---
>
> % git worktree list
> /path/to 2c3c5f2 [master]
> /path/to/wt 2c3c5f2 [branch-a]
> % git branch -d branch-a
> error: Cannot delete the branch 'branch-a' which is currently checked out
> at '/path/to/wt'
Thanks for an example of the new behavior. It's also helpful to
reviewers if you use this space to explain what changed since the
previous version, and to provide a link to the previous attempt, like
this[1].
[1]: http://thread.gmane.org/gmane.comp.version-control.git/289413/focus=289932
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -215,16 +216,21 @@ static int delete_branches(int argc, const char **argv,
> int force, int kinds,
> int flags = 0;
>
> strbuf_branchname(&bname, argv[i]);
> - if (kinds == FILTER_REFS_BRANCHES && !strcmp(head,
> bname.buf)) {
> - error(_("Cannot delete the branch '%s' "
> - "which you are currently on."), bname.buf);
> - ret = 1;
> - continue;
> - }
> -
> free(name);
> -
> name = mkpathdup(fmt, bname.buf);
> +
> + if (kinds == FILTER_REFS_BRANCHES) {
> + char *worktree = find_shared_symref("HEAD", name);
> + if (worktree) {
> + error(_("Cannot delete the branch '%s' "
> + "which is currently checked out at
> '%s'"),
This could be stated more concisely as:
"Cannot delete branch '%s' checked out at '%s'"
> + bname.buf, worktree);
> + free(worktree);
Would it make sense to show all worktrees at which this branch is
checked out, rather than only one, or is that not worth the effort and
extra code ugliness?
> + ret = 1;
> + continue;
> + }
> + }
> +
> target = resolve_ref_unsafe(name,
> RESOLVE_REF_READING
> | RESOLVE_REF_NO_RECURSE
--
To unsubscribe from this list: send the line "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: [GSoC] A late proposal: a modern send-email
On Sat, Mar 26, 2016 at 3:13 AM, 惠轶群 wrote: > 2016-03-26 2:16 GMT+08:00 Junio C Hamano : >> 惠轶群 writes: >> >>> # Purpose >>> The current implementation of send-email is based on perl and has only >>> a tui, it has two problems: >>> - user must install a ton of dependencies before submit a single patch. >>> - tui and parameter are both not quite friendly to new users. >> >> Is "a ton of dependencies" true? "apt-cache show git-email" >> suggests otherwise. Is "a ton of dependencies" truly a problem? >> "apt-get install" would resolve the dependencies for you. > > There are three perl packages needed to send patch through gmail: > - perl-mime-tools > - perl-net-smtp-ssl > - perl-authen-sasl > > Yes, not too many, but is it better none of them? > > What's more, when I try to send mails, I was first disrupted by > "no perl-mime-tools" then by "no perl-net-smtp-ssl or perl-authen-sasl". > Then I think, why not just a mailto link? I think your proposal should clarify a bit who these users are that find it too difficult to install these perl module dependencies. Users on OSX & Windows I would assume, because in the case of Linux distros getting these is the equivalent of an apt-get command away. If installing these dependencies is hard for users perhaps a better thing to focus on is altering the binary builds on Git for platforms that don't have package systems to include these dependencies. In this case it would mean shipping a statically linked OpenSSL since that's what these perl SSL packages eventually depend on. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] rebase-i: print an abbreviated hash when stop for editing
The message that is shown when rebase-i stops for editing prints
the full hash of the commit where it stopped which makes the message
overflow to the next line on smaller terminal windows. Print an
abbreviated hash instead.
Signed-off-by: Ralf Thielow
---
git-rebase--interactive.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4cde685..9ea3075 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -548,7 +548,8 @@ do_next () {
mark_action_done
do_pick $sha1 "$rest"
- warn "Stopped at $sha1... $rest"
+ sha1_abbrev=$(git rev-parse --short $sha1)
+ warn "Stopped at $sha1_abbrev... $rest"
exit_with_patch $sha1 0
;;
squash|s|fixup|f)
--
2.8.0.rc4.285.gc3ac548
--
To unsubscribe from this list: send the line "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] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
Sorry, I sent the patch repeatedly to fix a wrongly indent with space.
2016-03-28 23:57 GMT+08:00 Hui Yiqun :
> According to strbuf.h, strbuf_detach is the sole supported method
> to unwrap a memory buffer from its strbuf shell.
>
> So we should not return the pointer of strbuf.buf directly.
>
> What's more, some memory leakages are solved.
> ---
> path.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/path.c b/path.c
> index 969b494..9801617 100644
> --- a/path.c
> +++ b/path.c
> @@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
> {
> static struct strbuf validated_path = STRBUF_INIT;
> static struct strbuf used_path = STRBUF_INIT;
> + char * dbuf = NULL;
>
> if (!path)
> return NULL;
> @@ -654,7 +655,7 @@ const char *enter_repo(const char *path, int strict)
> if (used_path.buf[0] == '~') {
> char *newpath = expand_user_path(used_path.buf);
> if (!newpath)
> - return NULL;
> + goto return_null;
> strbuf_attach(&used_path, newpath, strlen(newpath),
> strlen(newpath));
> }
> @@ -671,22 +672,22 @@ const char *enter_repo(const char *path, int strict)
> strbuf_setlen(&used_path, baselen);
> }
> if (!suffix[i])
> - return NULL;
> + goto return_null;
> gitfile = read_gitfile(used_path.buf);
> if (gitfile) {
> strbuf_reset(&used_path);
> strbuf_addstr(&used_path, gitfile);
> }
> if (chdir(used_path.buf))
> - return NULL;
> - path = validated_path.buf;
> + goto return_null;
> + path = dbuf = strbuf_detach(&validated_path, NULL);
> }
> else {
> const char *gitfile = read_gitfile(path);
> if (gitfile)
> path = gitfile;
> if (chdir(path))
> - return NULL;
> + goto return_null;
> }
>
> if (is_git_directory(".")) {
> @@ -695,6 +696,10 @@ const char *enter_repo(const char *path, int strict)
> return path;
> }
>
> +return_null:
> + free(dbuf);
> + strbuf_release(&used_path);
> + strbuf_release(&validated_path);
> return NULL;
> }
>
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
According to strbuf.h, strbuf_detach is the sole supported method
to unwrap a memory buffer from its strbuf shell.
So we should not return the pointer of strbuf.buf directly.
What's more, some memory leakages are solved.
---
path.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/path.c b/path.c
index 969b494..9801617 100644
--- a/path.c
+++ b/path.c
@@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
{
static struct strbuf validated_path = STRBUF_INIT;
static struct strbuf used_path = STRBUF_INIT;
+ char * dbuf = NULL;
if (!path)
return NULL;
@@ -654,7 +655,7 @@ const char *enter_repo(const char *path, int strict)
if (used_path.buf[0] == '~') {
char *newpath = expand_user_path(used_path.buf);
if (!newpath)
- return NULL;
+ goto return_null;
strbuf_attach(&used_path, newpath, strlen(newpath),
strlen(newpath));
}
@@ -671,22 +672,22 @@ const char *enter_repo(const char *path, int strict)
strbuf_setlen(&used_path, baselen);
}
if (!suffix[i])
- return NULL;
+ goto return_null;
gitfile = read_gitfile(used_path.buf);
if (gitfile) {
strbuf_reset(&used_path);
strbuf_addstr(&used_path, gitfile);
}
if (chdir(used_path.buf))
- return NULL;
- path = validated_path.buf;
+ goto return_null;
+ path = dbuf = strbuf_detach(&validated_path, NULL);
}
else {
const char *gitfile = read_gitfile(path);
if (gitfile)
path = gitfile;
if (chdir(path))
- return NULL;
+ goto return_null;
}
if (is_git_directory(".")) {
@@ -695,6 +696,10 @@ const char *enter_repo(const char *path, int strict)
return path;
}
+return_null:
+ free(dbuf);
+ strbuf_release(&used_path);
+ strbuf_release(&validated_path);
return NULL;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
According to strbuf.h, strbuf_detach is the sole supported method
to unwrap a memory buffer from its strbuf shell.
So we should not return the pointer of strbuf.buf directly.
What's more, some memory leakages are solved.
---
path.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/path.c b/path.c
index 969b494..b07e5a7 100644
--- a/path.c
+++ b/path.c
@@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
{
static struct strbuf validated_path = STRBUF_INIT;
static struct strbuf used_path = STRBUF_INIT;
+ char * dbuf = NULL;
if (!path)
return NULL;
@@ -654,7 +655,7 @@ const char *enter_repo(const char *path, int strict)
if (used_path.buf[0] == '~') {
char *newpath = expand_user_path(used_path.buf);
if (!newpath)
- return NULL;
+ goto return_null;
strbuf_attach(&used_path, newpath, strlen(newpath),
strlen(newpath));
}
@@ -671,22 +672,22 @@ const char *enter_repo(const char *path, int strict)
strbuf_setlen(&used_path, baselen);
}
if (!suffix[i])
- return NULL;
+ goto return_null;
gitfile = read_gitfile(used_path.buf);
if (gitfile) {
strbuf_reset(&used_path);
strbuf_addstr(&used_path, gitfile);
}
if (chdir(used_path.buf))
- return NULL;
- path = validated_path.buf;
+ goto return_null;
+ path = dbuf = strbuf_detach(&validated_path, NULL);
}
else {
const char *gitfile = read_gitfile(path);
if (gitfile)
path = gitfile;
if (chdir(path))
- return NULL;
+ goto return_null;
}
if (is_git_directory(".")) {
@@ -695,6 +696,10 @@ const char *enter_repo(const char *path, int strict)
return path;
}
+return_null:
+free(dbuf);
+ strbuf_release(&used_path);
+ strbuf_release(&validated_path);
return NULL;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] path.c enter_repo(): fix unproper strbuf unwrapping and memory leakage
According to strbuf.h, strbuf_detach is the sole supported method
to unwrap a memory buffer from its strbuf shell.
So we should not return the pointer of strbuf.buf directly.
What's more, some memory leakages are solved.
---
path.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/path.c b/path.c
index 969b494..b07e5a7 100644
--- a/path.c
+++ b/path.c
@@ -625,6 +625,7 @@ const char *enter_repo(const char *path, int strict)
{
static struct strbuf validated_path = STRBUF_INIT;
static struct strbuf used_path = STRBUF_INIT;
+ char * dbuf = NULL;
if (!path)
return NULL;
@@ -654,7 +655,7 @@ const char *enter_repo(const char *path, int strict)
if (used_path.buf[0] == '~') {
char *newpath = expand_user_path(used_path.buf);
if (!newpath)
- return NULL;
+ goto return_null;
strbuf_attach(&used_path, newpath, strlen(newpath),
strlen(newpath));
}
@@ -671,22 +672,22 @@ const char *enter_repo(const char *path, int strict)
strbuf_setlen(&used_path, baselen);
}
if (!suffix[i])
- return NULL;
+ goto return_null;
gitfile = read_gitfile(used_path.buf);
if (gitfile) {
strbuf_reset(&used_path);
strbuf_addstr(&used_path, gitfile);
}
if (chdir(used_path.buf))
- return NULL;
- path = validated_path.buf;
+ goto return_null;
+ path = dbuf = strbuf_detach(&validated_path, NULL);
}
else {
const char *gitfile = read_gitfile(path);
if (gitfile)
path = gitfile;
if (chdir(path))
- return NULL;
+ goto return_null;
}
if (is_git_directory(".")) {
@@ -695,6 +696,10 @@ const char *enter_repo(const char *path, int strict)
return path;
}
+return_null:
+free(dbuf);
+ strbuf_release(&used_path);
+ strbuf_release(&validated_path);
return NULL;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
Hi Hannes, On Mon, 28 Mar 2016, Johannes Sixt wrote: > A change like this whould have been preferable: > [...] The problem with your patch is that it does not account for backslashes in paths resulting in quoting. I am afraid that your patch will most likely *not* let the tests pass in Git for Windows SDK, while my patch does. Ciao, Dscho -- To unsubscribe from this list: send the line "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/GSoC 2/5] path.c: implement xdg_runtime_dir()
2016-03-28 22:50 GMT+08:00 Junio C Hamano :
> 惠轶群 writes:
>
>> After read the source code of strbuf more carefully, I get the conclusion
>> that if a strbuf is initialized with STRBUF_INIT but is not used, there is
>> no need to release it. Is it true?
>
> If it is initialized with STRBUF_INIT and never used, there is no
> reason for the variable to exist ;-)
I mean, if some developer return before the strbuf is used, there seems
no need to release the strbuf according to the current implementation.
But I'm not sure whether this is suitable for the abstraction of strbuf.
For example:
const char *enter_repo(const char *path, int strict)
{
static struct strbuf validated_path = STRBUF_INIT;
static struct strbuf used_path = STRBUF_INIT;
if (!path)
return NULL; // no need to release, right?
...
}
> Leaving the variable in the code, and not calling release on it at
> the end, would be OK (i.e. there is no leak) today, but may invite
> future bugs (e.g. people may use the variable to tentatively build
> their string before the function returns to leave the scope of the
> variable without adding _release() themselves).
--
To unsubscribe from this list: send the line "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: How to use @{-1} with push?
On Fri, Mar 25, 2016 at 1:58 PM, Junio C Hamano wrote:
> I thought these are clear from their documentation. "push" works on
> refnames, "branch" works on branch names. "push" takes an branch
> name as a short-hand and adds refs/heads/ when it makes sense, but
> because it does not make any sense for "git branch" to create a
> "branch" in a random place in refs/ (e.g. "refs/tags/foo" is not a
> branch), it takes "foo" (i.e. the name of the branch, whose
> underlying ref is "refs/heads/foo").
>
> So
>
> ref=$(git rev-parse --symbolic-full-name "$2") &&
> case "$ref" in
> '') echo >&2 "No such thing $2"; exit 1 ;;
> refs/heads/*) ref=${ref#refs/heads/} ;;
> *) echo >&2 "That's not a branch $2"; exit 2 ;;
> esac &&
> git push "$1" "refs/heads/$ref" &&
> git branch -D "$ref"
>
> or something?
Reading the git documentation feels a lot like reading the C++
standard. Not the best place to go to learn something. Some of the
terminology is very detail-oriented. For example, until you explained
the differences between push & branch, I always thought "ref" and
"branch" were interchangeable. But now it's clear to me that a branch
is just a type of ref, but refs are not branches. Also as a Windows
developer, I am not as well-versed in bash scripting as I'd like to
be.
So thanks for your explanation, it is clear now.
--
To unsubscribe from this list: send the line "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/GSoC 2/5] path.c: implement xdg_runtime_dir()
惠轶群 writes: > After read the source code of strbuf more carefully, I get the conclusion > that if a strbuf is initialized with STRBUF_INIT but is not used, there is > no need to release it. Is it true? If it is initialized with STRBUF_INIT and never used, there is no reason for the variable to exist ;-) Leaving the variable in the code, and not calling release on it at the end, would be OK (i.e. there is no leak) today, but may invite future bugs (e.g. people may use the variable to tentatively build their string before the function returns to leave the scope of the variable without adding _release() themselves). -- To unsubscribe from this list: send the line "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/GSoC 2/5] path.c: implement xdg_runtime_dir()
惠轶群 writes: > Excuse me, but I could not find > `Documentation/technical/api-strbuf.txt` in master branch. Do you > refer to the header of `strbuf.h`? In which, I learnt how to > initialize the strbuf and how to take use of it when I began to > use it. Ah, yes, it was a relatively recent development that usage text from the former was moved to the latter. > If there is also a note about whether I should release it > and how do it, such as: > > For every strbuf that has been initialized and buffer of it has > not been detached with strbuf_detach, you should release the > resource by strbuf_release. > > It will save me (maybe others) much time to explore the entire method list. While it may not hurt, it sounds somewhat extreme and funny me, as it sounds as if you are saying "for every pointer variable that points at a piece of memory allocated with malloc(3) and friends, you must free(3) them after you are done". The way _release() refers to its internal resource usage could be improved, I guess. "Release ... the memory it used" may not click as freeing the memory to some readers. strbuf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strbuf.h b/strbuf.h index f72fd14..a783f09 100644 --- a/strbuf.h +++ b/strbuf.h @@ -82,7 +82,7 @@ extern char strbuf_slopbuf[]; extern void strbuf_init(struct strbuf *, size_t); /** - * Release a string buffer and the memory it used. You should not use the + * Release a string buffer and free the memory it used. You should not use the * string buffer after using this function, unless you initialize it again. */ extern void strbuf_release(struct strbuf *); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git difftool help
'git diff 'is opening in meld. I could not create a patch using - git diff > ./patch-01 i did not make any change to pick meld, by default it is picking meld. Which "git difftool" will help to create patch ? i do want to use format patch as changes are not yet committed ? -Ratheesh -- To unsubscribe from this list: send the line "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/GSoC 2/5] path.c: implement xdg_runtime_dir()
2016-03-26 1:59 GMT+08:00 Jeff King : > On Fri, Mar 25, 2016 at 10:21:48PM +0800, 惠轶群 wrote: > >> > There are some minor English problems here (and elsewhere). E.g., you >> > probably want "So we just issue a warning and leave it to the user to >> > solve.". >> >> Sorry for my English. > > Thanks. And sorry if that sounded too harsh. I know that working in a > non-native language is tough. Usually in a review I'll try to provide > specific English fixes, but in this case, I think a lot of these > messages are still in flux, so I I didn't want to waste either of our > time going over specifics if the content is just going to change later. > >> > These ones leak, too. >> >> I will deal with it. >> >> I find there are some similar leakage in this file. I'll fix them in >> another patch. > > Great, thanks. After read the source code of strbuf more carefully, I get the conclusion that if a strbuf is initialized with STRBUF_INIT but is not used, there is no need to release it. Is it true? > -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
[no subject]
subscribe -- To unsubscribe from this list: send the line "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/GSoC 2/5] path.c: implement xdg_runtime_dir()
2016-03-26 0:55 GMT+08:00 Junio C Hamano : > 惠轶群 writes: > >>> There's a lot of "what" here that the caller doesn't really care about, >>> and which may go stale with respect to the implementation over time. Can >>> we make something more succinct like: >>> >>> /* >>>* Return a path suitable for writing run-time files related to git, >>>* or NULL if no such path can be established. The resulting string >>>* should be freed by the caller. >>>*/ >>> >>> ? >> >> That's clearer, but if I were the caller, I would worry about the >> security of the path. >> How about adding: >> >> The security of the path is ensured by file permission. > > Is "by file permission" descriptive enough? > > To protect /a/b/c/socket, what filesystem entities have the right > permission bits set? If the parent directory is writable by an > attacker, the permission bits on 'socket' itself may not matter as > the attacker can rename it away and create new one herself, for > example. > >> I will deal with it. >> >> I find there are some similar leakage in this file. I'll fix them in >> another patch. >> >> Do you think we need some additional comments for the release of strbuf? > > As Documentation/technical/api-strbuf.txt has this, I think we are > already OK. > > `strbuf_release`:: > > Release a string buffer and the memory it used. You should not use the > string buffer after using this function, unless you initialize it > again. > Excuse me, but I could not find `Documentation/technical/api-strbuf.txt` in master branch. Do you refer to the header of `strbuf.h`? In which, I learnt how to initialize the strbuf and how to take use of it when I began to use it. If there is also a note about whether I should release it and how do it, such as: For every strbuf that has been initialized and buffer of it has not been detached with strbuf_detach, you should release the resource by strbuf_release. It will save me (maybe others) much time to explore the entire method list. -- To unsubscribe from this list: send the line "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: [GSoC] A late proposal: a modern send-email
2016-03-28 6:00 GMT+08:00 Eric Wong : > Junio C Hamano wrote: >> 惠轶群 writes: >> > - Build a simple email client (maybe a web components based web app or >> > wxwidgets based GUI client, they are both cross-platform) which is >> > easy to use for sending patch without disrupting the mailbox format. >> >> I suspect it would yield a better result if the plan were to update >> a popular email client and make it possible to tell it to read an >> existing text file (i.e. mbox) without corrupting its contents. >> People do not have to learn a new mail client if done that way. > > Another bigger problem is people rely heavily on webmail UIs > nowadays instead of running any local mail clients :< I know many heavily email user choose to migrate to client such as alpine or mutt after using gmail for a long time. > While Gmail provides SMTP access, it was (last I was told) > incompatible with two-factor auth; so I've encountered users > unable to send patches with their normal 2FA-enabled accounts. That's the origin of this idea of `mailto`. In fact, you could send mail via 2FA-enabled accounts via "app password" metioned by Javier. But it's annoying to create "app password" for every client. If there is a `mailto` method to send patch, you could type something like git send-patch --mailto origin/master..HEAD Then, gmail is launched with the content of patch in it. You could edit the list of `to` and `cc`(You could even take use of gmail contact). Then just send. That's all. No need to SMTP config or "app password" any more. > Maybe git hackers at Google have enough pull to lobby Gmail's > web interface to make it easier to send plain-text patches; > but I would love more to see users running local mail clients > and even running their own SMTP servers. Yes, this should be free with user to pick their favorite mail client. >> That may not be a "Git" project, but GSoC is not limited to Git ;-) > > Completely agreed; email is critical to decentralized development; > but I don't believe decentralization is in the best interests of > any large and powerful corporation. > > IMHO, what we need is a SoIS: Summer of Independent Sysadmins :> -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] worktree: add: introduce --checkout option
By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.
Helped-by: Eric Sunshine
Helped-by: Junio C Hamano
Reviewed-by: Eric Sunshine
Signed-off-by: Ray Zhang
---
Changes since last version of this patch[v3]:
Documentation/git-worktree.txt: HEAD --> ``
t/t2025-worktree-add.sh: fix style
[v3]: http://article.gmane.org/gmane.comp.version-control.git/289877
[v2]: http://article.gmane.org/gmane.comp.version-control.git/289713
[v1]: http://article.gmane.org/gmane.comp.version-control.git/289659
---
Documentation/git-worktree.txt | 8 +++-
builtin/worktree.c | 15 ++-
t/t2025-worktree-add.sh| 13 +
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 62c76c1..c622345 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees
SYNOPSIS
[verse]
-'git worktree add' [-f] [--detach] [-b ] []
+'git worktree add' [-f] [--detach] [--checkout] [-b ]
[]
'git worktree prune' [-n] [-v] [--expire ]
'git worktree list' [--porcelain]
@@ -87,6 +87,12 @@ OPTIONS
With `add`, detach HEAD in the new working tree. See "DETACHED HEAD"
in linkgit:git-checkout[1].
+--[no-]checkout::
+ By default, `add` checks out ``, however, `--no-checkout` can
+ be used to suppress checkout in order to make customizations,
+ such as configuring sparse-checkout. See "Sparse checkout"
+ in linkgit:git-read-tree[1].
+
-n::
--dry-run::
With `prune`, do not remove anything; just report what it would
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 38b5609..e677cd7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -21,6 +21,7 @@ static const char * const worktree_usage[] = {
struct add_opts {
int force;
int detach;
+ int checkout;
const char *new_branch;
int force_new_branch;
};
@@ -284,11 +285,13 @@ static int add_worktree(const char *path, const char
*refname,
if (ret)
goto done;
- cp.argv = NULL;
- argv_array_clear(&cp.args);
- argv_array_pushl(&cp.args, "reset", "--hard", NULL);
- cp.env = child_env.argv;
- ret = run_command(&cp);
+ if (opts->checkout) {
+ cp.argv = NULL;
+ argv_array_clear(&cp.args);
+ argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+ cp.env = child_env.argv;
+ ret = run_command(&cp);
+ }
if (!ret) {
is_junk = 0;
free(junk_work_tree);
@@ -320,10 +323,12 @@ static int add(int ac, const char **av, const char
*prefix)
OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
N_("create or reset a branch")),
OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named
commit")),
+ OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new
working tree")),
OPT_END()
};
memset(&opts, 0, sizeof(opts));
+ opts.checkout = 1;
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
die(_("-b, -B, and --detach are mutually exclusive"));
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index cbfa41e..472b811 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -213,4 +213,17 @@ test_expect_success 'local clone from linked checkout' '
( cd here-clone && git fsck )
'
+test_expect_success '"add" worktree with --no-checkout' '
+ git worktree add --no-checkout -b swamp swamp &&
+ ls swamp >actual &&
+ test_line_count = 0 actual &&
+ git -C swamp reset --hard &&
+ test_cmp init.t swamp/init.t
+'
+
+test_expect_success '"add" worktree with --checkout' '
+ git worktree add --checkout -b swmap2 swamp2 &&
+ test_cmp init.t swamp2/init.t
+'
+
test_done
--
https://github.com/git/git/pull/217
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] git-push-update, tool to push with "server-side" merge or rebase
Hello. I would like to announce git-push-update, a tool which emulates server-side merge or rebase. The link: https://github.com/max630/git-push-update It is a bash script which fetches latest remote branch, creates temporary clone, performs there merge or rebase, pushes results to server. If during the merge or rebase remote branch was updated, it fails cleanly, so you are not left with merge which did not go anywhere. Or it can even retry the whole task from the beginning, until it eventually succeeds. I tried to make it easy to use by unexperienced users by making as few options as possible and checking for some dangerous mistakes. It would be nice if somebody tried to really use it, so there would be some data does this direction worth exploring. Any other feedback is also welcome. A longer explanation: While topic branches/pullrequests/whatever-it-named workflow is obviously superior to push-to-trunk approach which is used with centralized VCSes, there can be cases to use the latter one. But doing it with Git is not easy: * when the trunk goes forward, user have to run merge or rebase (further "update"), interrupting other work which might be in progress. * while doing fetch, update and push back a concurrent push can happen, making user to have to repeat it all over. * some scenarios allow user to make a mistake combining branches which mean to be unrelated, for example merging or rebasing active development branch into maintenance branch for older version. * for a merge case there is a problem of "evil pull" (http://thread.gmane.org/gmane.comp.version-control.git/247237) In short: the merges which are to go to remote branch should be "from local to remote", and git-pull merges "from remote to local". This was discussed around some time ago, but I could not find anything done about it. It might seem like nobody really interested much. But I still can see discussions here and there. Also, some time ago extension "pushrebase" for mercurial appeared, which indicates that there is really a demand. Looks like current git remote protocol does not really allow server to tweak pushed commits: if it accepts reference, client will remember exactly the commit it was pushing, no modifications is possible. Also, if it is implemented as server-side feature, it might take years to appear at github or other big public hostings, if ever. Until that most users would not be able to try it. This leaves only one option: perform merge or rebase locally, pretending that it was done at server. It does not even have to be implemented in git itself, instead a wrapper script can do everything. So here is the script. -- 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 1/4] config --show-origin: report paths with forward slashes
Am 22.03.2016 um 18:42 schrieb Johannes Schindelin:
> On Windows, the backslash is the native directory separator, but all
> supported Windows versions also accept the forward slash in most
> circumstances.
>
> Our tests expect forward slashes.
>
> Relative paths are generated by Git using forward slashes.
>
> So let's try to be consistent and use forward slashes in the $HOME part
> of the paths reported by `git config --show-origin`, too.
>
> Signed-off-by: Johannes Schindelin
> ---
> compat/mingw.h | 6 ++
> path.c | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 8c5bf50..c008694 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -396,6 +396,12 @@ static inline char *mingw_find_last_dir_sep(const char
> *path)
> ret = (char *)path;
> return ret;
> }
> +static inline void convert_slashes(char *path)
> +{
> + for (; *path; path++)
> + if (*path == '\\')
> + *path = '/';
> +}
> #define find_last_dir_sep mingw_find_last_dir_sep
> int mingw_offset_1st_component(const char *path);
> #define offset_1st_component mingw_offset_1st_component
> diff --git a/path.c b/path.c
> index 8b7e168..969b494 100644
> --- a/path.c
> +++ b/path.c
> @@ -584,6 +584,9 @@ char *expand_user_path(const char *path)
> if (!home)
> goto return_null;
> strbuf_addstr(&user_path, home);
> +#ifdef GIT_WINDOWS_NATIVE
> + convert_slashes(user_path.buf);
> +#endif
> } else {
> struct passwd *pw = getpw_str(username, username_len);
> if (!pw)
>
This change is not warranted for the following reasons:
- It is only there to satisfy the --show-origin tests, but not
for the benefit of the users.
- The use of $HOME in those tests is just incidental, but not necessary.
- There is no reason to change only paths depending on $HOME,
but no other paths imported through the environment.
I see no advantage for the users of --show-origin that they now
see C:/foo/bar instead of C:\foo\bar. (For this reason, I'm not
suggesting to change all paths imported from the environment, just
the contrary, to leave them all alone).
A change like this whould have been preferable:
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6767da8..4c96044 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1209,7 +1209,7 @@ test_expect_success POSIXPERM,PERL 'preserves existing
permissions' '
HOME="$(pwd)" # convert to Windows path
test_expect_success 'set up --show-origin tests' '
- INCLUDE_DIR="$HOME/include" &&
+ INCLUDE_DIR="$(pwd)/include" &&
mkdir -p "$INCLUDE_DIR" &&
cat >"$INCLUDE_DIR"/absolute.include <<-\EOF &&
[user]
@@ -1219,7 +1219,7 @@ test_expect_success 'set up --show-origin tests' '
[user]
relative = include
EOF
- cat >"$HOME"/.gitconfig <<-EOF &&
+ cat >"$(pwd)"/.gitconfig <<-EOF &&
[user]
global = true
override = global
@@ -1237,9 +1237,9 @@ test_expect_success 'set up --show-origin tests' '
test_expect_success '--show-origin with --list' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfig user.global=true
- file:$HOME/.gitconfig user.override=global
- file:$HOME/.gitconfig
include.path=$INCLUDE_DIR/absolute.include
+ file:$(pwd)/.gitconfig user.global=true
+ file:$(pwd)/.gitconfig user.override=global
+ file:$(pwd)/.gitconfig
include.path=$INCLUDE_DIR/absolute.include
file:$INCLUDE_DIR/absolute.include user.absolute=include
file:.git/configuser.local=true
file:.git/configuser.override=local
@@ -1253,9 +1253,9 @@ test_expect_success '--show-origin with --list' '
test_expect_success '--show-origin with --list --null' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfigQuser.global
- trueQfile:$HOME/.gitconfigQuser.override
- globalQfile:$HOME/.gitconfigQinclude.path
+ file:$(pwd)/.gitconfigQuser.global
+ trueQfile:$(pwd)/.gitconfigQuser.override
+ globalQfile:$(pwd)/.gitconfigQinclude.path
$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute
includeQfile:.git/configQuser.local
trueQfile:.git/configQuser.override
@@ -1284,7 +1284,7 @@ test_expect_success '--show-origin with single file' '
test_expect_success '--show-origin with --get-regexp' '
cat >expect <<-EOF &&
- file:$HOME/.gitconfig user.global true
+ file:$(pwd)/.gitconfig user.global true
file:.git/config
[PATCH v2] branch -d: refuse deleting a branch which is currently checked out
When a branch is checked out by current working tree, deleting the
branch is forbidden. However when the branch is checked out only by
other working trees, deleting is allowed.
Use find_shared_symref() to check if the branch is in use, not just
comparing with the current working tree's HEAD.
Signed-off-by: Kazuki Yamaguchi
---
% git worktree list
/path/to 2c3c5f2 [master]
/path/to/wt 2c3c5f2 [branch-a]
% git branch -d branch-a
error: Cannot delete the branch 'branch-a' which is currently checked out at
'/path/to/wt'
builtin/branch.c | 22 ++
t/t3200-branch.sh | 6 ++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6bd6b80..de1641306c9e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -20,6 +20,7 @@
#include "utf8.h"
#include "wt-status.h"
#include "ref-filter.h"
+#include "worktree.h"
static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r | -a] [--merged | --no-merged]"),
@@ -215,16 +216,21 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
int flags = 0;
strbuf_branchname(&bname, argv[i]);
- if (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {
- error(_("Cannot delete the branch '%s' "
- "which you are currently on."), bname.buf);
- ret = 1;
- continue;
- }
-
free(name);
-
name = mkpathdup(fmt, bname.buf);
+
+ if (kinds == FILTER_REFS_BRANCHES) {
+ char *worktree = find_shared_symref("HEAD", name);
+ if (worktree) {
+ error(_("Cannot delete the branch '%s' "
+ "which is currently checked out at
'%s'"),
+ bname.buf, worktree);
+ free(worktree);
+ ret = 1;
+ continue;
+ }
+ }
+
target = resolve_ref_unsafe(name,
RESOLVE_REF_READING
| RESOLVE_REF_NO_RECURSE
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a89724849065..508007fd3772 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -403,6 +403,12 @@ test_expect_success 'test deleting branch without config' '
test_i18ncmp expect actual
'
+test_expect_success 'deleting currently checked out branch fails' '
+ git worktree add -b my7 my7 &&
+ test_must_fail git -C my7 branch -d my7 &&
+ test_must_fail git branch -d my7
+'
+
test_expect_success 'test --track without .fetch entries' '
git branch --track my8 &&
test "$(git config branch.my8.remote)" &&
--
2.8.0.rc4.17.gdb328b3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] branch -d: refuse deleting a branch which is currently checked out
On Sun, Mar 27, 2016 at 01:52:18PM -0400, Eric Sunshine wrote:
> On Fri, Mar 25, 2016 at 2:28 PM, Kazuki Yamaguchi wrote:
> > When a branch is checked out by current working tree, deleting the
> > branch is forbidden. However when the branch is checked out only by
> > other working trees, deleting is allowed.
> > Use find_shared_symref() to check if the branch is in use, not just
> > comparing with the current working tree's HEAD.
> >
> > Signed-off-by: Kazuki Yamaguchi
> > ---
> > diff --git a/builtin/branch.c b/builtin/branch.c
> > @@ -215,16 +216,17 @@ static int delete_branches(int argc, const char
> > **argv, int force, int kinds,
> > int flags = 0;
> >
> > strbuf_branchname(&bname, argv[i]);
> > - if (kinds == FILTER_REFS_BRANCHES && !strcmp(head,
> > bname.buf)) {
> > + free(name);
> > + name = mkpathdup(fmt, bname.buf);
> > +
> > + if (kinds == FILTER_REFS_BRANCHES &&
> > + find_shared_symref("HEAD", name)) {
> > error(_("Cannot delete the branch '%s' "
> > - "which you are currently on."), bname.buf);
> > + "which is currently checked out."),
> > bname.buf);
>
> Would it be possible to do a better job of letting the user know what
> went wrong by stating in which worktree(s) the branch is checked out?
> My concern is that someone seeing this message might respond "huh? I
> have 'master' checked out, so why is this telling me that 'foo' is
> checked out", and not realize that 'foo' is in fact checked out in a
> different worktree.
Yes, indeed. I'll do it. 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
