* Re: [RFC] Making pathspec limited log play nicer with --first-parent
From: Junio C Hamano @ 2012-01-19 20:43 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <CA+55aFxucaeX7it_Kj7WV3ZbwCukN+wvbuxqJzh3V5Rxz4ib1g@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, Jan 19, 2012 at 11:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Comments?
>
> Looks conceptually right, but I do have to admit to hating that new variable.
>
> I don't see a better way to do it, though. Sure, you could do it with just
>
> if (revs->first_parent_only && pp != &commit->parents)
> break;
>
> and avoid the new variable that way, but that replaces the annoying
> variable with a pretty subtle thing.
>
> Or we could re-write that while() loop and move the 'parent' variable
> into it. Like the appended untested thing.
>
> But maybe your patch is better, and my dislike for that parent counter
> is just irrational.
I didn't like that parent counter that _only_ increments when we are
running under first-parent-only mode at the conceptual level. At the
implementation level, of course it is the right thing to do because
outside first-parent-only mode nobody cares about the parent counter,
so it is a valid but subtle optimization.
But I personally find your loop
do {
...
} while (!revs->first_parent_only);
is even more disgusting. It is misleading to have something that is not
supposed to change inside the loop as the terminating condition as if we
are saying "loop until somebody flips that bit" which is clearly not the
case.
So obviously I am saying that I do not think either patch is pretty
without offering a better alternative implementation, which is my usual
badness. As this is not an ultra urgent fix, I'll wait and see if somebody
else comes up with a more readable version.
Thanks for eyeballing the logic side of it, anyway. That was what I was
worried about the change the most.
^ permalink raw reply
* Re: [RFC] Making pathspec limited log play nicer with --first-parent
From: Linus Torvalds @ 2012-01-19 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwr8niftt.fsf@alter.siamese.dyndns.org>
On Thu, Jan 19, 2012 at 12:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> But I personally find your loop is even more disgusting
Yeah, I can't really argue with that.
The thing I was playing with was to move the entire loop content into
a helper function (which would return the new pp), and then the
"first-parent only" case just wouldn't do a loop at all.
But I couldn't be bothered. Your patch certainly does have the
advantage of being minimally intrusive.
Linus
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Jehan Bing @ 2012-01-19 21:07 UTC (permalink / raw)
To: git; +Cc: Michael Nahas
In-Reply-To: <CADo4Y9iKvoXhKg5pEAB+cbA7Rkfa=nF4TLu0xgcS3dnkNi_n4g@mail.gmail.com>
On 2012-01-19 09:20, Michael Nahas wrote:
> The problem I'm running into is that whenever I change a file in a
> directory, I have to bump up the version number in the configuration
> file. The larger version value in the config file causes my changes
> to be loaded over the old ones.
>
> Most of my commits are edits to a file like "foo.js" and an increment
> to the version number in "config". Ideally, each of my features
> should live in a single commit and I should be able to make a sequence
> of them, each time incrementing the version number in config.
>
> The problem I'm running into starts with me editing version=100. I
> create new commits where I set the version to 101, 102, 103, 104.
> When I go to push ("git svn dcommit"), my coworkers have incremented
> the version to 103. So, I rebase my changes, and get conflicts every
> time because of the version number!
>
> Is there a good way to avoid these conflicts? Is there a hook I can
> write? Is there a change to this process that would work smoother
> with Git and its distributed development? It's okay if the version
> number skips numbers (e.g., jumps from 100 to 104), as long as it
> increases.
Maybe you can do something with "git rerere"
(http://progit.org/2010/03/08/rerere.html). It supposed to automatically
resolve known conflicts.
I've never used myself, I just know it exists, so I don't know it's
usable in your case. But possibly you would pre-fill the rerere cache
(assuming that the format is simple enough) then just run rebase.
Jehan
^ permalink raw reply
* Re: [PATCH] git-add: allow --ignore-missing always, not just in dry run
From: Junio C Hamano @ 2012-01-19 21:26 UTC (permalink / raw)
To: Dieter Plaetinck; +Cc: Jens Lehmann, git
In-Reply-To: <20120119115216.2773a02f@plaetinck.be>
Dieter Plaetinck <dieter@plaetinck.be> writes:
> So basically, if this tool needs to check which files still/no-longer
> exist before calling git-add, that's vulnerable to race conditions.
I do not think you are solving the real problem in your script even if you
allowed "add --ignore-missing".
I suspect you are making things even worse by using "--ignore-missing" in
your script. If a user is actively updating the files in the filesystem,
at least "git add" without "--ignore-missing" would catch the case where
you _thought_ the user modified but still has the file, but in reality the
further updates in the working tree removed the file, which is a clear
indication that the rate you are processing the notify stream is slower
than the activity generated by the user and allows you to notice that you
may be better off waiting a bit until things calm down before running your
automated commit.
Also, with or without "--ignore-missing", I think we have safety valves to
cause "git add" fail if the file being added is updated while git is
working on it (i.e. we read and compute the object name, and then store it
compressed, and check the hash of what is stored matches the object name
we computed earlier, which would fail if the file is updated in the middle
at the right time).
This means that the "--ignore-missing" option will _not_ eliminate all
cases where "git add" may detect an error and fails. In other words, your
script needs to deal with error return from "git add" anyway even if we
applied your patch and you used "--ignore-missing" in your script.
I have to say that the basic premise of your script is simply broken, and
I am afraid that it is unfixable without an atomic snapshot support from
the underlying filesystem (i.e. take a snapshot, run 'git add' on it, and
then release the snapshot).
Having said all that, I do agree to the view that it is OK to let it
happen if the user explicitly asks a typo'ed pathspec on the command line
to be ignored for interactive use cases, and for that reason alone, I am
not fundamentally opposed to allowing the use of --ignore-missing outside
the --dry-run context.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/2] am: learn passing -b to mailinfo
From: Junio C Hamano @ 2012-01-19 21:26 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <a804650f805fd8c89a843302cb92bbbdf36b8c0b.1326710194.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> git-am could pass -k to mailinfo, but not -b. Introduce an option
> that does so. We change the meaning of the 'keep' state file, but are
> careful not to cause a problem unless you downgrade in the middle of
> an 'am' run.
>
> This uncovers a bug in mailinfo -b, hence the failing test.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> This fixes the broken 'if', and the use of 'echo' with an argument
> that starts with '-'.
After re-reading the code that parses the command line options given to
"am" and the previous invocation state we read from $dotest/*, however, I
think the way this change uses $keep makes things somewhat inconsistent
and harder to follow.
Currently the variables are given abstract meaning (e.g. "are we told to
record to utf8? yes or no") when we parse our command line options and
read from the previous invocation state, and then based on that abstract
meaning, a later code decides what exact option we throw at the git
commands we invoke (e.g. "utf8=t" -> "-u").
How about doing something like this instead at least for now? It might be
better to decide when we parse our options and $dotest/* immediately what
options we give to the git commands we run (which your patch does but only
to $keep option), but that kind of change (1) belongs to a separate topic
and should be done consistently to all options, and (2) I am not convinced
if it is necessarily a good change.
Thanks.
diff --git a/git-am.sh b/git-am.sh
index 6cdd591..8b755d9 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -15,6 +15,7 @@ q,quiet be quiet
s,signoff add a Signed-off-by line to the commit message
u,utf8 recode into utf8 (default)
k,keep pass -k flag to git-mailinfo
+keep-non-patch pass -b flag to git-mailinfo
keep-cr pass --keep-cr flag to git-mailsplit for mbox format
no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
c,scissors strip everything before a scissors line
@@ -345,6 +346,8 @@ do
utf8= ;;
-k|--keep)
keep=t ;;
+ --keep-non-patch)
+ keep=b ;;
-c|--scissors)
scissors=t ;;
--no-scissors)
@@ -522,16 +525,25 @@ case "$resolved" in
fi
esac
+# Now, decide what command line options we will give to the git
+# commands we invoke, based on the result of parsing command line
+# options and previous invocation state stored in $dotest/ files.
+
if test "$(cat "$dotest/utf8")" = t
then
utf8=-u
else
utf8=-n
fi
-if test "$(cat "$dotest/keep")" = t
-then
- keep=-k
-fi
+keep=$(cat "$dotest/keep")
+case "$keep" in
+t)
+ keep=-k ;;
+b)
+ keep=-b ;;
+*)
+ keep= ;;
+esac
case "$(cat "$dotest/keepcr")" in
t)
keepcr=--keep-cr ;;
^ permalink raw reply related
* Re: Rebase and incrementing version numbers
From: Jon Seymour @ 2012-01-19 21:33 UTC (permalink / raw)
To: mike; +Cc: git
In-Reply-To: <CADo4Y9iKvoXhKg5pEAB+cbA7Rkfa=nF4TLu0xgcS3dnkNi_n4g@mail.gmail.com>
On Fri, Jan 20, 2012 at 4:20 AM, Michael Nahas <mike.nahas@gmail.com> wrote:
> I'm at a new job and using Git-SVN at a place that is accustomed to SVN.
>
> The problem I'm running into is that whenever I change a file in a
> directory, I have to bump up the version number in the configuration
> file. The larger version value in the config file causes my changes
> to be loaded over the old ones.
>
> Most of my commits are edits to a file like "foo.js" and an increment
> to the version number in "config". Ideally, each of my features
> should live in a single commit and I should be able to make a sequence
> of them, each time incrementing the version number in config.
>
> The problem I'm running into starts with me editing version=100. I
> create new commits where I set the version to 101, 102, 103, 104.
> When I go to push ("git svn dcommit"), my coworkers have incremented
> the version to 103. So, I rebase my changes, and get conflicts every
> time because of the version number!
>
> Is there a good way to avoid these conflicts? Is there a hook I can
> write? Is there a change to this process that would work smoother
> with Git and its distributed development? It's okay if the version
> number skips numbers (e.g., jumps from 100 to 104), as long as it
> increases.
>
> Thanks,
>
> Mike
I wonder if you can defer your changes to the config files until after
you have synced with the current SVN head, so that you typically only
modify the latest configuration file. Then use git to work out what
numbers you have to update (by working out which files you changed
that the SVN upstream has not seen yet). Not perfect, because of race
conditions, and may not work with your integration testing processes,
but perhaps worth considering.
Something like:
1. pull latest SVN
2. work on file
3. test. skip back to 2 until done.
4. ready to push to upstream
5. pull latest SVN
6. calculate configuration changes required
7. apply configuration changes
8. push work + configuration changes upstream
So, there is a window between steps 5 and 8 where you might still have
to deal with a conflict, but at least it should be much reduced.
I agree with other comments, though, a saner approach might be to
generate the configuration as part of a build process rather than
trying to maintain it in source control.
jon.
^ permalink raw reply
* Re: post-update to stash after push to non-bare current branch
From: Neal Kreitzinger @ 2012-01-19 21:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Neal Kreitzinger, git
In-Reply-To: <7vboq0ld5e.fsf@alter.siamese.dyndns.org>
On 1/18/2012 7:00 PM, Junio C Hamano wrote:
> Neal Kreitzinger<nkreitzinger@gmail.com> writes:
>
>>> Have you checked where in the filesystem hierarchy that script is run
>>> (hint: pwd)?
>>>
>> echo pwd in post-update echoes /path/WORKTREE/.git in git-push stdout.
>> ...
>> 'git-checkout -f' works manually, but in post-update hook...
>
> Stronger hint. Did you run "git checkout -f" in /path/WORKTREE/.git to
> back that "works manually" claim?
(Manual behavior):
If pwd is WORKTREE/ then git-checkout has correct effect, ie. worktree,
index, and HEAD match.
If pwd is WORKTREE/.git/ then git-checkout complains 'must be run in a
worktree' and has no effect.
(post-update hook behavior):
If pwd is WORKTREE/.git/ then git-push verbage does not complain, and
git-checkout exits zero status but has incorrect effect, ie. index and
HEAD match, but worktree matches HEAD@{1}.
If pwd is WORKTREE/ then git-push complains 'not a git repository', and
git-checkout exits non-zero status and has no effect, ie. worktree and
index match HEAD@{1}.
Evidence:
(post-update hook script):
MYPWD=`pwd`
echo $MYPWD
pushd /home/neal/FSNMSTHTML
MYPWD=`pwd`
echo $MYPWD
git checkout -f HEAD
if [ $? -ne 0 ]; then
echo "error on checkout!"
else
echo "checkout HEAD to non-bare remote current branch after push"
fi
echo $MYPWD
popd
MYPWD=`pwd`
echo $MYPWD
(git-push verbage):
$ git push origin HEAD
Counting objects: 9, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 462 bytes, done.
Total 5 (delta 4), reused 0 (delta 0)
Unpacking objects: 100% (5/5), done.
remote: warning: updating the current branch
remote: /home/neal/FSNMSTHTML/.git
remote: ~/FSNMSTHTML ~/FSNMSTHTML/.git
remote: /home/neal/FSNMSTHTML
remote: fatal: Not a git repository: '.'
remote: error on checkout!
remote: /home/neal/FSNMSTHTML
remote: ~/FSNMSTHTML/.git
remote: /home/neal/FSNMSTHTML/.git
To file:///home/neal/FSNMSTHTML
cee9269..34dc5a9 HEAD -> master
v/r,
neal
^ permalink raw reply
* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Junio C Hamano @ 2012-01-19 22:01 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: git
In-Reply-To: <1327004244-18892-1-git-send-email-kirr@navytux.spb.ru>
Kirill Smelkov <kirr@navytux.spb.ru> writes:
> Starting from GNU Make 3.82 $(wildcard ...) no longer sorts the result
> (from NEWS):
>
> * WARNING: Backward-incompatibility!
> Wildcards were not documented as returning sorted values, but the results
> have been sorted up until this release.. If your makefiles require sorted
> results from wildcard expansions, use the $(sort ...) function to request
> it explicitly.
>
> http://repo.or.cz/w/make.git/commitdiff/2a59dc32aaf0681dec569f32a9d7ab88a379d34f
>
> so we have to sort tests list or else they are executed in seemingly
> random order even for -j1.
I do not necessarily buy your "so we HAVE TO, OR ELSE".
Even though I can understand "We can sort the list of tests _if_ we do not
want them executed in seemingly random order when running 'make -j1'", I
tend to think that *if* is a big one. Aren't these tests designed not to
depend on each other anyway?
^ permalink raw reply
* Re: Unexpected "clean -Xd" behavior
From: pgit @ 2012-01-19 22:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jonathan Nieder, Pete Harlan, Git Mailing List, Shawn Bohrer
In-Reply-To: <CACsJy8AE+rwmOVUZez5GRXRHJsTy+W8ekzr59NTd7_C+gB0Byw@mail.gmail.com>
Thank you very much for looking at this.
2012/1/19 "Nguyen Thai Ngoc Duy" <pclouds@gmail.com>:
> 2012/1/19 Jonathan Nieder <jrnieder@gmail.com>:
>> Pete Harlan wrote:
>>
>>> When a directory contains nothing but an ignored subdirectory, that
>>> subdirectory does not get removed by "git clean -Xdf".
>>>
>>> For example, in a new directory:
>>>
>>> # git init
>>> Initialized empty Git repository in /tmp/foo/.git/
>>> # echo a/ >.gitignore
>>> # git add .gitignore
>>> # git commit -m "Initial commit"
>>> [master (root-commit) c3af24c] Initial commit
>>> Â 1 files changed, 1 insertions(+), 0 deletions(-)
>>> Â create mode 100644 .gitignore
>>> # mkdir -p foo/a
>>> # touch foo/a/junk.o
>>> # git status
>>> # On branch master
>>> nothing to commit (working directory clean)
>>> # git clean -Xdn  # <--- DOES NOT MENTION foo/a
>
> -X is to remove ignored files _only_ (DIR_SHOW_IGNORED flag). And
> "foo" is not ignored according to .gitignore, so it cuts short there
> and never gets to "foo/a". -x works.
But the presence of a tracked file in foo makes it not cut short there, so
the logic seems a bit off. (If we're interested in removing ignored files
only, then the ignored files (not a tracked file) should trigger us
looking into foo. I don't know Git internals but I'm guessing it's not
quite that simple.)
> May be intentional, may be not
> (we hit a corner case). I don't know. Commit message b991625 might
> help:
>
> dir.c: Omit non-excluded directories with dir->show_ignored
>
> This makes "git-ls-files --others --directory --ignored" behave
> as documented and consequently also fixes "git-clean -d -X".
> Previously, git-clean would remove non-excluded directories
> even when using the -X option.
It can (and does) leave foo behind (because it's not ignored), but it
would conform better to the -X documentation if the ignored files were
removed.
BTW the above commit doesn't affect the behavior in this example.
If a fix isn't desirable then as Jonathan said updating the documentation
makes sense. (And those of us using it as a poor man's "make clean" can
just fix our Makefiles instead...)
Thanks,
--Pete
^ permalink raw reply
* Re: More support on branch description?
From: Junio C Hamano @ 2012-01-19 22:22 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <CACsJy8D0_EB6jN7KxpzLtnPnj0HjdU6sNHJRyqXJf-2-ZNatFA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> The coming v1.7.9 will introduce branch description, mainly used in
> integration process. I think we could make it useful for users who
> don't extensively use request-pull/format-patch. Showing a short
> summary along with branch name in "git branch" would be nice.
I agree that it would be nice to give users access to the information even
if the branch ends up being merged to the master branch by you and never
leaves your repository by itself.
You however are misguided to say "Showing a short summary along...".
The branch description support is to give users a place to record detailed
explanation about a branch, similar in size to what you would normally
place in a log message of a commit or a cover letter of a series. There
wasn't any convenient place to do so for a branch that is (1) inherently a
moving target while it is being developed and (2) is not a good match for
tags and notes.
There already is a good place for a brief summary and it is called "branch
name". Name your branches just like you name your functions.
^ permalink raw reply
* [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn O. Pearce @ 2012-01-19 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
From: "Shawn O. Pearce" <spearce@spearce.org>
The protocol between transport-helper.c and remote-curl requires
remote-curl to always print a blank line after the push command
has run. If the blank line is ommitted, transport-helper kills its
container process (the git push the user started) with exit(128)
and no message indicating a problem, assuming the helper already
printed reasonable error text to the console.
However if the remote rejects all branches with "ng" commands in the
report-status reply, send-pack terminates with non-zero status, and
in turn remote-curl exited with non-zero status before outputting
the blank line after the helper status printed by send-pack. No
error messages reach the user.
This caused users to see the following from git push over HTTP
when the remote side's update hook rejected the branch:
$ git push http://... master
Counting objects: 4, done.
Delta compression using up to 6 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 301 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
$
Always print a blank line after the send-pack process terminates,
ensuring the helper status report (if it was output) will be
correctly parsed by the calling transport-helper.c. This ensures
the helper doesn't abort before the status report can be shown to
the user.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
remote-curl.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 48c20b8..d6054e2 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -805,7 +805,7 @@ static int push(int nr_spec, char **specs)
static void parse_push(struct strbuf *buf)
{
char **specs = NULL;
- int alloc_spec = 0, nr_spec = 0, i;
+ int alloc_spec = 0, nr_spec = 0, i, ret;
do {
if (!prefixcmp(buf->buf, "push ")) {
@@ -822,12 +822,11 @@ static void parse_push(struct strbuf *buf)
break;
} while (1);
- if (push(nr_spec, specs))
+ ret = push(nr_spec, specs);
+ xwrite(1, "\n", 1);
+ if (ret)
exit(128); /* error already reported */
- printf("\n");
- fflush(stdout);
-
free_specs:
for (i = 0; i < nr_spec; i++)
free(specs[i]);
--
1.7.8.4.dirty
^ permalink raw reply related
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-19 22:57 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1327011899-18883-1-git-send-email-spearce@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Always print a blank line after the send-pack process terminates,
> ensuring the helper status report (if it was output) will be
> correctly parsed by the calling transport-helper.c. This ensures
> the helper doesn't abort before the status report can be shown to
> the user.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
Anybody wants to add a simple test for this failure mode?
> remote-curl.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 48c20b8..d6054e2 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -805,7 +805,7 @@ static int push(int nr_spec, char **specs)
> static void parse_push(struct strbuf *buf)
> {
> char **specs = NULL;
> - int alloc_spec = 0, nr_spec = 0, i;
> + int alloc_spec = 0, nr_spec = 0, i, ret;
>
> do {
> if (!prefixcmp(buf->buf, "push ")) {
> @@ -822,12 +822,11 @@ static void parse_push(struct strbuf *buf)
> break;
> } while (1);
>
> - if (push(nr_spec, specs))
> + ret = push(nr_spec, specs);
> + xwrite(1, "\n", 1);
> + if (ret)
> exit(128); /* error already reported */
>
> - printf("\n");
> - fflush(stdout);
> -
This is not a fault of this patch, but could we fix this ugly mixture of
xwrite() and printf() in the same program? I can see that the loop in the
main() function carefully tries to call fflush(stdout) to make sure that
nothing is pending after processing a single command so using xwrite() may
not cause any harm here, but the thing is that you do not check the error
return from this xwrite(), so use of it is not giving us any potential
benefit of being able to detect I/O errors in a finer grained manner,
i.e. it is no better than the printf("\n"); fflush(stdout); sequence it
replaces.
Thanks.
^ permalink raw reply
* Re: Rebase and incrementing version numbers
From: Santi Béjar @ 2012-01-19 23:31 UTC (permalink / raw)
To: mike; +Cc: demerphq, git
In-Reply-To: <CADo4Y9is9mBOJaU+YRTMedTz7FfDrMFoDiqiUvQpVxQpyariPQ@mail.gmail.com>
On Thu, Jan 19, 2012 at 7:48 PM, Michael Nahas <mike.nahas@gmail.com> wrote:
> On Thu, Jan 19, 2012 at 1:12 PM, demerphq <demerphq@gmail.com> wrote:
>> Stop using version numbers and start using the git sha1 of the code
>> you are using.
>>
>> Yves
>
[...]
> 2. The version number needs to be increasing, to work with the current
> process. SHA1's are random.
Yes, but you can use "git describe" output:
$ git describe
v1.7.6-180-gdf3f3d8
HTH,
Santi
^ permalink raw reply
* Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default
From: Tay Ray Chuan @ 2012-01-20 1:14 UTC (permalink / raw)
To: Thomas Rast; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <87bopzofir.fsf@thomas.inf.ethz.ch>
On Thu, Jan 19, 2012 at 11:53 PM, Thomas Rast <trast@student.ethz.ch> wrote:
>[snip]
> Under [^[:space:]]+ neither of the examples would work. Actually,
> [^[:space:]]+ is the same as today's default, the [^[:space:]]* I
> mentioned later is (strictly speaking) broken as it allows for a
> 0-length match. (It doesn't really matter because IIRC the engine
> ignores 0-length words.)
My bad.
>[snip]
> I tried measuring it across a few commits, but it mostly gets drowned
> out by the diff effort. For a commit with stat
>
> exercises/cgal/cover/cover.cpp | 5 +-
> exercises/cgal/cover/cover.in1 |27014 +++++++++++++++-----
> exercises/cgal/cover/cover.in2 |48996 +++++++++++++++++++++++------------
> exercises/cgal/cover/cover.in3 |55041 +++++++++++++++++++++++++--------------
> exercises/cgal/cover/cover.in4 |47600 ++++++++++++++++++++--------------
> exercises/cgal/cover/cover.int |43491 ++++++++++++++++++++++---------
> exercises/cgal/cover/cover.out1 | 53 +-
> exercises/cgal/cover/cover.out2 | 24 +-
> exercises/cgal/cover/cover.out3 | 11 +-
> exercises/cgal/cover/cover.out4 | 2 +-
> exercises/cgal/cover/cover.outt | 23 +-
> exercises/cgal/cover/gen | 39 +-
> exercises/cgal/cover/gen-1.cpp | 4 +-
> exercises/cgal/cover/gen-2.cpp | 6 +-
> exercises/cgal/cover/gen-3.cpp | 6 +-
>
> (sorry, can't share as those testcases are secret) I get best-of-5
> timings
>
> --word-diff-regex='[^[:space:]]+' 0:07.50real 7.40user 0.07system
> --word-diff 0:07.47real 7.41user 0.03system
>
> In conclusion, "meh". I think ripping out the isspace() part would make
> for a nice code reduction.
Thanks for the numbers. Well, that agrees with the intuition that
regex is slower than isspace(), since you have run it through the
regex engine.
>>> and your proposal is equivalent to
>>>
>>> [^[:space:]]|UTF_8_GUARD
>>>
>>> I think there is a case to be made for a default of
>>>
>>> [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+
>>>
>>> or some such. There's a lot of bikeshedding lurking in the (non)extent
>>> of the [[:alnum:]] here, however.
>>
>> Care to explain further? Not to sure what you mean here.
>
> For natural language, it may or may not make sense to match numbers as
> part of a word.
>
> For typical use in e.g. emails, a lot of punctuation has a double role;
> breaking words in
>
> http://article.gmane.org/gmane.comp.version-control.git/188391
>
> may or may not make sense.
>
> For some uses, especially source code, it would be better to match an
> underscore _ as part of a complete word, too.
>
> For some programming languages, say lisp, a dash - would also belong in
> the same category.
>
> There's no real reason other than ease of implementation why the pattern
> handles ASCII non-alphanumerics separately, but non-ASCII UTF-8
> non-alnums (like, say, unicode NO-BREAK SPACE which would show as \xc2
> \xa0) always goes into a word. But if you were to make UTF-8 sequences
> a single word, text in (say) many European languages would become
> chunked at accented letters.
>
> I'm sure you can find more items for this list. It's a grey area.
Thanks.
--
Cheers,
Ray Chuan
^ permalink raw reply
* [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn O. Pearce @ 2012-01-20 3:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vzkdjgv1i.fsf@alter.siamese.dyndns.org>
From: "Shawn O. Pearce" <spearce@spearce.org>
The protocol between transport-helper.c and remote-curl requires
remote-curl to always print a blank line after the push command
has run. If the blank line is ommitted, transport-helper kills its
container process (the git push the user started) with exit(128)
and no message indicating a problem, assuming the helper already
printed reasonable error text to the console.
However if the remote rejects all branches with "ng" commands in the
report-status reply, send-pack terminates with non-zero status, and
in turn remote-curl exited with non-zero status before outputting
the blank line after the helper status printed by send-pack. No
error messages reach the user.
This caused users to see the following from git push over HTTP
when the remote side's update hook rejected the branch:
$ git push http://... master
Counting objects: 4, done.
Delta compression using up to 6 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 301 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
$
Always print a blank line after the send-pack process terminates,
ensuring the helper status report (if it was output) will be
correctly parsed by the calling transport-helper.c. This ensures
the helper doesn't abort before the status report can be shown to
the user.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
remote-curl.c | 9 +++++----
t/t5541-http-push.sh | 27 +++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 48c20b8..25c1af7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -805,7 +805,7 @@ static int push(int nr_spec, char **specs)
static void parse_push(struct strbuf *buf)
{
char **specs = NULL;
- int alloc_spec = 0, nr_spec = 0, i;
+ int alloc_spec = 0, nr_spec = 0, i, ret;
do {
if (!prefixcmp(buf->buf, "push ")) {
@@ -822,12 +822,13 @@ static void parse_push(struct strbuf *buf)
break;
} while (1);
- if (push(nr_spec, specs))
- exit(128); /* error already reported */
-
+ ret = push(nr_spec, specs);
printf("\n");
fflush(stdout);
+ if (ret)
+ exit(128); /* error already reported */
+
free_specs:
for (i = 0; i < nr_spec; i++)
free(specs[i]);
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 9b85d42..4723930 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -95,6 +95,31 @@ test_expect_success 'create and delete remote branch' '
test_must_fail git show-ref --verify refs/remotes/origin/dev
'
+cat >"$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update" <<EOF
+#!/bin/sh
+exit 1
+EOF
+chmod a+x "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
+
+cat >exp <<EOF
+remote: error: hook declined to update refs/heads/dev2
+To http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git
+ ! [remote rejected] dev2 -> dev2 (hook declined)
+error: failed to push some refs to 'http://127.0.0.1:5541/smart/test_repo.git'
+EOF
+
+test_expect_success 'rejected update prints status' '
+ cd "$ROOT_PATH"/test_repo_clone &&
+ git checkout -b dev2 &&
+ : >path4 &&
+ git add path4 &&
+ test_tick &&
+ git commit -m dev2 &&
+ git push origin dev2 2>act
+ test_cmp exp act
+'
+rm -f "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
+
cat >exp <<EOF
GET /smart/test_repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
@@ -106,6 +131,8 @@ GET /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
GET /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
+GET /smart/test_repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
+POST /smart/test_repo.git/git-receive-pack HTTP/1.1 200
EOF
test_expect_success 'used receive-pack service' '
sed -e "
--
1.7.9.rc2.124.g1c075
^ permalink raw reply related
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-20 5:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1327029129-11424-1-git-send-email-spearce@spearce.org>
Thanks; will queue.
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Junio C Hamano @ 2012-01-20 6:00 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <1327029129-11424-1-git-send-email-spearce@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
> index 9b85d42..4723930 100755
> --- a/t/t5541-http-push.sh
> +++ b/t/t5541-http-push.sh
> @@ -95,6 +95,31 @@ test_expect_success 'create and delete remote branch' '
> test_must_fail git show-ref --verify refs/remotes/origin/dev
> '
>
> +cat >"$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update" <<EOF
> +#!/bin/sh
> +exit 1
> +EOF
> +chmod a+x "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/hooks/update"
> +
> +cat >exp <<EOF
> +remote: error: hook declined to update refs/heads/dev2
Curious. Where do we get these eight trailing whitespaces?
The call to rp_error("hook declined to update %s", name) seems to be
giving the name properly.
^ permalink raw reply
* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Kirill Smelkov @ 2012-01-20 6:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kirill Smelkov, git
In-Reply-To: <7v8vl3ic6o.fsf@alter.siamese.dyndns.org>
On Thu, Jan 19, 2012 at 02:01:51PM -0800, Junio C Hamano wrote:
> Kirill Smelkov <kirr@navytux.spb.ru> writes:
>
> > Starting from GNU Make 3.82 $(wildcard ...) no longer sorts the result
> > (from NEWS):
> >
> > * WARNING: Backward-incompatibility!
> > Wildcards were not documented as returning sorted values, but the results
> > have been sorted up until this release.. If your makefiles require sorted
> > results from wildcard expansions, use the $(sort ...) function to request
> > it explicitly.
> >
> > http://repo.or.cz/w/make.git/commitdiff/2a59dc32aaf0681dec569f32a9d7ab88a379d34f
> >
> > so we have to sort tests list or else they are executed in seemingly
> > random order even for -j1.
>
> I do not necessarily buy your "so we HAVE TO, OR ELSE".
>
> Even though I can understand "We can sort the list of tests _if_ we do not
> want them executed in seemingly random order when running 'make -j1'", I
> tend to think that *if* is a big one. Aren't these tests designed not to
> depend on each other anyway?
Yes, they don't depend on each other, but what's the point in not
sorting them? I usually watch test progress visually, and if tests are
sorted, even with make -j4 they go more or less incrementally by their t
number.
On my netbook, adding $(sort ...) adds approximately 0.008s to make
startup, so imho there is no performance penalty to adding that sort.
Thanks,
Kirill
^ permalink raw reply
* Re: Interesting behavior in git mergetool with no BASE revision
From: David Aguilar @ 2012-01-20 6:43 UTC (permalink / raw)
To: Jason Wenger; +Cc: git
In-Reply-To: <CAM6z=4_+yC4EnL9SZFd6=Nxs89QeHevNCakVzVycGBe7G+nTKQ@mail.gmail.com>
On Wed, Jan 18, 2012 at 3:05 PM, Jason Wenger <jcwenger@gmail.com> wrote:
> Doing a git merge on 1.7.4.3, on a case where both branches have a
> file created, and the base does not. Per git-mergetool:
>
> "the configured command line will be invoked with $BASE set to the
> name of a temporary file containing the common base for the merge, if
> available;"
>
> So testing in this case, I set my mergetool cmd as "echo $MERGED
> $LOCAL $REMOTE $BASE", and I get the following:
I have not seen this in practice. Maybe escape the variables?
In any case, I added a test case to cover this case. I think we
should at least provide an empty file in place of a missing $BASE.
> cio/.cproject ./cio/.cproject.LOCAL.9029.cproject
> ./cio/.cproject.REMOTE.9029.cproject
> ./cio/.cproject.BASE.9029.cproject
>
> ls -a cio shows the following files:
>
> .cproject
> .cproject.LOCAL.9325.cproject
> .cproject.BACKUP.9325.cproject
> .cproject.REMOTE.9325.cproject
>
> So the lack of base file makes sense -- There is no base to start
> from. However, $BASE evaluates to ./cio/.cproject.BASE.9029.cproject,
> which is a nonexistent file. This makes my actual mergetool upset to
> no end. Intuitively from documents, I would expect $BASE to evaluate
> to an empty string in this case.
>
> Is this intended behavior?
Not really. I'll send a patch shortly.
--
David
^ permalink raw reply
* [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20 6:46 UTC (permalink / raw)
To: gitster; +Cc: git
In-Reply-To: <CAJDDKr5mUiJkNk-urNn5fP5x+gkzaTfx2y=K1S0AJZCy7Muwdg@mail.gmail.com>
Some mergetools cannot cope when $BASE is missing.
This can happen when two branches add the same file.
Provide an empty file to make these tools happy.
Reported-by: Jason Wenger <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool.sh | 6 +++++-
t/t7610-mergetool.sh | 27 ++++++++++++++++++++++++++-
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..8521b81 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,7 +224,11 @@ merge_file () {
mv -- "$MERGED" "$BACKUP"
cp -- "$BACKUP" "$MERGED"
- base_present && checkout_staged_file 1 "$MERGED" "$BASE"
+ if base_present; then
+ checkout_staged_file 1 "$MERGED" "$BASE"
+ else
+ touch "$BASE"
+ fi
local_present && checkout_staged_file 2 "$MERGED" "$LOCAL"
remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 4aab2a7..2272743 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -39,6 +39,7 @@ test_expect_success 'setup' '
echo branch1 change >file1 &&
echo branch1 newfile >file2 &&
echo branch1 spaced >"spaced name" &&
+ echo branch1 both added > both &&
echo branch1 change file11 >file11 &&
echo branch1 change file13 >file13 &&
echo branch1 sub >subdir/file3 &&
@@ -50,6 +51,7 @@ test_expect_success 'setup' '
git checkout -b submod-branch1
) &&
git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
+ git add both &&
git rm file12 &&
git commit -m "branch1 changes" &&
@@ -58,6 +60,7 @@ test_expect_success 'setup' '
echo master updated >file1 &&
echo master new >file2 &&
echo master updated spaced >"spaced name" &&
+ echo master both added > both &&
echo master updated file12 >file12 &&
echo master updated file14 >file14 &&
echo master new sub >subdir/file3 &&
@@ -69,18 +72,22 @@ test_expect_success 'setup' '
git checkout -b submod-master
) &&
git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
+ git add both &&
git rm file11 &&
git commit -m "master updates" &&
git config merge.tool mytool &&
git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
- git config mergetool.mytool.trustExitCode true
+ git config mergetool.mytool.trustExitCode true &&
+ git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
+ git config mergetool.mybase.trustExitCode true
'
test_expect_success 'custom mergetool' '
git checkout -b test1 branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "" | git mergetool file1 file1 ) &&
( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
@@ -101,6 +108,7 @@ test_expect_success 'mergetool crlf' '
( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
@@ -131,6 +139,7 @@ test_expect_success 'mergetool on file in parent dir' '
cd subdir &&
( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&
@@ -212,6 +221,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -228,6 +238,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test ! -e submod &&
@@ -241,6 +252,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test ! -e submod &&
@@ -256,6 +268,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -279,6 +292,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -294,6 +308,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
git submodule update -N &&
@@ -309,6 +324,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test -d submod.orig &&
@@ -324,6 +340,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both>/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -445,4 +462,12 @@ test_expect_success 'directory vs modified submodule' '
git submodule update -N
'
+test_expect_success 'file with no base' '
+ git checkout -b test13 branch1 &&
+ test_must_fail git merge master &&
+ git mergetool --no-prompt --tool mybase -- base &&
+ test "$(cat "$MERGED")" = "" &&
+ git reset --hard master >/dev/null 2>&1
+'
+
test_done
--
1.7.9.rc2.1.gb566f
^ permalink raw reply related
* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20 6:57 UTC (permalink / raw)
To: gitster; +Cc: git, Jason Wenger
In-Reply-To: <1327042010-79552-1-git-send-email-davvid@gmail.com>
[apologies for forgetting to cc: you on the patch Jason]
On Thu, Jan 19, 2012 at 10:46 PM, David Aguilar <davvid@gmail.com> wrote:
> Some mergetools cannot cope when $BASE is missing.
> This can happen when two branches add the same file.
This might be better worded as "when two branches independently add
the same filename".
Feel free to squash... or I'll resend if you prefer.
Let me know if this does the trick for you, Jason. The test case is
happy; I hope your mergetool is as well.
Cheers,
David
> Provide an empty file to make these tools happy.
>
> Reported-by: Jason Wenger <jcwenger@gmail.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> git-mergetool.sh | 6 +++++-
> t/t7610-mergetool.sh | 27 ++++++++++++++++++++++++++-
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 085e213..8521b81 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -224,7 +224,11 @@ merge_file () {
> mv -- "$MERGED" "$BACKUP"
> cp -- "$BACKUP" "$MERGED"
>
> - base_present && checkout_staged_file 1 "$MERGED" "$BASE"
> + if base_present; then
> + checkout_staged_file 1 "$MERGED" "$BASE"
> + else
> + touch "$BASE"
> + fi
> local_present && checkout_staged_file 2 "$MERGED" "$LOCAL"
> remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
>
> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
> index 4aab2a7..2272743 100755
> --- a/t/t7610-mergetool.sh
> +++ b/t/t7610-mergetool.sh
> @@ -39,6 +39,7 @@ test_expect_success 'setup' '
> echo branch1 change >file1 &&
> echo branch1 newfile >file2 &&
> echo branch1 spaced >"spaced name" &&
> + echo branch1 both added > both &&
> echo branch1 change file11 >file11 &&
> echo branch1 change file13 >file13 &&
> echo branch1 sub >subdir/file3 &&
> @@ -50,6 +51,7 @@ test_expect_success 'setup' '
> git checkout -b submod-branch1
> ) &&
> git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
> + git add both &&
> git rm file12 &&
> git commit -m "branch1 changes" &&
>
> @@ -58,6 +60,7 @@ test_expect_success 'setup' '
> echo master updated >file1 &&
> echo master new >file2 &&
> echo master updated spaced >"spaced name" &&
> + echo master both added > both &&
> echo master updated file12 >file12 &&
> echo master updated file14 >file14 &&
> echo master new sub >subdir/file3 &&
> @@ -69,18 +72,22 @@ test_expect_success 'setup' '
> git checkout -b submod-master
> ) &&
> git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
> + git add both &&
> git rm file11 &&
> git commit -m "master updates" &&
>
> git config merge.tool mytool &&
> git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
> - git config mergetool.mytool.trustExitCode true
> + git config mergetool.mytool.trustExitCode true &&
> + git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
> + git config mergetool.mybase.trustExitCode true
> '
>
> test_expect_success 'custom mergetool' '
> git checkout -b test1 branch1 &&
> git submodule update -N &&
> test_must_fail git merge master >/dev/null 2>&1 &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool file1 file1 ) &&
> ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
> @@ -101,6 +108,7 @@ test_expect_success 'mergetool crlf' '
> ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
> @@ -131,6 +139,7 @@ test_expect_success 'mergetool on file in parent dir' '
> cd subdir &&
> ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&
> @@ -212,6 +221,7 @@ test_expect_success 'deleted vs modified submodule' '
> test_must_fail git merge master &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "r" | git mergetool submod ) &&
> rmdir submod && mv submod-movedaside submod &&
> @@ -228,6 +238,7 @@ test_expect_success 'deleted vs modified submodule' '
> test_must_fail git merge master &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool submod ) &&
> test ! -e submod &&
> @@ -241,6 +252,7 @@ test_expect_success 'deleted vs modified submodule' '
> test_must_fail git merge test6 &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "r" | git mergetool submod ) &&
> test ! -e submod &&
> @@ -256,6 +268,7 @@ test_expect_success 'deleted vs modified submodule' '
> test_must_fail git merge test6 &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool submod ) &&
> test "$(cat submod/bar)" = "master submodule" &&
> @@ -279,6 +292,7 @@ test_expect_success 'file vs modified submodule' '
> test_must_fail git merge master &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "r" | git mergetool submod ) &&
> rmdir submod && mv submod-movedaside submod &&
> @@ -294,6 +308,7 @@ test_expect_success 'file vs modified submodule' '
> test_must_fail git merge master &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool submod ) &&
> git submodule update -N &&
> @@ -309,6 +324,7 @@ test_expect_success 'file vs modified submodule' '
> test_must_fail git merge test7 &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "r" | git mergetool submod ) &&
> test -d submod.orig &&
> @@ -324,6 +340,7 @@ test_expect_success 'file vs modified submodule' '
> test_must_fail git merge test7 &&
> test -n "$(git ls-files -u)" &&
> ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
> + ( yes "" | git mergetool both>/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool submod ) &&
> test "$(cat submod/bar)" = "master submodule" &&
> @@ -445,4 +462,12 @@ test_expect_success 'directory vs modified submodule' '
> git submodule update -N
> '
>
> +test_expect_success 'file with no base' '
> + git checkout -b test13 branch1 &&
> + test_must_fail git merge master &&
> + git mergetool --no-prompt --tool mybase -- base &&
> + test "$(cat "$MERGED")" = "" &&
> + git reset --hard master >/dev/null 2>&1
> +'
> +
> test_done
> --
> 1.7.9.rc2.1.gb566f
>
^ permalink raw reply
* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: Junio C Hamano @ 2012-01-20 7:00 UTC (permalink / raw)
To: David Aguilar; +Cc: git
In-Reply-To: <1327042010-79552-1-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Some mergetools cannot cope when $BASE is missing.
> This can happen when two branches add the same file.
> Provide an empty file to make these tools happy.
>
> Reported-by: Jason Wenger <jcwenger@gmail.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> git-mergetool.sh | 6 +++++-
> t/t7610-mergetool.sh | 27 ++++++++++++++++++++++++++-
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 085e213..8521b81 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -224,7 +224,11 @@ merge_file () {
> mv -- "$MERGED" "$BACKUP"
> cp -- "$BACKUP" "$MERGED"
>
> - base_present && checkout_staged_file 1 "$MERGED" "$BASE"
> + if base_present; then
> + checkout_staged_file 1 "$MERGED" "$BASE"
> + else
> + touch "$BASE"
> + fi
Using "touch" for things like this is a disease.
You not just want to make sure it exists, but also you want to make sure
it is empty, so it would make your intention more explicit and clear if
you wrote this as
>"$BASE"
instead.
I also wonder if it may help mergetools if we come up with a fake base
image using the common material between the two files, in a way similar to
how git-merge-one-file.sh does it (look for "Added $4 in both, but
differently"), but obviously it would belong to a separate patch.
Thanks.
^ permalink raw reply
* Re: [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20 7:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhazqhn8u.fsf@alter.siamese.dyndns.org>
On Thu, Jan 19, 2012 at 11:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> Some mergetools cannot cope when $BASE is missing.
>> This can happen when two branches add the same file.
>> Provide an empty file to make these tools happy.
>>
>> Reported-by: Jason Wenger <jcwenger@gmail.com>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> git-mergetool.sh | 6 +++++-
>> t/t7610-mergetool.sh | 27 ++++++++++++++++++++++++++-
>> 2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-mergetool.sh b/git-mergetool.sh
>> index 085e213..8521b81 100755
>> --- a/git-mergetool.sh
>> +++ b/git-mergetool.sh
>> @@ -224,7 +224,11 @@ merge_file () {
>> mv -- "$MERGED" "$BACKUP"
>> cp -- "$BACKUP" "$MERGED"
>>
>> - base_present && checkout_staged_file 1 "$MERGED" "$BASE"
>> + if base_present; then
>> + checkout_staged_file 1 "$MERGED" "$BASE"
>> + else
>> + touch "$BASE"
>> + fi
>
> Using "touch" for things like this is a disease.
>
> You not just want to make sure it exists, but also you want to make sure
> it is empty, so it would make your intention more explicit and clear if
> you wrote this as
>
> >"$BASE"
>
> instead.
>
> I also wonder if it may help mergetools if we come up with a fake base
> image using the common material between the two files, in a way similar to
> how git-merge-one-file.sh does it (look for "Added $4 in both, but
> differently"), but obviously it would belong to a separate patch.
>
> Thanks.
Ah, thanks for the pointer. I'll resend shortly. A "fake base" would
be very helpful.
--
David
^ permalink raw reply
* [PATCH] mergetool: Provide an empty file when no base exists
From: David Aguilar @ 2012-01-20 7:10 UTC (permalink / raw)
To: gitster; +Cc: jcwenger, git
In-Reply-To: <7vhazqhn8u.fsf@alter.siamese.dyndns.org>
Some mergetools cannot cope when $BASE is missing.
This can happen when two branches add the same file.
Provide an empty file to make these tools happy.
Reported-by: Jason Wenger <jcwenger@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool.sh | 6 +++++-
t/t7610-mergetool.sh | 27 ++++++++++++++++++++++++++-
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 085e213..0131559 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -224,7 +224,11 @@ merge_file () {
mv -- "$MERGED" "$BACKUP"
cp -- "$BACKUP" "$MERGED"
- base_present && checkout_staged_file 1 "$MERGED" "$BASE"
+ if base_present; then
+ checkout_staged_file 1 "$MERGED" "$BASE"
+ else
+ :>"$BASE"
+ fi
local_present && checkout_staged_file 2 "$MERGED" "$LOCAL"
remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 4aab2a7..2272743 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -39,6 +39,7 @@ test_expect_success 'setup' '
echo branch1 change >file1 &&
echo branch1 newfile >file2 &&
echo branch1 spaced >"spaced name" &&
+ echo branch1 both added > both &&
echo branch1 change file11 >file11 &&
echo branch1 change file13 >file13 &&
echo branch1 sub >subdir/file3 &&
@@ -50,6 +51,7 @@ test_expect_success 'setup' '
git checkout -b submod-branch1
) &&
git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
+ git add both &&
git rm file12 &&
git commit -m "branch1 changes" &&
@@ -58,6 +60,7 @@ test_expect_success 'setup' '
echo master updated >file1 &&
echo master new >file2 &&
echo master updated spaced >"spaced name" &&
+ echo master both added > both &&
echo master updated file12 >file12 &&
echo master updated file14 >file14 &&
echo master new sub >subdir/file3 &&
@@ -69,18 +72,22 @@ test_expect_success 'setup' '
git checkout -b submod-master
) &&
git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
+ git add both &&
git rm file11 &&
git commit -m "master updates" &&
git config merge.tool mytool &&
git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
- git config mergetool.mytool.trustExitCode true
+ git config mergetool.mytool.trustExitCode true &&
+ git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
+ git config mergetool.mybase.trustExitCode true
'
test_expect_success 'custom mergetool' '
git checkout -b test1 branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "" | git mergetool file1 file1 ) &&
( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
@@ -101,6 +108,7 @@ test_expect_success 'mergetool crlf' '
( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
@@ -131,6 +139,7 @@ test_expect_success 'mergetool on file in parent dir' '
cd subdir &&
( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&
@@ -212,6 +221,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -228,6 +238,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test ! -e submod &&
@@ -241,6 +252,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test ! -e submod &&
@@ -256,6 +268,7 @@ test_expect_success 'deleted vs modified submodule' '
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -279,6 +292,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -294,6 +308,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
git submodule update -N &&
@@ -309,6 +324,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test -d submod.orig &&
@@ -324,6 +340,7 @@ test_expect_success 'file vs modified submodule' '
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool both>/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -445,4 +462,12 @@ test_expect_success 'directory vs modified submodule' '
git submodule update -N
'
+test_expect_success 'file with no base' '
+ git checkout -b test13 branch1 &&
+ test_must_fail git merge master &&
+ git mergetool --no-prompt --tool mybase -- base &&
+ test "$(cat "$MERGED")" = "" &&
+ git reset --hard master >/dev/null 2>&1
+'
+
test_done
--
1.7.9.rc2.1.gdcba7
^ permalink raw reply related
* Re: [PATCH] t/Makefile: Use $(sort ...) explicitly where needed
From: Junio C Hamano @ 2012-01-20 7:14 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: git
In-Reply-To: <20120120063450.GA15371@mini.zxlink>
Kirill Smelkov <kirr@navytux.spb.ru> writes:
>> I do not necessarily buy your "so we HAVE TO, OR ELSE".
>>
>> Even though I can understand "We can sort the list of tests _if_ we do not
>> want them executed in seemingly random order when running 'make -j1'", I
>> tend to think that *if* is a big one. Aren't these tests designed not to
>> depend on each other anyway?
>
> Yes, they don't depend on each other, but what's the point in not
> sorting them? I usually watch test progress visually, and if tests are
> sorted, even with make -j4 they go more or less incrementally by their t
> number.
>
> On my netbook, adding $(sort ...) adds approximately 0.008s to make
> startup, so imho there is no performance penalty to adding that sort.
Heh, who said anything about performance?
I was pointing out that your justification "we HAVE TO" was wrong.
If you are doing this for perceived prettyness and not as a fix for any
correctness issue, I want to see the patch honestly described as such;
that's all.
By the way, if I recall correctly, $(sort) in GNU make not just sorts but
as a nice side effect removes duplicates. So if we used a(n fictional)
construct in our Makefile like this:
T = $(wildcard *.sh a.*)
that might produce duplicates (i.e. "a.sh" might appear twice), which
might leave us two identical pathnames in $T and cause us trouble. Even
if we do not have such a use currently, rewriting $(wildcard) like your
patch does using $(sort $(wildcard ...)) may be a good way to future-proof
our Makefile, and if you justify your patch that way, it would be a
possible correctness hardening, not just cosmetics, and phrasing it with
"HAVE TO" may be justifiable.
Care to try if $(wildcard *.sh a.*) give you duplicated output with newer
GNU make? I am lazy but am a bit curious ;-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox