Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 09:49:19PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > ... I think you could also argue that > > because of whitespace-highlighting, colorized diffs are fundamentally > > going to have colors intermingled with the content and should not be > > parsed this way. > > Painting of whitespace breakages is asymmetric [*1*]. If you change > something on a badly indented line without fixing the indentation, > e.g. > > -hello-world > +hello-people > > only the space-before-tab on the latter line is painted. > > For the purpose of your diff highlighting, however, you would want > to treat the leading "hello-" on preimage and postimage > lines as unchanged. I do strip it off, so it is OK for it to be different in both the pre-image and post-image. But what I can't tolerate is the intermingling with actual data: +\t\t\x1b[32m;foo +\t\x1b[32m;bar Those are both post-image lines. I can strip off the "+" from each side to compare their inner parts to the pre-image. But the intermingled color gets in my way. I can simply strip all colors from the whole line, but then information is lost; how do I know where to put them back again? It is not just "add back the color at the beginning" (which is what I do with the prefix). I think the answer is that I must strip them out, retaining the colors found in each line along with their offset into the line, and then as I write out the lines, re-add them at the appropriate spots (taking care to use the original offsets, not the ones with the highlighting added in). > > All the more reason to try to move this inside diff.c, I guess. > > That too, probably. Hmm, I thought that would solve all my problems by operating on the pre-color version without much more work. But... > If we were to do this, I think it may make sense to separate the > logic to compute which span of the string need to be painted in what > color and the routine to actually emit the colored output. That is, > instead of letting ws-check-emit to decide which part should be in > what color _and_ emitting the result, we should have a helper that > reads , and give us an array of spans to flag as > whitespace violation. Then an optional diff-highlight code would > scan the same (or a collection of ) without > getting confused by the whitespace errors and annotate the spans to > be highlighted. After all that happens, the output routine would > coalesce the spans and produce colored output. > > Or something like that ;-) I think this "array of spans" is the only way to go. Otherwise whichever markup scheme processes the hunk first ruins the data for the next processor. So it is a lot more work to make the two work together. The --word-diff code would have the same issue, except that I imagine it just skips whitespace-highlighting altogether. The least-work thing may actually be teaching the separate diff-highlight script to strip out the colorizing and re-add it by offset. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: `git revert` on empty commit fails silently
Alistair Lynn writes: > $ git commit --allow-empty -m ‘test’ > $ git revert HEAD > > The latter fails silently, leaving HEAD pointing at the commit created > by the first command. I do not necessarily think it is a bug to ignore reverting a no-op. "revert a no-op" should probably fail by default, and the command should require --force or --allow-empty. But I agree that silently ignoring is not good. It should warn the user, saying that reverting no-op is nonsense, when refusing the request. > A subsequent `git commit --allow-empty` has the revert message as the > default commit message when starting the editor. And leaving a populated MERGE_MSG file to be picked up by the next commit, which is an unrelated operation, is clearly wrong, I would think. If we deem the "revert a no-op" as a nonsense and ignore it, we should ignore it completely and should not leave MERGE_MSG. But leaving MERGE_MSG is internally consistent, I think. When "revert" stops due to conflicts and returns the control to the user, it would explain the situation to the user loudly, and then after user helps Git by resolving the conflict, the user uses "commit", and the message is picked up from MERGE_MSG. I'd view what you saw very similar to that situation. Instead of seeing a conflict (with which the command cannot automatically continue), the command saw a "no-op" (which it is dubious that the user really meant to revert). Asking the user to help and then allowing the user to signal that s/he is now done with "git commit" is the right way to continue, and for that to work seamlessly, the message has to be in MERGE_MSG. So perhaps the only buggy part of this whole experience is that the command "silently" failed, instead of explaining the situation (i.e. "No changes to revert"); in case the user still does want to commit the revert of no-op by "commit --allow-empty", it did the right thing by leaving the message in MERGE_MSG to be picked up later. I dunno. > Hope this is the right place for bugs. Yes, this is the right place for bugs. 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Jeff King writes: > ... I think you could also argue that > because of whitespace-highlighting, colorized diffs are fundamentally > going to have colors intermingled with the content and should not be > parsed this way. Painting of whitespace breakages is asymmetric [*1*]. If you change something on a badly indented line without fixing the indentation, e.g. -hello-world +hello-people only the space-before-tab on the latter line is painted. For the purpose of your diff highlighting, however, you would want to treat the leading "hello-" on preimage and postimage lines as unchanged. Which means that you shouldn't be reading a colored output without ignoring the color-control sequences. So I think you arrived at the correct conclusion. > All the more reason to try to move this inside diff.c, I guess. That too, probably. If we were to do this, I think it may make sense to separate the logic to compute which span of the string need to be painted in what color and the routine to actually emit the colored output. That is, instead of letting ws-check-emit to decide which part should be in what color _and_ emitting the result, we should have a helper that reads , and give us an array of spans to flag as whitespace violation. Then an optional diff-highlight code would scan the same (or a collection of ) without getting confused by the whitespace errors and annotate the spans to be highlighted. After all that happens, the output routine would coalesce the spans and produce colored output. Or something like that ;-) [Footnote] *1* We recently added a knob to allow us paint them on preimage and common lines, too, but that is not the default. -- 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: co-authoring commits
On Thu, Jun 18, 2015 at 11:25:44PM +0200, Jakub Narębski wrote: > Author and committer include datetime in the contents of the > field, which is used by Git for heuristics limiting walk. Coauthor > would have the same date as author, isn't it? If, after long > and involved discussion, we didn't add 'generation' field (for > easier cutting history walking), what chance adding 'coauthor' > has. I don't think the two situations are comparable. I would (and did) argue that a "generation" field is a bad header to bake in because of what it means (it is redundant with the graph structure). Whereas "co-author" is not a fundamentally bad; it's just not something we chose to support early on, and it would have to be added now. > OTOH it would be nice to have support for .mailmap, and for > grepping... but the former could conceivably be added to the trailer > tool, the latter can be done with appropriate regexp in > "git log --grep=...". I don't think we munge trailers during "git log" pretty-printing at all now, but it is certainly something we could add (including mailmap-ing them). That doesn't seem like much more work than showing the co-author field, and it's a lot more generally applicable (you could mailmap S-O-B, Reviewed-by, and so forth). Similarly, something like "git shortlog" would have to learn about multiple authors under the "co-author" scheme. But likewise, it would not be much more work to teach it something like: git shortlog --field=Reviewed-by to handle an arbitrary trailer. And that is much more flexible. > I wonder what would break if one used 'Name , Name ' > as the author... The "normal" parser we use for pretty-printing goes left-to-right and will stop at the first ">", and show only the first author. Older versions of git would then get the date wrong, complaining about the ",". Newer versions parse the date from right-to-left to work around such bogosities (especially things like ">") and so will parse back to the second ">". Fsck will definitely complain about it. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] format-patch: introduce format.outputDirectory configuration
On Thu, Jun 18, 2015 at 02:46:36PM -0700, Junio C Hamano wrote:
> > If I were designing from scratch, I would consider making "-o -" output
> > to stdout, and letting it override a previous "-o" (or vice versa). We
> > could still do that (and make "--stdout" an alias for that), but I don't
> > know if it is worth the trouble (it does change the behavior for anybody
> > who wanted a directory called "-", but IMHO it is more likely to save
> > somebody a headache than create one).
>
> I agree with "later -o should override an earlier one", but I do not
> necessarily agree with "'-o -' should be --stdout", for a simple
> reason that "-o foo" is not "--stdout >foo".
Good point. At any rate, that was all in my "designing from scratch"
hypothetical, so it is doubly not worth considering.
> Perhaps something like this to replace builtin/ part of Alexander's
> patch?
> [...]
> @@ -1337,6 +1342,9 @@ int cmd_format_patch(int argc, const char **argv, const
> char *prefix)
> die (_("--subject-prefix and -k are mutually exclusive."));
> rev.preserve_subject = keep_subject;
>
> + if (!output_directory && !use_stdout)
> + output_directory = config_output_directory;
> +
Yeah, I think that is the sanest way to do it given the constraints.
-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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 05:23:56PM -0400, Jeff King wrote:
> +# Return the individual diff-able items of the hunk, but with any
> +# diff or color prefix/suffix for each line split out (we assume that the
> +# prefix/suffix for each line will be the same).
> +sub split_hunk {
> + my ($prefix, $suffix, @r);
> + foreach my $line (@_) {
> + $line =~ /^($COLOR*[+-]$COLOR*)(.*)($COLOR*)/
> + or die "eh, this is supposed to match everything!";
This isn't quite right. We'll never match the suffix, because it gets
sucked up by the greedy (.*). But making that non-greedy matches
nothing, unless we also add "$" at the end.
_But_, there is still something else weird going on. I replaced this
split:
> + push @r, split(//, $2), "\n";
with:
push @r, split(/([[:space:][:punct:]])/, $2), "\n";
which behaves much better. With that, 99a2cfb shows:
-(!peel_ref(path, peeled)) {
- _annotated = !!(, peeled);
+(!peel_ref(path, peeled<.hash>)) {
+ _annotated = !!(, <&>peeled);
The latter half of both lines looks perfect. But what's that weird
highlighting of the initial "if" and "is" on those lines?
It turns out that the colored output we produce is quite odd:
$ git show --color 99a2cfb | grep peel_ref | cat -A
^[[31m-^Iif (!peel_ref(path, peeled)) {^[[m$
^[[32m+^[[m^I^[[32mif (!peel_ref(path, peeled.hash)) {^[[m$
For the pre-image, we print the color, the "-", and then the line data.
Makes sense.
For the post-image, we print the color, "+", a reset, then the initial
whitespace, then the color again!
So of course the diff algorithm says "hey, there's this weird color in
here". The original implementation of diff-highlight didn't care,
because it skipped leading whitespace and colors as "boring". But this
one cannot do that. It pulls the strict prefix out of each line (and it
must, because it must get the same prefix for each line of a hunk).
I think git is actually wrong here; it's mingling the ANSI colors and
the actual content. Nobody ever noticed because it looks OK to a human,
and who would be foolish enough to try machine-parsing colorized diff
output?
The fix is:
diff --git a/diff.c b/diff.c
index 87b16d5..a80b5b4 100644
--- a/diff.c
+++ b/diff.c
@@ -501,9 +501,9 @@ static void emit_line_checked(const char *reset,
emit_line_0(ecbdata->opt, ws, reset, sign, line, len);
else {
/* Emit just the prefix, then the rest. */
- emit_line_0(ecbdata->opt, set, reset, sign, "", 0);
+ emit_line_0(ecbdata->opt, set, "", sign, "", 0);
ws_check_emit(line, len, ecbdata->ws_rule,
- ecbdata->opt->file, set, reset, ws);
+ ecbdata->opt->file, "", reset, ws);
}
}
But I'm a little worried it may interfere with the way the
whitespace-checker emits colors (e.g., it may emit its own colors for
the leading spaces, and we would need to re-assert our color before
showing the rest of the line). So I think you could also argue that
because of whitespace-highlighting, colorized diffs are fundamentally
going to have colors intermingled with the content and should not be
parsed this way.
All the more reason to try to move this inside diff.c, I guess.
-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 v5 00/19] Introduce an internal API to interact with the fsck machinery
Hi Junio, On 2015-06-19 00:11, Junio C Hamano wrote: > I haven't had a chance to go through the all the patches, but one > thing I noticed that did not appear in the interdiff is that some of > the message IDs are unclear. For example, there are BAD_something, > INVALID_something and MISSING_something. The last one is in a > different category and is good, but how are the former two > differenciated? Do they follow some systematic rules, or they are > named after the way how they happened to be reported in the original > textual error message? I basically made up names on the go, based on the messages. > Some of the questionable groups are: > > BAD_DATE DATE_OVERFLOW I guess it should be BAD_DATE_OVERFLOW to be more consistent? > BAD_TREE_SHA1 INVALID_OBJECT_SHA1 INVALID_TREE > > BAD_PARENT_SHA1 INVALID_OBJECT_SHA1 So how about s/INVALID_/BAD_/g? > Also it is unclear if NOT_SORTED is to be used ever for any error > other than a tree object sorted incorrectly, or if we start noticing > a new error that something is not sorted, we will reuse this one. s/NOT_SORTED/TREE_&/ maybe? > I also briefly wondered if fsck.skipList should be finer grained > than "these are know to be broken, do not bother reporting problems > with them" (e.g. I know v0.99 lacks "tagger" so I want to squelch > MISSING_TAGGER_ENTRY for it, but I want to be notified on any other > errors). But that only matters if we update Git to a version with a > new fsck that knows yet more kinds of breakages, so it is not a huge > issue, and the simplicity of "be silent on these objects" is > probably better overall. Well, the idea of skiplist is to say: "I have inspected this object and determined that errors in it should be ignored." As such, it does not really matter what problems future Git versions report because the person populating the skiplist is supposed to test thoroughly, not just asking `git fsck` what is going on. And yes, the motivation for this feature is to keep it super-simple. ;-) 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, 18 Jun 2015, Jeff King wrote:
On Thu, Jun 18, 2015 at 04:14:19PM -0400, Patrick Palka wrote:
in a test script becomes more clear. But some of the output is not so
great. For instance, the very commit under discussion has a
confusing and useless highlight. Or take a documentation patch like
5c31acfb, where I find the highlights actively distracting. We are saved
a little by the "if the whole line is different, do not highlight at
all" behavior of 097128d1bc.
To fix the useless highlights for both evenly and unevenly sized hunks
(like when all but a semicolon on a line changes), one can loosen the
criterion for not highlighting from "do not highlight if 0% of the
before and after lines are common between them" to, say, "do not
highlight if less than 10% of the before and after lines are common
between them". Then most of these useless highlights are gone for both
evenly and unevenly sized hunks.
Yeah, this is an idea I had considered but never actually experimented
with. It does make some things better, but it also makes some a little
worse. For example, in 8dbf3eb, the hunk:
- const char *plain = diff_get_color(ecb->color_diff,
- DIFF_PLAIN);
+ const char *context = diff_get_color(ecb->color_diff,
+DIFF_CONTEXT);
currently gets the plain/context change in the first line highlighted,
as well as the DIFF_PLAIN/DIFF_CONTEXT in the second line. With a 10%
limit, the second line isn't highlighted. That's correct by the
heuristic, but it's a bit harder to read, because the highlight draws
your eye to the first change, and it is easy to miss the second.
Good example, this actually exposes a "bug" in the heuristic. Each line
is around 15 characters long and the common affix ");" is 2 characters
long, which is about 15% of 15. So the DIFF_PLAIN/DIFF_CONTEXT pair
ought to be highlighted.
The patch was unintentionally comparing the lengths of the common
affixes against the length of the entire line, whitespace and color
codes and all. In effect this meant that the 10% threshold was much
higher. We should compare against the length of the non-boring parts of
the whole line when determining the percentage in common. Attached is a
revised patch.
Still, I think this is probably a minority case, and it may be
outweighed by the improvements. The "real" solution is to consider the
hunk as a whole and do an LCS diff on it, which would show that yes,
it's worth highlighting both of those spots, as they are a small
percentage of the total hunk.
Here is a patch that changes the criterion as mentioned. Testing this
change on the documentation patch 5c31acfb, only two pairs of lines are
highlighted instead of six. On my original patch, the useless highlight
is gone. The useless semicolon-related highlights on e.g. commit
99a2cfb are gone.
Nice, the ones like 99a2cfb are definitely wrong (I had though to fix
them eventually by treating some punctuation as uninteresting, but I
suspect the percentage heuristic covers that reasonably well in
practice).
Of course, these patches are both hacks but they seem to be surprisingly
effective hacks especially when paired together.
The whole script is a (surprisingly effective) hack. ;)
So I dunno. IMHO this does more harm than good, and I would not want to
use it myself. But it is somewhat a matter of taste; I am not opposed to
making it a configurable option.
That is something I can do :)
Coupled with the 10%-threshold patch, I think it would be OK to include
it unconditionally. So far we've just been diffing the two outputs and
micro-analyzing them. The real test to me will be using it in practice
and seeing if it's helpful or annoying.
-- >8 --
Subject: [PATCH] diff-highlight: don't highlight lines that have little in
common
---
contrib/diff-highlight/diff-highlight | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index 85d2eb0..0cc2b60 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -218,8 +218,21 @@ sub is_pair_interesting {
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
my $suffix_b = join('', @$b[($sb+1)..$#$b]);
- return $prefix_a !~ /^$COLOR*-$BORING*$/ ||
- $prefix_b !~ /^$COLOR*\+$BORING*$/ ||
- $suffix_a !~ /^$BORING*$/ ||
- $suffix_b !~ /^$BORING*$/;
+ $prefix_a =~ s/^$COLOR*-$BORING*//;
+ $prefix_b =~ s/^$COLOR*\+$BORING*//;
+ $suffix_a =~ s/$BORING*$//;
+ $suffix_b =~ s/$BORING*$//;
+
+ my $whole_a = join ('', @$a);
+ $whole_a =~ s/^$COLOR*-$BORING*//;
+ $whole_a =~ s/$BORING*$//;
+
+ my $whole_b = join ('', @$b);
+ $whole_b =~ s/^$COLOR*\+$BORING*//;
+ $whole_b =~ s/$BORING*$//;
+
+ # Only bother highlighti
Re: BUG: checkout won't checkout?
On 18 June 2015 at 23:53, Junio C Hamano wrote: > Luke Diamand writes: > $ git checkout upstream/master -- subtree $ git diff upstream/master -- subtree -- still lots of deltas >>> >>> Does this show _ONLY_ additions? Or does it include modifications >>> and removals? >> >> There are indeed _ONLY_ additions. > > http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234912 > http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234924 > > In short, it is an intended behaviour, both Peff and I consider that > the intention is bad and the behaviour should be changed. OK, thanks, it makes sense now! > > But nothing has happened yet (it is listed as one of the "leftover > bits" http://git-blame.blogspot.com/p/leftover-bits.html). OK, I'll keep it in mind. Luke -- 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: Using clean/smudge filters with difftool
John Keeping writes: > I think the summary is that there are some scenarios where the external > diff tool should see the smudged version and others where the clean > version is more appropriate and Git should support both options. It > seems this is a property of the filter, so I wonder if the best solution > is a new "filter..extdiff = [clean|smudge]" configuration > variable (there's probably a better name for the variable than > "extdiff"). Not just the external diff, but the textconv filter obeys the same rule. The setting should be done the same way for both, if we are going to go in that direction. -- 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: checkout won't checkout?
Luke Diamand writes: >>> $ git checkout upstream/master -- subtree >>> $ git diff upstream/master -- subtree >>> -- still lots of deltas >> >> Does this show _ONLY_ additions? Or does it include modifications >> and removals? > > There are indeed _ONLY_ additions. http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234912 http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234924 In short, it is an intended behaviour, both Peff and I consider that the intention is bad and the behaviour should be changed. But nothing has happened yet (it is listed as one of the "leftover bits" http://git-blame.blogspot.com/p/leftover-bits.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: BUG: checkout won't checkout?
On 18 June 2015 at 23:28, Junio C Hamano wrote: > Luke Diamand writes: > >> This is probably user error, but I'm not sure what I'm doing wrong. >> I'm posting here in case anyone else gets the same thing >> >> I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today. >> >> I've somehow ended up with history skipping back in time, but git not >> prepared to let let me fix it, or something. >> >> $ git diff upstream/master -- subtree >> - lots of deltas, which look like I've reverted this back several >> revisions (which I haven't AFAIK) > > Are you on the right branch that you think you are working on? Yes. > >> $ git checkout upstream/master -- subtree >> $ git diff upstream/master -- subtree >> -- still lots of deltas > > Does this show _ONLY_ additions? Or does it include modifications > and removals? There are indeed _ONLY_ additions. -- 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: Using clean/smudge filters with difftool
On Thu, Jun 18, 2015 at 01:00:36PM -0700, Junio C Hamano wrote: > John Keeping writes: > > > I think this is a difference between git-diff's internal and external > > diff modes which is working correctly, although possibly not desirably > > in this case. The internal diff always uses clean files (so it runs the > > working tree file through the "clean" filter before applying the diff > > algorithm) but the external diff uses the working tree file so it > > applies the "smudge" filter to any blobs that it needs to checkout. > > > > Commit 4e218f5 (Smudge the files fed to external diff and textconv, > > 2009-03-21) was the source of this behaviour. > > The fundamental design to use smudged version when interacting with > external programs actually predates that particular commit, I think. > > The caller of the function that was updated by that commit, i.e. > prepare_temp_file(), reuses what is checked out to the working tree > when we can (i.e. it hasn't been modified from what we think is > checked out) and when it is beneficial to do so (i.e. on a system > with FAST_WORKING_DIRECTORY defined), which means the temporary file > given by the prepare_temp_file() that is used by the external tools > (both --ext-diff program and textconv filter) are designed to be fed > and work on the smudged version of the file. 4e218f5 did not change > that fundamental design; it just made things more consistent between > the case where we do create a new temporary file out of blob and we > allow an unmodified checked out file to be reused. When I started looking at this, I assumed the problem would be that git-difftool wasn't smudging the non-working-tree files. But actually everything is working "correctly", I'm just not sure it's always what the user wants (at least it isn't what was wanted in this case). Currently, the behaviour is: internal diff: compare clean files external diff: compare smudged files This makes sense for LF/CRLF conversion, where platform-specific tools clearly want the platform's line ending but the internal diff machinery doesn't care. However, from the filter description in an earlier email, I think Florian is using a clean filter to remove output from IPython notebook files (it seems that IPython saves both the input and the output in the same file [1] and the output is the equivalent of, for example, C object files). In this case, the filter is one-way and discards information from the working tree file, producing a smaller and more readable diff in the process. I think the summary is that there are some scenarios where the external diff tool should see the smudged version and others where the clean version is more appropriate and Git should support both options. It seems this is a property of the filter, so I wonder if the best solution is a new "filter..extdiff = [clean|smudge]" configuration variable (there's probably a better name for the variable than "extdiff"). [1] http://pascalbugnion.net/blog/ipython-notebooks-and-git.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: BUG: checkout won't checkout?
Junio C Hamano writes: >> $ git checkout upstream/master -- subtree >> $ git diff upstream/master -- subtree >> -- still lots of deltas > > Does this show _ONLY_ additions? Or does it include modifications > and removals? The reason I ask this question is because of http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234924 -- 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: checkout won't checkout?
Luke Diamand writes: > This is probably user error, but I'm not sure what I'm doing wrong. > I'm posting here in case anyone else gets the same thing > > I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today. > > I've somehow ended up with history skipping back in time, but git not > prepared to let let me fix it, or something. > > $ git diff upstream/master -- subtree > - lots of deltas, which look like I've reverted this back several > revisions (which I haven't AFAIK) Are you on the right branch that you think you are working on? > $ git checkout upstream/master -- subtree > $ git diff upstream/master -- subtree > -- still lots of deltas Does this show _ONLY_ additions? Or does it include modifications and removals? -- 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 5:23 PM, Jeff King wrote: > On Thu, Jun 18, 2015 at 04:45:05PM -0400, Jeff King wrote: > >> Still, I think this is probably a minority case, and it may be >> outweighed by the improvements. The "real" solution is to consider the >> hunk as a whole and do an LCS diff on it, which would show that yes, >> it's worth highlighting both of those spots, as they are a small >> percentage of the total hunk. > > I've been meaning to play with this for years, so I took the opportunity > to spend a little time on it. :) Cool! > > Below is a (slightly hacky) patch I came up with. It seems to work, and > produces really great output in some cases. For instance, in 99a2cfb, it > produces (I put highlighted bits in angle brackets): > > - cpy(peeled, ); > + cpy(<&>peeled, ); > > It also produces nonsense like: > > - sed peeled<[20]>; > + sed peeled; That's not even so bad. The diff of the change itself is... interesting. > > but I think that is simply because my splitting function is terrible (it > splits each byte, whereas we'd probably want to use whitespace and > punctuation, or something content-specific). I hope you can polish this. It definitely has potential. -- 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: checkout won't checkout?
The other thing about these files is that they were all deleted a few weeks ago and have now come back. On 18 June 2015 at 23:07, Luke Diamand wrote: > This is probably user error, but I'm not sure what I'm doing wrong. > I'm posting here in case anyone else gets the same thing > > I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today. > > I've somehow ended up with history skipping back in time, but git not > prepared to let let me fix it, or something. > > $ git diff upstream/master -- subtree > - lots of deltas, which look like I've reverted this back several > revisions (which I haven't AFAIK) > $ git checkout upstream/master -- subtree > $ git diff upstream/master -- subtree > -- still lots of deltas > $ git checkout upstream/master -- subtree > $ git commit -m 'Revert unwanted changes' > $ git diff upstream/master -- subtree > -- still lots of deltas > > What am I doing wrong? Have I ended up in the middle of some weird > state (cherry-pick and rebase isn't in progress AFAIK). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/19] Introduce an internal API to interact with the fsck machinery
Johannes Schindelin writes: > At the moment, the git-fsck's integrity checks are targeted toward the > end user, i.e. the error messages are really just messages, intended for > human consumption. > > Under certain circumstances, some of those errors should be allowed to > be turned into mere warnings, though, because the cost of fixing the > issues might well be larger than the cost of carrying those flawed > objects. > Interdiff below the diffstat. It's huge. Sorry. Heh, no need to say sorry, though. A large interdiff means you did a lot more work, after all. I haven't had a chance to go through the all the patches, but one thing I noticed that did not appear in the interdiff is that some of the message IDs are unclear. For example, there are BAD_something, INVALID_something and MISSING_something. The last one is in a different category and is good, but how are the former two differenciated? Do they follow some systematic rules, or they are named after the way how they happened to be reported in the original textual error message? Some of the questionable groups are: BAD_DATE DATE_OVERFLOW BAD_TREE_SHA1 INVALID_OBJECT_SHA1 INVALID_TREE BAD_PARENT_SHA1 INVALID_OBJECT_SHA1 Also it is unclear if NOT_SORTED is to be used ever for any error other than a tree object sorted incorrectly, or if we start noticing a new error that something is not sorted, we will reuse this one. I also briefly wondered if fsck.skipList should be finer grained than "these are know to be broken, do not bother reporting problems with them" (e.g. I know v0.99 lacks "tagger" so I want to squelch MISSING_TAGGER_ENTRY for it, but I want to be notified on any other errors). But that only matters if we update Git to a version with a new fsck that knows yet more kinds of breakages, so it is not a huge issue, and the simplicity of "be silent on these objects" is probably better overall. 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
BUG: checkout won't checkout?
This is probably user error, but I'm not sure what I'm doing wrong. I'm posting here in case anyone else gets the same thing I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today. I've somehow ended up with history skipping back in time, but git not prepared to let let me fix it, or something. $ git diff upstream/master -- subtree - lots of deltas, which look like I've reverted this back several revisions (which I haven't AFAIK) $ git checkout upstream/master -- subtree $ git diff upstream/master -- subtree -- still lots of deltas $ git checkout upstream/master -- subtree $ git commit -m 'Revert unwanted changes' $ git diff upstream/master -- subtree -- still lots of deltas What am I doing wrong? Have I ended up in the middle of some weird state (cherry-pick and rebase isn't in progress AFAIK). -- 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] format-patch: introduce format.outputDirectory configuration
Jeff King writes:
> Much worse, though, is that we also have to interact with "--stdout". We
> currently treat "--stdout -o foo" as an error; you need a separate
> config_output_directory to continue to handle that (and allow "--stdout"
> to override the config).
>
> If I were designing from scratch, I would consider making "-o -" output
> to stdout, and letting it override a previous "-o" (or vice versa). We
> could still do that (and make "--stdout" an alias for that), but I don't
> know if it is worth the trouble (it does change the behavior for anybody
> who wanted a directory called "-", but IMHO it is more likely to save
> somebody a headache than create one).
I agree with "later -o should override an earlier one", but I do not
necessarily agree with "'-o -' should be --stdout", for a simple
reason that "-o foo" is not "--stdout >foo".
Perhaps something like this to replace builtin/ part of Alexander's
patch?
builtin/log.c | 8
1 file changed, 8 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index e67671e..e022d62 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -682,6 +682,8 @@ enum {
COVER_AUTO
};
+static const char *config_output_directory;
+
static int git_format_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "format.headers")) {
@@ -752,6 +754,9 @@ static int git_format_config(const char *var, const char
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON :
COVER_OFF;
return 0;
}
+ if (!strcmp(var, "format.outputdirectory")) {
+ return git_config_string(&config_output_directory, var, value);
+ }
return git_log_config(var, value, cb);
}
@@ -1337,6 +1342,9 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
die (_("--subject-prefix and -k are mutually exclusive."));
rev.preserve_subject = keep_subject;
+ if (!output_directory && !use_stdout)
+ output_directory = config_output_directory;
+
argc = setup_revisions(argc, argv, &rev, &s_r_opt);
if (argc > 1)
die (_("unrecognized argument: %s"), argv[1]);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Jeff King writes: > On Thu, Jun 18, 2015 at 04:45:05PM -0400, Jeff King wrote: > >> Still, I think this is probably a minority case, and it may be >> outweighed by the improvements. The "real" solution is to consider the >> hunk as a whole and do an LCS diff on it, which would show that yes, >> it's worth highlighting both of those spots, as they are a small >> percentage of the total hunk. > > I've been meaning to play with this for years, so I took the opportunity > to spend a little time on it. :) I envy you ;-) It certainly looks like a fun side project. > Below is a (slightly hacky) patch I came up with. It seems to work, and > produces really great output in some cases. For instance, in 99a2cfb, it > produces (I put highlighted bits in angle brackets): > > - cpy(peeled, ); > + cpy(<&>peeled, ); > > It also produces nonsense like: > > - sed peeled<[20]>; > + sed peeled; ROTFL ;-) And the change does not look bad at all. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v3 10/31] am: refresh the index at start
Paul Tan writes:
> If a file is unchanged but stat-dirty, git-apply may erroneously fail to
> apply patches, thinking that they conflict with a dirty working tree.
>
> As such, since 2a6f08a (am: refresh the index at start and --resolved,
> 2011-08-15), git-am will refresh the index before applying patches.
> Re-implement this behavior.
Good.
I would actually have expected to see this as part of 08/31, though.
>
> Signed-off-by: Paul Tan
> ---
> builtin/am.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index dfb6f7e..a7efe85 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -13,6 +13,7 @@
> #include "cache-tree.h"
> #include "refs.h"
> #include "commit.h"
> +#include "lockfile.h"
>
> /**
> * Returns 1 if the file is empty or does not exist, 0 otherwise.
> @@ -471,6 +472,20 @@ static const char *msgnum(const struct am_state *state)
> }
>
> /**
> + * Refresh and write index.
> + */
> +static void refresh_and_write_cache(void)
> +{
> + static struct lock_file lock_file;
> +
> + hold_locked_index(&lock_file, 1);
> + refresh_cache(REFRESH_QUIET);
> + if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
> + die(_("unable to write index file"));
> + rollback_lock_file(&lock_file);
> +}
> +
> +/**
> * Parses `patch` using git-mailinfo. state->msg will be set to the patch
> * message. state->author_name, state->author_email, state->author_date will
> be
> * set to the patch author's name, email and date respectively. The patch's
> @@ -607,6 +622,8 @@ static void do_commit(const struct am_state *state)
> */
> static void am_run(struct am_state *state)
> {
> + refresh_and_write_cache();
> +
> while (state->cur <= state->last) {
> const char *patch = am_path(state, msgnum(state));
>
> @@ -696,6 +713,9 @@ int cmd_am(int argc, const char **argv, const char
> *prefix)
>
> argc = parse_options(argc, argv, prefix, am_options, am_usage, 0);
>
> + if (read_index_preload(&the_index, NULL) < 0)
> + die(_("failed to read the index"));
> +
> if (am_in_progress(&state))
> am_load(&state);
> else {
--
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/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Matthieu Moy writes: > Cool. Then almost all the work is done to get an automated test. Next > step would be to add the tests itself in the code. I would do that by > adding a hidden --selfcheck option to git send-email that would compare > Mail::Address->parse($string); and split_addrs($string); for all your > testcases, and die if they do not match. Then calling it from the > testsuite would be trivial. Ok, are there such "--selfcheck" options elsewhere? If I understand it right, you want to put the tests inside the git-send-email script. I don't feel really good about that but I guess it's hard to test it otherwise... Also what will we do with the failing tests? Just discard them? I think there's two sort of failing test: - When output provided by parse_address_ without Mail::Address is better or has no impact at all on the code. Such as: Input: "Doe, Ja"ne Split: "Doe, Ja ne" M::A : "Doe, Ja" ne This output is done on purpose. If it was the same output with Mail::Address, we could have avoided commit 8/9 of this serie btw. I think we should also test these cases. - When we don't really care about the output, because the user entry is wrong, and we just expect the script to be aborted somehow... We don't need to test that. We could also add an option to specify whether we want to use Mail::Address or not and do the tests in t9001* (but this would take much more time). > I can do that on top of your series if you don't have time. Time will become a problem soon, but I think I can handle it unless you really want to do it ! -- 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: co-authoring commits
Junio C Hamano wrote: j...@joshtriplett.org writes: Author and committer are used by many git tools; if they weren't part of the object header, they'd need to be part of some pseudo-header with a standardized format that git can parse. Yes, the same goes to the address on Signed-off-by: footers. There recently was a series to enhance the footer list handling (Christian Cc'ed) for the generation and maintenance side, and I do think it is reasonable to further add enhanced support for footers. That does not argue for having a new "coauthor" as a new commit object header at all, though. The threshold for modifying commit object is high. This is an ABI-level change, something to do if there is no other solution. Author and committer include datetime in the contents of the field, which is used by Git for heuristics limiting walk. Coauthor would have the same date as author, isn't it? If, after long and involved discussion, we didn't add 'generation' field (for easier cutting history walking), what chance adding 'coauthor' has. OTOH it would be nice to have support for .mailmap, and for grepping... but the former could conceivably be added to the trailer tool, the latter can be done with appropriate regexp in "git log --grep=...". I wonder what would break if one used 'Name , Name ' as the author... -- Jakub Narębski -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: co-authoring commits
On Thu, Jun 18, 2015 at 12:52 AM, Theodore Ts'o wrote: > On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: > > > > By allowing multiple authors, you don't have to decide who's the > > primary author, as in such situations usually there is no primary > > at all. I sometimes deliberately override the author when > > committing and add myself just as another co-author in the commit > > message, but as others have noted it would be really great if we > > can just specify multiple authors. > > Just recently, there a major thread on the IETF mailing list where > IETF working group had drafts where people were listed as co-authors > without their permission, and were upset that the fact that their > name was added made it seem as if they agreed with the end product. > (i.e., that they were endorsing the I-D). So while adding formal > coauthor might solves (a few) problems, it can also introduce > others. You can misuse signed-off/reviewed-by/etc the same way. > Ultimately there is one person who can decide which parts of the > changes to put in the commit that gets sent to the maintainer. So > there *is* someone who is the primary author; the person who takes > the final pass on the patch and then hits the send key. If you (do it in isolation and) want to take full responsibility, yes, but I consider reviewed-by/signed-off as taking partial responsibility because it's a vetting process. > One could imagine some frankly, quite rare example where there is a > team of people who votes on each commit before it gets sent out and > where everyone is equal and there is no hierarchy. In that case, > perhaps you could set the from field to a mailing list address. But > honestly, how often is that *all* of the authors are completely > equal[1]? For that case something like patchwork, phabricator, or gerrit seems to be the logical tool to use, and should ideally leave a trace of approvals and such in the resulting commit message(s). If the patch management tool takes care of merging the commit(s), it can be harder to misattribute signed-off/reviewed-by/etc, which is a good thing. > In my personal practice, if I make significant changes to a patch, I > will indeed simply change the submitter, and then give credit the > original author. This is the case where I'm essentially saying, "Bob > did a lot of work, but I made a bunch of changes, so if things break > horribly, blame *me*, not Bob". > > Alternatively, if I just need to make a few cosmetic changes to > Alice's patch (i.e., fix white spaces, correct spelling, change the > commit description so it's validly parsable and understandable > English, etc.), I'll just add a comment in square brackets > indicating what changes I made before I committed the change. This > seems to work just fine, and I don't think we should try to fix > something that isn't broken. Perfectly valid use cases, but different from the scenarios Josh mentioned. You could of course use multiple (everybody makes their own) commits, where you risk breaking bisectability and avoid the need for equal co-authorship support. In pair programming such intermediate commits will quite often be fixups, and when you attempt to squash the fixups for bisectability's sake, you may get a desire for co-authorship of the resulting commit. -- 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 04:45:05PM -0400, Jeff King wrote:
> Still, I think this is probably a minority case, and it may be
> outweighed by the improvements. The "real" solution is to consider the
> hunk as a whole and do an LCS diff on it, which would show that yes,
> it's worth highlighting both of those spots, as they are a small
> percentage of the total hunk.
I've been meaning to play with this for years, so I took the opportunity
to spend a little time on it. :)
Below is a (slightly hacky) patch I came up with. It seems to work, and
produces really great output in some cases. For instance, in 99a2cfb, it
produces (I put highlighted bits in angle brackets):
- cpy(peeled, );
+ cpy(<&>peeled, );
It also produces nonsense like:
- sed peeled<[20]>;
+ sed peeled;
but I think that is simply because my splitting function is terrible (it
splits each byte, whereas we'd probably want to use whitespace and
punctuation, or something content-specific).
---
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index ffefc31..7165518 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -3,6 +3,7 @@
use 5.008;
use warnings FATAL => 'all';
use strict;
+use Algorithm::Diff;
# Highlight by reversing foreground and background. You could do
# other things like bold or underline if you prefer.
@@ -88,131 +89,54 @@ sub show_hunk {
return;
}
- # If we have mismatched numbers of lines on each side, we could try to
- # be clever and match up similar lines. But for now we are simple and
- # stupid, and only handle multi-line hunks that remove and add the same
- # number of lines.
- if (@$a != @$b) {
- print @$a, @$b;
- return;
- }
-
- my @queue;
- for (my $i = 0; $i < @$a; $i++) {
- my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
- print $rm;
- push @queue, $add;
- }
- print @queue;
-}
-
-sub highlight_pair {
- my @a = split_line(shift);
- my @b = split_line(shift);
+ my ($prefix_a, $suffix_a, @hunk_a) = split_hunk(@$a);
+ my ($prefix_b, $suffix_b, @hunk_b) = split_hunk(@$b);
- # Find common prefix, taking care to skip any ansi
- # color codes.
- my $seen_plusminus;
- my ($pa, $pb) = (0, 0);
- while ($pa < @a && $pb < @b) {
- if ($a[$pa] =~ /$COLOR/) {
- $pa++;
- }
- elsif ($b[$pb] =~ /$COLOR/) {
- $pb++;
- }
- elsif ($a[$pa] eq $b[$pb]) {
- $pa++;
- $pb++;
- }
- elsif (!$seen_plusminus && $a[$pa] eq '-' && $b[$pb] eq '+') {
- $seen_plusminus = 1;
- $pa++;
- $pb++;
- }
- else {
- last;
- }
- }
+ my $diff = Algorithm::Diff->new(\@hunk_a, \@hunk_b);
+ my (@out_a, @out_b);
+ while ($diff->Next()) {
+ my $bits = $diff->Diff();
- # Find common suffix, ignoring colors.
- my ($sa, $sb) = ($#a, $#b);
- while ($sa >= $pa && $sb >= $pb) {
- if ($a[$sa] =~ /$COLOR/) {
- $sa--;
- }
- elsif ($b[$sb] =~ /$COLOR/) {
- $sb--;
- }
- elsif ($a[$sa] eq $b[$sb]) {
- $sa--;
- $sb--;
- }
- else {
- last;
- }
- }
+ push @out_a, $OLD_HIGHLIGHT[1] if $bits & 1;
+ push @out_a, $diff->Items(1);
+ push @out_a, $OLD_HIGHLIGHT[2] if $bits & 1;
- if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) {
- return highlight_line(\@a, $pa, $sa, \@OLD_HIGHLIGHT),
- highlight_line(\@b, $pb, $sb, \@NEW_HIGHLIGHT);
- }
- else {
- return join('', @a),
- join('', @b);
+ push @out_b, $NEW_HIGHLIGHT[1] if $bits & 2;
+ push @out_b, $diff->Items(2);
+ push @out_b, $NEW_HIGHLIGHT[2] if $bits & 2;
}
-}
-sub split_line {
- local $_ = shift;
- return utf8::decode($_) ?
- map { utf8::encode($_); $_ }
- map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR+)/ :
- map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR+)/;
+ output_split_hunk($prefix_a, $suffix_a, @out_a);
+ output_split_hunk($prefix_b, $suffix_b, @out_b);
}
-sub highlight_line {
- my ($line, $prefix, $suffix, $theme) = @_;
-
- my $start = join('', @{$line}[0..($prefix
Re: [PATCH/WIP v3 08/31] am: apply patch with git-apply
Paul Tan writes:
> Implement applying the patch to the index using git-apply.
>
> Signed-off-by: Paul Tan
> ---
> builtin/am.c | 57 -
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index d6434e4..296a5fc 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -27,6 +27,18 @@ static int is_empty_file(const char *filename)
> return !st.st_size;
> }
>
> +/**
> + * Returns the first line of msg
> + */
> +static const char *firstline(const char *msg)
> +{
> + static struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_reset(&sb);
> + strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg);
> + return sb.buf;
> +}
Hmm. This is not wrong per-se but a more efficient way to do it may
be to have a helper function that returns a bytecount of the first
line of the msg, i.e. strchrnul(msg, '\n') - msg. Then a caller can
do
printf("Applying: %.*s", linelen(msg), msg);
instead of
printf("Applying: %s", firstline(msg));
relying on that the firstline() copies the contents to a static
strbuf that does not have to be freed.
> + struct child_process cp = CHILD_PROCESS_INIT;
> +
> + cp.git_cmd = 1;
> +
> + argv_array_push(&cp.args, "apply");
> +
> + argv_array_push(&cp.args, "--index");
> +
> + argv_array_push(&cp.args, am_path(state, "patch"));
You seem to like blank lines a lot ;-) While it is a good tool to
separate different groups while grouping related things together,
these three argv-push calls are intimately related, and reads better
without blanks in between.
Looks nicely done so far...
--
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/WIP v3 07/31] am: extract patch, message and authorship with git-mailinfo
Paul Tan writes:
> + /* commit message and metadata */
> + struct strbuf author_name;
> + struct strbuf author_email;
> + struct strbuf author_date;
> + struct strbuf msg;
Same comment as "dir" in the earlier patch applies to these. If the
fields are read or computed and then kept constant, use a temporary
variable that is a strbuf to read/compute the final value, and then
detach to a "const char *" field. If they are constantly changing
and in-place updates are vital, then they can and should be strbufs,
but I do not think that is the case for these.
For example...
> +/**
> + * Saves state->author_name, state->author_email and state->author_date in
> + * `filename` as an "author script", which is the format used by git-am.sh.
> + */
> +static void write_author_script(const struct am_state *state)
> +{
> + static const char fmt[] = "GIT_AUTHOR_NAME=%s\n"
> + "GIT_AUTHOR_EMAIL=%s\n"
> + "GIT_AUTHOR_DATE=%s\n";
> + struct strbuf author_name = STRBUF_INIT;
> + struct strbuf author_email = STRBUF_INIT;
> + struct strbuf author_date = STRBUF_INIT;
> +
> + sq_quote_buf(&author_name, state->author_name.buf);
> + sq_quote_buf(&author_email, state->author_email.buf);
> + sq_quote_buf(&author_date, state->author_date.buf);
As you use a separate author_name variable here, what gets sq-quoted
that is in *state does not have to be strbuf.
The code to read is the same story:
> +static int read_shell_var(struct strbuf *value, FILE *fp, const char *key)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + char *str;
> +
> + if (strbuf_getline(&sb, fp, '\n'))
> + return -1;
> +...
> + strbuf_reset(value);
> + strbuf_addstr(value, str);
> +
> + strbuf_release(&sb);
> +
> + return 0;
> +}
As you use a separate sb strbuf variable here, there is no need for
"value" to be pointing at a strbuf; it could be "char **" that sb's
contents is detached into.
--
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/WIP v3 06/31] am: detect mbox patches
Paul Tan writes:
> +static int is_email(const char *filename)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + FILE *fp = xfopen(filename, "r");
> + int ret = 1;
> +
> + while (!strbuf_getline(&sb, fp, '\n')) {
> + const char *x;
> +
> + strbuf_rtrim(&sb);
Is this a good thing? strbuf_getline() already has stripped the LF
at the end, so you'd be treating a line with only whitespaces as if
it is a truly empty line.
I know the series is about literal translation and the script may
lose the distinction between the two, but I do not think you need
(or want) to be literally same for things like this.
Same comment applies to other uses of "trim" in this patch.
> @@ -177,6 +267,14 @@ static int split_patches(struct am_state *state, enum
> patch_format patch_format,
> static void am_setup(struct am_state *state, enum patch_format patch_format,
> struct string_list *paths)
> {
> + if (!patch_format)
> + patch_format = detect_patch_format(paths);
> +
> + if (!patch_format) {
> + fprintf_ln(stderr, _("Patch format detection failed."));
> + exit(128);
> + }
> +
> if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
> die_errno(_("failed to create directory '%s'"), state->dir.buf);
I really like the way this keeps building incrementally ;-)
The series is an enjoyable read.
--
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
Selectively clone Git submodules -- a useful feature?
Hi, AFAIK Git has two ways to clone a repository with respect to submodules: (1) Plain clone of just the repository itself: git clone git://github.com/foo/bar.git (2) Recursive clone of the repository including all its submodules: git clone --recursive git://github.com/foo/bar.git I am working on a big cross platform project and on certain platforms I don't need certain submodules. AFAIK there is no way to selectively clone only a subset of the submodules with the standard command line interface. I wonder if something like an exclude pattern for submodules would be of general interest. I imagine a call like this after a plain "clone" operation: git submodule update --init --recursive --exclude 3rdParty/Windows/* or even: git clone --recursive --exclude 3rdParty/Windows/* git://github.com/foo/bar.git Please let me know what you think. Thanks, Lars PS: I posted this question already on the Google Git group here: https://groups.google.com/forum/?fromgroups=#!topic/git-users/jyKsd45d2MA I am sorry, but I discovered this mailing list afterwards and I am not sure which one is the appropriate one. Please advise. --- https://larsxschneider.github.io/-- 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/WIP v3 05/31] am: split out mbox/maildir patches with git-mailsplit
Paul Tan writes:
> @@ -111,13 +122,69 @@ static void am_destroy(const struct am_state *state)
> }
>
> /**
> + * Splits out individual patches from `paths`, where each path is either a
> mbox
> + * file or a Maildir. Return 0 on success, -1 on failure.
> + */
"Splits" and then "Return"? Be consistent.
> +static int split_patches_mbox(struct am_state *state, struct string_list
> *paths)
> +{
> ...
> +}
Looks straightforward ;-)
> +/**
> + * parse_options() callback that validates and sets opt->value to the
> + * PATCH_FORMAT_* enum value corresponding to `arg`.
> + */
> +static int parse_opt_patchformat(const struct option *opt, const char *arg,
> int unset)
> +{
> + int *opt_value = opt->value;
> +
> + if (!strcmp(arg, "mbox"))
> + *opt_value = PATCH_FORMAT_MBOX;
> + else
> + return -1;
> + return 0;
> +}
> +
> static struct am_state state;
> +static int opt_patch_format;
>
> static const char * const am_usage[] = {
> N_("git am [options] [(|)...]"),
> @@ -156,6 +239,8 @@ static const char * const am_usage[] = {
> };
>
> static struct option am_options[] = {
> + OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
> + N_("format the patch(es) are in"), parse_opt_patchformat),
> OPT_END()
> };
Looking good ;-).
Just FYI, you do not have to make am_options[], and the variables
that are referenced from there, e.g. opt_patch_format, etc., global
variables (instead you can have them all in the function scope of
cmd_am()).
--
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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 04:14:19PM -0400, Patrick Palka wrote: > >in a test script becomes more clear. But some of the output is not so > >great. For instance, the very commit under discussion has a > >confusing and useless highlight. Or take a documentation patch like > >5c31acfb, where I find the highlights actively distracting. We are saved > >a little by the "if the whole line is different, do not highlight at > >all" behavior of 097128d1bc. > > To fix the useless highlights for both evenly and unevenly sized hunks > (like when all but a semicolon on a line changes), one can loosen the > criterion for not highlighting from "do not highlight if 0% of the > before and after lines are common between them" to, say, "do not > highlight if less than 10% of the before and after lines are common > between them". Then most of these useless highlights are gone for both > evenly and unevenly sized hunks. Yeah, this is an idea I had considered but never actually experimented with. It does make some things better, but it also makes some a little worse. For example, in 8dbf3eb, the hunk: - const char *plain = diff_get_color(ecb->color_diff, - DIFF_PLAIN); + const char *context = diff_get_color(ecb->color_diff, +DIFF_CONTEXT); currently gets the plain/context change in the first line highlighted, as well as the DIFF_PLAIN/DIFF_CONTEXT in the second line. With a 10% limit, the second line isn't highlighted. That's correct by the heuristic, but it's a bit harder to read, because the highlight draws your eye to the first change, and it is easy to miss the second. Still, I think this is probably a minority case, and it may be outweighed by the improvements. The "real" solution is to consider the hunk as a whole and do an LCS diff on it, which would show that yes, it's worth highlighting both of those spots, as they are a small percentage of the total hunk. > Here is a patch that changes the criterion as mentioned. Testing this > change on the documentation patch 5c31acfb, only two pairs of lines are > highlighted instead of six. On my original patch, the useless highlight > is gone. The useless semicolon-related highlights on e.g. commit > 99a2cfb are gone. Nice, the ones like 99a2cfb are definitely wrong (I had though to fix them eventually by treating some punctuation as uninteresting, but I suspect the percentage heuristic covers that reasonably well in practice). > Of course, these patches are both hacks but they seem to be surprisingly > effective hacks especially when paired together. The whole script is a (surprisingly effective) hack. ;) > >So I dunno. IMHO this does more harm than good, and I would not want to > >use it myself. But it is somewhat a matter of taste; I am not opposed to > >making it a configurable option. > > That is something I can do :) Coupled with the 10%-threshold patch, I think it would be OK to include it unconditionally. So far we've just been diffing the two outputs and micro-analyzing them. The real test to me will be using it in practice and seeing if it's helpful or annoying. -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/WIP v3 04/31] am: implement patch queue mechanism
Paul Tan writes:
> diff --git a/builtin/am.c b/builtin/am.c
> index dbc8836..af68c51 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -6,6 +6,158 @@
> #include "cache.h"
> #include "builtin.h"
> #include "exec_cmd.h"
> +#include "parse-options.h"
> +#include "dir.h"
> +
> +struct am_state {
> + /* state directory path */
> + struct strbuf dir;
Is this a temporary variable you will append "/patch", etc. to form
a different string to use for fopen() etc., or do you use separate
strbufs for things like that and this is only used to initialize
them?
- If the former then "dir" is a misnomer.
- If the latter, then it probably does not have to be a strbuf;
rather, it should probably be a "const char *". Unless you pass
this directly to functions that take a strbuf, such as
remove_dir_recursively(), that is.
> +/**
> + * Release memory allocated by an am_state.
> + */
Everybody else in this file seems to say "Initializes", "Returns",
"Reads", etc. While I personally prefer to use imperative
(i.e. give command to this function to "release memory allocated"),
you would want to be consistent throughout the file; "Releases
memory" would make it so.
> +/**
> + * Setup a new am session for applying patches
> + */
> +static void am_setup(struct am_state *state)
> +{
> + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
> + die_errno(_("failed to create directory '%s'"), state->dir.buf);
> +
> + write_file(am_path(state, "next"), 1, "%d", state->cur);
> +
> + write_file(am_path(state, "last"), 1, "%d", state->last);
These two lines are closely related pair; drop the blank in between.
I am tno sure if write_file() is an appropriate thing to use,
though. What happens when you get interrupted after opening the
file but before you manage to write and close? Shouldn't we be
doing the usual "write to temp, close and then rename to final"
dance? This comment applies to all the other use of write_file().
--
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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 3:08 PM, Jeff King wrote: > On Thu, Jun 18, 2015 at 12:28:58PM -0400, Patrick Palka wrote: > >> By the way, what would it take to get something like this script into >> git proper? It is IMHO immensely useful even in its current form, yet >> because it's not baked into the application hardly anybody knows about >> it. > > I think if we were going to make it more official, it would make sense > to do it inside the diff code itself (i.e., not as a separate script), > and it might be reasonable at that point to actually do a "real" > character-based diff rather than the hacky prefix/suffix thing (or > possibly even integrate with the color-words patterns to find > syntactically interesting breaks). There is some discussion in the > "Bugs" section of contrib/diff-highlight/README. > > -Peff Thanks for the pointers. This is something I am interested in implementing (though not any time soon). I was actually in the process of familiarizing myself with the diff code before I discovered the existence of diff-highlight by accident. -- 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/WIP v3 03/31] am: implement skeletal builtin am
Paul Tan writes:
> For the purpose of rewriting git-am.sh into a C builtin, implement a
> skeletal builtin/am.c that redirects to $GIT_EXEC_PATH/git-am if the
> environment variable _GIT_USE_BUILTIN_AM is not defined. Since in the
> Makefile git-am.sh takes precedence over builtin/am.c,
> $GIT_EXEC_PATH/git-am will contain the shell script git-am.sh, and thus
> this allows us to fall back on the functional git-am.sh when running the
> test suite for tests that depend on a working git-am implementation.
>
> Since git-am.sh cannot handle any environment modifications by
> setup_git_directory(), "am" has to be declared as NO_SETUP in git.c. On
> the other hand, to re-implement git-am.sh in builtin/am.c, we do need to
> run all the git dir and work tree setup logic that git.c does for us. As
> such, we work around this temporarily by copying the logic in git.c's
> run_builtin(), which amounts to:
>
> prefix = setup_git_directory();
> trace_repo_setup(prefix);
> setup_work_tree();
>
> This redirection should be removed when all the features of git-am.sh
> have been re-implemented in builtin/am.c.
>
> Helped-by: Junio C Hamano
> Signed-off-by: Paul Tan
> ---
>
> Notes:
> v3
>
> * Style fixes
>
> * git-am.sh cannot handle the chdir() and GIT_DIR envionment variable
> that setup_git_directory() sets, so we work around it by copying the
> logic of git.c's run_builtin(), and running it only when we are using
> the builtin am.
>
> Makefile | 1 +
> builtin.h| 1 +
> builtin/am.c | 28
> git.c| 1 +
> 4 files changed, 31 insertions(+)
> create mode 100644 builtin/am.c
>
> diff --git a/Makefile b/Makefile
> index 93e4fa2..ff9bdc0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -811,6 +811,7 @@ LIB_OBJS += xdiff-interface.o
> LIB_OBJS += zlib.o
>
> BUILTIN_OBJS += builtin/add.o
> +BUILTIN_OBJS += builtin/am.o
> BUILTIN_OBJS += builtin/annotate.o
> BUILTIN_OBJS += builtin/apply.o
> BUILTIN_OBJS += builtin/archive.o
> diff --git a/builtin.h b/builtin.h
> index ea3c834..f30cf00 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode,
> const unsigned char
> extern int is_builtin(const char *s);
>
> extern int cmd_add(int argc, const char **argv, const char *prefix);
> +extern int cmd_am(int argc, const char **argv, const char *prefix);
> extern int cmd_annotate(int argc, const char **argv, const char *prefix);
> extern int cmd_apply(int argc, const char **argv, const char *prefix);
> extern int cmd_archive(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/am.c b/builtin/am.c
> new file mode 100644
> index 000..dbc8836
> --- /dev/null
> +++ b/builtin/am.c
> @@ -0,0 +1,28 @@
> +/*
> + * Builtin "git am"
> + *
> + * Based on git-am.sh by Junio C Hamano.
> + */
> +#include "cache.h"
> +#include "builtin.h"
> +#include "exec_cmd.h"
> +
> +int cmd_am(int argc, const char **argv, const char *prefix)
> +{
> + /*
> + * FIXME: Once all the features of git-am.sh have been re-implemented
> + * in builtin/am.c, this preamble can be removed.
> + */
It's not broken, so "FIXME" is not quite appropriate (and that is
why I sent you "NEEDSWORK"). Also mention that the entry in the
commands[] array needs "RUN_SETUP | NEED_WORK_TREE" added, I think.
> + if (!getenv("_GIT_USE_BUILTIN_AM")) {
> + const char *path = mkpath("%s/git-am", git_exec_path());
> +
> + if (sane_execvp(path, (char **)argv) < 0)
> + die_errno("could not exec %s", path);
> + } else {
> + prefix = setup_git_directory();
> + trace_repo_setup(prefix);
> + setup_work_tree();
> + }
> +
> + return 0;
> +}
> diff --git a/git.c b/git.c
> index e7a7713..a671535 100644
> --- a/git.c
> +++ b/git.c
> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
> const char **argv)
>
> static struct cmd_struct commands[] = {
> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> + { "am", cmd_am, NO_SETUP },
NO_SETUP is for things like init and clone that start without a
repository and then work in the one that they create. I think
imitating "archive" or "diff" is more appropriate.
> { "annotate", cmd_annotate, RUN_SETUP },
> { "apply", cmd_apply, RUN_SETUP_GENTLY },
> { "archive", cmd_archive },
--
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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 2:08 PM, Junio C Hamano wrote: > Patrick Palka writes: > >>> I have this nagging feeling that it is just as likely that two >>> uneven hunks align at the top as they align at the bottom, so while >>> this might not hurt it may not be the right approach for a better >>> solution, in the sense that when somebody really wants to do a >>> better solution, this change and the original code may need to be >>> ripped out and redone from scratch. >> >> Hmm, maybe. I stuck with assuming hunks are top-aligned because it >> required less code to implement :) > > Yeah, I understand that. > > If we will need to rip out only this change but keep the original in > order to implement a future better solution, then we might be better > off not having this change (if we anticipate such a better solution > to come reasonably soon), because it would make it more work for the > final improved solution. But if we need to rip out the original as > well as this change while we do so, then having this patch would not > make it more work, either. > > So as I said, I do not think it would hurt to have this as an > incremental improvement (albeit going in a possibly wrong > direction). > > Of course, it is a separate question if this change makes the output > worse, by comparing unmatched early parts of two hunks and making > nonsense highlight by calling highlight_pair() more often. As long > as that is not an issue, I am not opposed to this change, which was > what I meant to say by "this might not hurt". > That makes sense. The extra useless highlighting indeed is currently a problem but it may yet be worked around. -- 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] format-patch: introduce format.outputDirectory configuration
On Thu, Jun 18, 2015 at 04:13:23PM -0400, Jeff King wrote: > > > You would also need to remove the "oh you gave me -o twice?" check, > > > and change the semantics to "later -o overrides an earlier one", > > > wouldn't you? Otherwise you would never be able to override what > > > you read from the config, I am afraid. > > > > By the way, I actually think "later -o overrides an earlier one" is > > a good change by itself, regardless of this new configuration. > > Ah, I didn't realize we did that. Yeah, I think we should switch to > "later overrides earlier". There is no need for "-o" to behave > completely differently than all of our other options. Much worse, though, is that we also have to interact with "--stdout". We currently treat "--stdout -o foo" as an error; you need a separate config_output_directory to continue to handle that (and allow "--stdout" to override the config). If I were designing from scratch, I would consider making "-o -" output to stdout, and letting it override a previous "-o" (or vice versa). We could still do that (and make "--stdout" an alias for that), but I don't know if it is worth the trouble (it does change the behavior for anybody who wanted a directory called "-", but IMHO it is more likely to save somebody a headache than create one). -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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, 18 Jun 2015, Jeff King wrote:
On Thu, Jun 18, 2015 at 11:08:16AM -0700, Junio C Hamano wrote:
So as I said, I do not think it would hurt to have this as an
incremental improvement (albeit going in a possibly wrong
direction).
Of course, it is a separate question if this change makes the output
worse, by comparing unmatched early parts of two hunks and making
nonsense highlight by calling highlight_pair() more often. As long
as that is not an issue, I am not opposed to this change, which was
what I meant to say by "this might not hurt".
Yes, that is my big concern, and why I punted on mismatched-size hunks
in the first place. Now that we have a patch, it is easy enough to "git
log -p | diff-highlight" with the old and new versions to compare the
results.
It certainly does improve some cases. E.g.:
-foo
+foo &&
+bar
in a test script becomes more clear. But some of the output is not so
great. For instance, the very commit under discussion has a
confusing and useless highlight. Or take a documentation patch like
5c31acfb, where I find the highlights actively distracting. We are saved
a little by the "if the whole line is different, do not highlight at
all" behavior of 097128d1bc.
To fix the useless highlights for both evenly and unevenly sized hunks
(like when all but a semicolon on a line changes), one can loosen the
criterion for not highlighting from "do not highlight if 0% of the
before and after lines are common between them" to, say, "do not
highlight if less than 10% of the before and after lines are common
between them". Then most of these useless highlights are gone for both
evenly and unevenly sized hunks.
Here is a patch that changes the criterion as mentioned. Testing this
change on the documentation patch 5c31acfb, only two pairs of lines are
highlighted instead of six. On my original patch, the useless highlight
is gone. The useless semicolon-related highlights on e.g. commit
99a2cfb are gone.
Ten percent is a modest threshold, and perhaps it should be increased
when highlighting unevenly sized hunks and decreased when highlighting
evenly sized hunks.
Of course, these patches are both hacks but they seem to be surprisingly
effective hacks especially when paired together.
So I dunno. IMHO this does more harm than good, and I would not want to
use it myself. But it is somewhat a matter of taste; I am not opposed to
making it a configurable option.
That is something I can do :)
-Peff
-- >8 --
Subject: [PATCH] diff-highlight: don't highlight lines that have little in
common
---
contrib/diff-highlight/diff-highlight | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index 85d2eb0..e4829ec 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -218,8 +218,13 @@ sub is_pair_interesting {
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
my $suffix_b = join('', @$b[($sb+1)..$#$b]);
- return $prefix_a !~ /^$COLOR*-$BORING*$/ ||
- $prefix_b !~ /^$COLOR*\+$BORING*$/ ||
- $suffix_a !~ /^$BORING*$/ ||
- $suffix_b !~ /^$BORING*$/;
+ $prefix_a =~ s/^$COLOR*-$BORING*//;
+ $prefix_b =~ s/^$COLOR*\+$BORING*//;
+ $suffix_a =~ s/$BORING*$//;
+ $suffix_b =~ s/$BORING*$//;
+
+ # Only bother highlighting if at least 10% of each line is common among
+ # the lines.
+ return ((length($prefix_a)+length($suffix_a))*100 >= @$a*10) &&
+ ((length($prefix_b)+length($suffix_b))*100 >= @$b*10);
}
--
2.4.4.410.g43ed522.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: [PATCH v4 00/19] Make git-pull a builtin
Paul Tan writes: > This is a re-roll of [v3]. It squashes in Ramsay's patch "fix some sparse > warnings", and fixes the use-before-free reported by Duy. Thanks a lot for > dealing with my mess :-). > > Other than that, there are no other changes as I'm working on the git-am side > of things. I didn't look carefully, but does that mean 04/19 has the "what if you start from a subdirectory and are still using the scripted one?" issue we discussed recently for "am"? -- 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] format-patch: introduce format.outputDirectory configuration
On Thu, Jun 18, 2015 at 01:06:54PM -0700, Junio C Hamano wrote: > >> Don't we load the config before parsing options here? In that case, we > >> can use our usual strategy to just set output_directory (which is > >> already a static global) from the config callback, and everything Just > >> Works. > >> > >> We do have to bump the definition of output_directory up above the > >> config callback, like so (while we are here, we might also want to > >> drop the unnecessary static initializers, which violate our style guide): > > > > You would also need to remove the "oh you gave me -o twice?" check, > > and change the semantics to "later -o overrides an earlier one", > > wouldn't you? Otherwise you would never be able to override what > > you read from the config, I am afraid. > > By the way, I actually think "later -o overrides an earlier one" is > a good change by itself, regardless of this new configuration. Ah, I didn't realize we did that. Yeah, I think we should switch to "later overrides earlier". There is no need for "-o" to behave completely differently than all of our other options. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 16/19] fsck: Support demoting errors to warnings
We already have support in `git receive-pack` to deal with some legacy
repositories which have non-fatal issues.
Let's make `git fsck` itself useful with such repositories, too, by
allowing users to ignore known issues, or at least demote those issues
to mere warnings.
Example: `git -c fsck.missingemail=ignore fsck` would hide
problems with missing emails in author, committer and tagger lines.
In the same spirit that `git receive-pack`'s usage of the fsck machinery
differs from `git fsck`'s – some of the non-fatal warnings in `git fsck`
are fatal with `git receive-pack` when receive.fsckObjects = true, for
example – we strictly separate the fsck. from the
receive.fsck. settings.
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt | 11 +++
builtin/fsck.c | 12
t/t1450-fsck.sh | 11 +++
3 files changed, 34 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 306ab7a..41fd460 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1250,6 +1250,17 @@ filter..smudge::
object to a worktree file upon checkout. See
linkgit:gitattributes[5] for details.
+fsck.::
+ Allows overriding the message type (error, warn or ignore) of a
+ specific message ID such as `missingemail`.
++
+For convenience, fsck prefixes the error/warning with the message ID,
+e.g. "missingemail: invalid author/committer line - missing email" means
+that setting `fsck.missingemail = ignore` will hide that issue.
++
+This feature is intended to support working with legacy repositories
+which cannot be repaired without disruptive changes.
+
gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'. This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index fff38fe..6de9f3e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -46,6 +46,16 @@ static int show_dangling = 1;
#define DIRENT_SORT_HINT(de) ((de)->d_ino)
#endif
+static int fsck_config(const char *var, const char *value, void *cb)
+{
+ if (skip_prefix(var, "fsck.", &var)) {
+ fsck_set_msg_type(&fsck_obj_options, var, -1, value, -1);
+ return 0;
+ }
+
+ return git_default_config(var, value, cb);
+}
+
static void objreport(struct object *obj, const char *msg_type,
const char *err)
{
@@ -646,6 +656,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
include_reflogs = 0;
}
+ git_config(fsck_config, NULL);
+
fsck_head_link();
fsck_object_dir(get_object_directory());
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 286a643..fe4bb03 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -287,6 +287,17 @@ test_expect_success 'rev-list --verify-objects with bad
sha1' '
grep -q "error: sha1 mismatch 63ff"
out
'
+test_expect_success 'force fsck to ignore double author' '
+ git cat-file commit HEAD >basis &&
+ sed "s/^author .*/&,&/" multiple-authors &&
+ new=$(git hash-object -t commit -w --stdin http://vger.kernel.org/majordomo-info.html
[PATCH v5 18/19] fsck: git receive-pack: support excluding objects from fsck'ing
The optional new config option `receive.fsck.skiplist` specifies the path
to a file listing the names, i.e. SHA-1s, one per line, of objects that
are to be ignored by `git receive-pack` when `receive.fsckObjects = true`.
This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).
The intended use case is for server administrators to inspect objects
that are reported by `git push` as being too problematic to enter the
repository, and to add the objects' SHA-1 to a (preferably sorted) file
when the objects are legitimate, i.e. when it is determined that those
problematic objects should be allowed to enter the server.
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt| 7 ++
builtin/receive-pack.c | 8 ++
fsck.c | 54 +
fsck.h | 1 +
t/t5504-fetch-receive-strict.sh | 12 +
5 files changed, 82 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 41fd460..5f45115 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2230,6 +2230,13 @@ which would not pass pushing when `receive.fsckObjects =
true`, allowing
the host to accept repositories with certain known issues but still catch
other issues.
+receive.fsck.skipList::
+ The path to a sorted list of object names (i.e. one SHA-1 per
+ line) that are known to be broken in a non-fatal way and should
+ be ignored. This feature is useful when an established project
+ should be accepted despite early commits containing errors that
+ can be safely ignored such as invalid committer email addresses.
+
receive.unpackLimit::
If the number of objects received in a push is below this
limit then the objects will be unpacked into loose object
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 3afe8f8..80574f9 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -117,6 +117,14 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (strcmp(var, "receive.fsck.skiplist") == 0) {
+ const char *path = is_absolute_path(value) ?
+ value : git_path("%s", value);
+ strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
+ fsck_msg_types.len ? ',' : '=', path);
+ return 0;
+ }
+
if (skip_prefix(var, "receive.fsck.", &var)) {
if (is_valid_msg_type(var, value))
strbuf_addf(&fsck_msg_types, "%c%s=%s",
diff --git a/fsck.c b/fsck.c
index a5e7dfb..9b8981e 100644
--- a/fsck.c
+++ b/fsck.c
@@ -8,6 +8,7 @@
#include "fsck.h"
#include "refs.h"
#include "utf8.h"
+#include "sha1-array.h"
#define FSCK_FATAL -1
#define FSCK_INFO -2
@@ -122,6 +123,43 @@ static int fsck_msg_type(enum fsck_msg_id msg_id,
return msg_type;
}
+static void init_skiplist(struct fsck_options *options, const char *path)
+{
+ static struct sha1_array skiplist = SHA1_ARRAY_INIT;
+ int sorted, fd;
+ char buffer[41];
+ unsigned char sha1[20];
+
+ if (options->skiplist)
+ sorted = options->skiplist->sorted;
+ else {
+ sorted = 1;
+ options->skiplist = &skiplist;
+ }
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ die("Could not open skip list: %s", path);
+ for (;;) {
+ int result = read_in_full(fd, buffer, sizeof(buffer));
+ if (result < 0)
+ die_errno("Could not read '%s'", path);
+ if (!result)
+ break;
+ if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
+ die("Invalid SHA-1: %s", buffer);
+ sha1_array_append(&skiplist, sha1);
+ if (sorted && skiplist.nr > 1 &&
+ hashcmp(skiplist.sha1[skiplist.nr - 2],
+ sha1) > 0)
+ sorted = 0;
+ }
+ close(fd);
+
+ if (sorted)
+ skiplist.sorted = 1;
+}
+
static inline int substrcmp(const char *string, int len, const char *match)
{
int match_len = strlen(match);
@@ -193,6 +231,18 @@ void fsck_set_msg_types(struct fsck_options *options,
const char *values)
if (values[equal] == '=' || values[equal] == ':')
break;
+ if (!substrcmp(values, equal, "skiplist")) {
+ char *path = xstrndup(values + equal + 1,
+ len - equal - 1);
+
+ if (equal == len)
+ die("skiplist requires a path");
+ init_
[PATCH v5 19/19] fsck: support ignoring objects in `git fsck` via fsck.skiplist
Identical to support in `git receive-pack for the config option
`receive.fsck.skiplist`, we now support ignoring given objects in
`git fsck` via `fsck.skiplist` altogether.
This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).
Signed-off-by: Johannes Schindelin
---
Documentation/config.txt | 7 +++
builtin/fsck.c | 10 ++
2 files changed, 17 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f45115..5aba63a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1261,6 +1261,13 @@ that setting `fsck.missingemail = ignore` will hide that
issue.
This feature is intended to support working with legacy repositories
which cannot be repaired without disruptive changes.
+fsck.skipList::
+ The path to a sorted list of object names (i.e. one SHA-1 per
+ line) that are known to be broken in a non-fatal way and should
+ be ignored. This feature is useful when an established project
+ should be accepted despite early commits containing errors that
+ can be safely ignored such as invalid committer email addresses.
+
gc.aggressiveDepth::
The depth parameter used in the delta compression
algorithm used by 'git gc --aggressive'. This defaults
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 75fcb5f..ce538ac 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -54,6 +54,16 @@ static int fsck_config(const char *var, const char *value,
void *cb)
return 0;
}
+ if (strcmp(var, "fsck.skiplist") == 0) {
+ const char *path = is_absolute_path(value) ?
+ value : git_path("%s", value);
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "skiplist=%s", path);
+ fsck_set_msg_types(&fsck_obj_options, sb.buf);
+ strbuf_release(&sb);
+ return 0;
+ }
+
return git_default_config(var, value, cb);
}
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 15/19] fsck: Document the new receive.fsck. options
Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 3e37b93..306ab7a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2205,6 +2205,20 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. +receive.fsck.:: + When `receive.fsckObjects` is set to true, errors can be switched + to warnings and vice versa by configuring the `receive.fsck.` + setting where the `` is the fsck message ID and the value + is one of `error`, `warn` or `ignore`. For convenience, fsck prefixes + the error/warning with the message ID, e.g. "missingemail: invalid + author/committer line - missing email" means that setting + `receive.fsck.missingemail = ignore` will hide that issue. ++ +This feature is intended to support working with legacy repositories +which would not pass pushing when `receive.fsckObjects = true`, allowing +the host to accept repositories with certain known issues but still catch +other issues. + receive.unpackLimit:: If the number of objects received in a push is below this limit then the objects will be unpacked into loose object -- 2.3.1.windows.1.9.g8c01ab4 -- 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 v5 14/19] fsck: Allow upgrading fsck warnings to errors
The 'invalid tag name' and 'missing tagger entry' warnings can now be
upgraded to errors by specifying `invalidtagname` and
`missingtaggerentry` in the receive.fsck. config setting.
Incidentally, the missing tagger warning is now really shown as a warning
(as opposed to being reported with the "error:" prefix, as it used to be
the case before this commit).
Signed-off-by: Johannes Schindelin
---
fsck.c| 24 +---
t/t5302-pack-index.sh | 2 +-
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/fsck.c b/fsck.c
index 0f7eb22..a5e7dfb 100644
--- a/fsck.c
+++ b/fsck.c
@@ -10,6 +10,7 @@
#include "utf8.h"
#define FSCK_FATAL -1
+#define FSCK_INFO -2
#define FOREACH_MSG_ID(FUNC) \
/* fatal errors */ \
@@ -55,10 +56,11 @@
FUNC(HAS_DOT, WARN) \
FUNC(HAS_DOTDOT, WARN) \
FUNC(HAS_DOTGIT, WARN) \
- FUNC(INVALID_TAG_NAME, WARN) \
- FUNC(MISSING_TAGGER_ENTRY, WARN) \
FUNC(NULL_SHA1, WARN) \
- FUNC(ZERO_PADDED_FILEMODE, WARN)
+ FUNC(ZERO_PADDED_FILEMODE, WARN) \
+ /* infos (reported as warnings, but ignored by default) */ \
+ FUNC(INVALID_TAG_NAME, INFO) \
+ FUNC(MISSING_TAGGER_ENTRY, INFO)
#define MSG_ID(id, msg_type) FSCK_MSG_##id,
enum fsck_msg_id {
@@ -227,6 +229,8 @@ static int report(struct fsck_options *options, struct
object *object,
if (msg_type == FSCK_FATAL)
msg_type = FSCK_ERROR;
+ else if (msg_type == FSCK_INFO)
+ msg_type = FSCK_WARN;
append_msg_id(&sb, msg_id_info[id].id_string);
@@ -685,15 +689,21 @@ static int fsck_tag_buffer(struct tag *tag, const char
*data,
goto done;
}
strbuf_addf(&sb, "refs/tags/%.*s", (int)(eol - buffer), buffer);
- if (check_refname_format(sb.buf, 0))
- report(options, &tag->object, FSCK_MSG_INVALID_TAG_NAME,
+ if (check_refname_format(sb.buf, 0)) {
+ ret = report(options, &tag->object, FSCK_MSG_INVALID_TAG_NAME,
"invalid 'tag' name: %.*s",
(int)(eol - buffer), buffer);
+ if (ret)
+ goto done;
+ }
buffer = eol + 1;
- if (!skip_prefix(buffer, "tagger ", &buffer))
+ if (!skip_prefix(buffer, "tagger ", &buffer)) {
/* early tags do not contain 'tagger' lines; warn only */
- report(options, &tag->object, FSCK_MSG_MISSING_TAGGER_ENTRY,
"invalid format - expected 'tagger' line");
+ ret = report(options, &tag->object,
FSCK_MSG_MISSING_TAGGER_ENTRY, "invalid format - expected 'tagger' line");
+ if (ret)
+ goto done;
+ }
else
ret = fsck_ident(&buffer, &tag->object, options);
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 61bc8da..3dc5ec4 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -259,7 +259,7 @@ EOF
thirtyeight=${tag#??} &&
rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight &&
git index-pack --strict tag-test-${pack1}.pack 2>err &&
-grep "^error:.* expected .tagger. line" err
+grep "^warning:.* expected .tagger. line" err
'
test_done
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 17/19] fsck: Introduce `git fsck --quick`
This option avoids unpacking each and all objects, and just verifies the
connectivity. In particular with large repositories, this speeds up the
operation, at the expense of missing corrupt blobs and ignoring
unreachable objects, if any.
Signed-off-by: Johannes Schindelin
---
Documentation/git-fsck.txt | 7 ++-
builtin/fsck.c | 7 ++-
t/t1450-fsck.sh| 22 ++
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index 25c431d..b98fb43 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
-[--[no-]full] [--strict] [--verbose] [--lost-found]
+[--[no-]full] [--quick] [--strict] [--verbose] [--lost-found]
[--[no-]dangling] [--[no-]progress] [*]
DESCRIPTION
@@ -60,6 +60,11 @@ index file, all SHA-1 references in `refs` namespace, and
all reflogs
object pools. This is now default; you can turn it off
with --no-full.
+--quick::
+ Check only the connectivity of tags, commits and tree objects. By
+ avoiding to unpack blobs, this speeds up the operation, at the
+ expense of missing corrupt objects.
+
--strict::
Enable more strict checking, namely to catch a file mode
recorded with g+w bit set, which was created by older
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6de9f3e..75fcb5f 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -23,6 +23,7 @@ static int show_tags;
static int show_unreachable;
static int include_reflogs = 1;
static int check_full = 1;
+static int quick;
static int check_strict;
static int keep_cache_objects;
static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
@@ -181,6 +182,8 @@ static void check_reachable_object(struct object *obj)
if (!(obj->flags & HAS_OBJ)) {
if (has_sha1_pack(obj->sha1))
return; /* it is in pack - forget about it */
+ if (quick && has_sha1_file(obj->sha1))
+ return;
printf("missing %s %s\n", typename(obj->type),
sha1_to_hex(obj->sha1));
errors_found |= ERROR_REACHABLE;
return;
@@ -623,6 +626,7 @@ static struct option fsck_opts[] = {
OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head
nodes")),
OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes
(default)")),
OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate
objects")),
+ OPT_BOOL(0, "quick", &quick, N_("check only connectivity")),
OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")),
OPT_BOOL(0, "lost-found", &write_lost_and_found,
N_("write dangling objects in
.git/lost-found")),
@@ -659,7 +663,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
git_config(fsck_config, NULL);
fsck_head_link();
- fsck_object_dir(get_object_directory());
+ if (!quick)
+ fsck_object_dir(get_object_directory());
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt->next) {
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index fe4bb03..471e2ea 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -431,4 +431,26 @@ test_expect_success 'fsck notices ref pointing to missing
tag' '
test_must_fail git -C missing fsck
'
+test_expect_success 'fsck --quick' '
+ rm -rf quick &&
+ git init quick &&
+ (
+ cd quick &&
+ touch empty &&
+ git add empty &&
+ test_commit empty &&
+ empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
+ rm -f $empty &&
+ echo invalid >$empty &&
+ test_must_fail git fsck --strict &&
+ git fsck --strict --quick &&
+ tree=$(git rev-parse HEAD:) &&
+ suffix=${tree#??} &&
+ tree=.git/objects/${tree%$suffix}/$suffix &&
+ rm -f $tree &&
+ echo invalid >$tree &&
+ test_must_fail git fsck --strict --quick
+ )
+'
+
test_done
--
2.3.1.windows.1.9.g8c01ab4
--
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 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD
On 2015-06-18 21:45, Junio C Hamano wrote: > This round looks good, except one trivial nit (below), which I'll > locally squash-in a fix for. Thanks, 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
[PATCH v5 12/19] fsck: Disallow demoting grave fsck errors to warnings
Some kinds of errors are intrinsically unrecoverable (e.g. errors while
uncompressing objects). It does not make sense to allow demoting them to
mere warnings.
Signed-off-by: Johannes Schindelin
---
fsck.c | 14 --
t/t5504-fetch-receive-strict.sh | 11 +++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index bd4bfc2..2b2a360 100644
--- a/fsck.c
+++ b/fsck.c
@@ -9,7 +9,12 @@
#include "refs.h"
#include "utf8.h"
+#define FSCK_FATAL -1
+
#define FOREACH_MSG_ID(FUNC) \
+ /* fatal errors */ \
+ FUNC(NUL_IN_HEADER, FATAL) \
+ FUNC(UNTERMINATED_HEADER, FATAL) \
/* errors */ \
FUNC(BAD_DATE, ERROR) \
FUNC(BAD_EMAIL, ERROR) \
@@ -40,10 +45,8 @@
FUNC(MISSING_TYPE_ENTRY, ERROR) \
FUNC(MULTIPLE_AUTHORS, ERROR) \
FUNC(NOT_SORTED, ERROR) \
- FUNC(NUL_IN_HEADER, ERROR) \
FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
FUNC(UNKNOWN_TYPE, ERROR) \
- FUNC(UNTERMINATED_HEADER, ERROR) \
FUNC(ZERO_PADDED_DATE, ERROR) \
/* warnings */ \
FUNC(BAD_FILEMODE, WARN) \
@@ -157,6 +160,10 @@ void fsck_set_msg_type(struct fsck_options *options,
die("Unhandled message id: %.*s", msg_id_len, msg_id);
type = parse_msg_type(msg_type, msg_type_len);
+ if (type != FSCK_ERROR && msg_id_info[id].msg_type == FSCK_FATAL)
+ die("Cannot demote %.*s to %.*s", msg_id_len, msg_id,
+ msg_type_len, msg_type);
+
if (!options->msg_type) {
int i;
int *msg_type = xmalloc(sizeof(int) * FSCK_MSG_MAX);
@@ -213,6 +220,9 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_type = fsck_msg_type(id, options), result;
+ if (msg_type == FSCK_FATAL)
+ msg_type = FSCK_ERROR;
+
append_msg_id(&sb, msg_id_info[id].id_string);
va_start(ap, fmt);
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 3f7e96a..0d64229 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -136,4 +136,15 @@ test_expect_success 'push with
receive.fsck.missingemail=warn' '
grep "missingemail" act
'
+test_expect_success \
+ 'receive.fsck.unterminatedheader=warn triggers error' '
+ rm -rf dst &&
+ git init dst &&
+ git --git-dir=dst/.git config receive.fsckobjects true &&
+ git --git-dir=dst/.git config \
+ receive.fsck.unterminatedheader warn &&
+ test_must_fail git push --porcelain dst HEAD >act 2>&1 &&
+ grep "Cannot demote unterminatedheader" act
+'
+
test_done
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 13/19] fsck: Optionally ignore specific fsck issues completely
An fsck issue in a legacy repository might be so common that one would
like not to bother the user with mentioning it at all. With this change,
that is possible by setting the respective message type to "ignore".
This change "abuses" the missingemail=warn test to verify that "ignore"
is also accepted and works correctly. And while at it, it makes sure
that multiple options work, too (they are passed to unpack-objects or
index-pack as a comma-separated list via the --strict=... command-line
option).
Signed-off-by: Johannes Schindelin
---
fsck.c | 5 +
fsck.h | 1 +
t/t5504-fetch-receive-strict.sh | 9 -
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 2b2a360..0f7eb22 100644
--- a/fsck.c
+++ b/fsck.c
@@ -137,6 +137,8 @@ static int parse_msg_type(const char *str, int len)
return FSCK_ERROR;
else if (!substrcmp(str, len, "warn"))
return FSCK_WARN;
+ else if (!substrcmp(str, len, "ignore"))
+ return FSCK_IGNORE;
else
die("Unknown fsck message type: '%.*s'",
len, str);
@@ -220,6 +222,9 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_type = fsck_msg_type(id, options), result;
+ if (msg_type == FSCK_IGNORE)
+ return 0;
+
if (msg_type == FSCK_FATAL)
msg_type = FSCK_ERROR;
diff --git a/fsck.h b/fsck.h
index 738c9df..7e49372 100644
--- a/fsck.h
+++ b/fsck.h
@@ -3,6 +3,7 @@
#define FSCK_ERROR 1
#define FSCK_WARN 2
+#define FSCK_IGNORE 3
struct fsck_options;
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 0d64229..cb077b7 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -133,7 +133,14 @@ test_expect_success 'push with
receive.fsck.missingemail=warn' '
git --git-dir=dst/.git config \
receive.fsck.missingemail warn &&
git push --porcelain dst bogus >act 2>&1 &&
- grep "missingemail" act
+ grep "missingemail" act &&
+ git --git-dir=dst/.git branch -D bogus &&
+ git --git-dir=dst/.git config --add \
+ receive.fsck.missingemail ignore &&
+ git --git-dir=dst/.git config --add \
+ receive.fsck.baddate warn &&
+ git push --porcelain dst bogus >act 2>&1 &&
+ test_must_fail grep "missingemail" act
'
test_expect_success \
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 11/19] fsck: Add a simple test for receive.fsck.
Signed-off-by: Johannes Schindelin --- t/t5504-fetch-receive-strict.sh | 21 + 1 file changed, 21 insertions(+) diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 69ee13c..3f7e96a 100755 --- a/t/t5504-fetch-receive-strict.sh +++ b/t/t5504-fetch-receive-strict.sh @@ -115,4 +115,25 @@ test_expect_success 'push with transfer.fsckobjects' ' test_cmp exp act ' +cat >bogus-commit <<\EOF +tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 +author Bugs Bunny 1234567890 + +committer Bugs Bunny 1234567890 + + +This commit object intentionally broken +EOF + +test_expect_success 'push with receive.fsck.missingemail=warn' ' + commit="$(git hash-object -t commit -w --stdin act 2>&1 && + grep "missingemail" act +' + test_done -- 2.3.1.windows.1.9.g8c01ab4 -- 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 v5 10/19] fsck: Make fsck_tag() warn-friendly
When fsck_tag() identifies a problem with the commit, it should try
to make it possible to continue checking the commit object, in case the
user wants to demote the detected errors to mere warnings.
Just like fsck_commit(), there are certain problems that could hide other
issues with the same tag object. For example, if the 'type' line is not
encountered in the correct position, the 'tag' line – if there is any –
would not be handled at all.
Signed-off-by: Johannes Schindelin
---
fsck.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 856221d..bd4bfc2 100644
--- a/fsck.c
+++ b/fsck.c
@@ -640,7 +640,8 @@ static int fsck_tag_buffer(struct tag *tag, const char
*data,
}
if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') {
ret = report(options, &tag->object,
FSCK_MSG_INVALID_OBJECT_SHA1, "invalid 'object' line format - bad sha1");
- goto done;
+ if (ret)
+ goto done;
}
buffer += 41;
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 09/19] fsck: Handle multiple authors in commits specially
This problem has been detected in the wild, and is the primary reason
to introduce an option to demote certain fsck errors to warnings. Let's
offer to ignore this particular problem specifically.
Technically, we could handle such repositories by setting
receive.fsck. to missingcommitter=warn, but that could hide
missing tree objects in the same commit because we cannot continue
verifying any commit object after encountering a missing committer line,
while we can continue in the case of multiple author lines.
Signed-off-by: Johannes Schindelin
---
fsck.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/fsck.c b/fsck.c
index 31d218d..856221d 100644
--- a/fsck.c
+++ b/fsck.c
@@ -38,6 +38,7 @@
FUNC(MISSING_TREE, ERROR) \
FUNC(MISSING_TYPE, ERROR) \
FUNC(MISSING_TYPE_ENTRY, ERROR) \
+ FUNC(MULTIPLE_AUTHORS, ERROR) \
FUNC(NOT_SORTED, ERROR) \
FUNC(NUL_IN_HEADER, ERROR) \
FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
@@ -571,6 +572,14 @@ static int fsck_commit_buffer(struct commit *commit, const
char *buffer,
err = fsck_ident(&buffer, &commit->object, options);
if (err)
return err;
+ while (skip_prefix(buffer, "author ", &buffer)) {
+ err = report(options, &commit->object,
FSCK_MSG_MULTIPLE_AUTHORS, "invalid format - multiple 'author' lines");
+ if (err)
+ return err;
+ err = fsck_ident(&buffer, &commit->object, options);
+ if (err)
+ return err;
+ }
if (!skip_prefix(buffer, "committer ", &buffer))
return report(options, &commit->object,
FSCK_MSG_MISSING_COMMITTER, "invalid format - expected 'committer' line");
err = fsck_ident(&buffer, &commit->object, options);
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 08/19] fsck: Make fsck_commit() warn-friendly
When fsck_commit() identifies a problem with the commit, it should try
to make it possible to continue checking the commit object, in case the
user wants to demote the detected errors to mere warnings.
Note that some problems are too problematic to simply ignore. For
example, when the header lines are mixed up, we punt after encountering
an incorrect line. Therefore, demoting certain warnings to errors can
hide other problems. Example: demoting the missingauthor error to
a warning would hide a problematic committer line.
Signed-off-by: Johannes Schindelin
---
fsck.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/fsck.c b/fsck.c
index 8a1eea3..31d218d 100644
--- a/fsck.c
+++ b/fsck.c
@@ -534,12 +534,18 @@ static int fsck_commit_buffer(struct commit *commit,
const char *buffer,
if (!skip_prefix(buffer, "tree ", &buffer))
return report(options, &commit->object, FSCK_MSG_MISSING_TREE,
"invalid format - expected 'tree' line");
- if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')
- return report(options, &commit->object, FSCK_MSG_BAD_TREE_SHA1,
"invalid 'tree' line format - bad sha1");
+ if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n') {
+ err = report(options, &commit->object, FSCK_MSG_BAD_TREE_SHA1,
"invalid 'tree' line format - bad sha1");
+ if (err)
+ return err;
+ }
buffer += 41;
while (skip_prefix(buffer, "parent ", &buffer)) {
- if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
- return report(options, &commit->object,
FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1");
+ if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') {
+ err = report(options, &commit->object,
FSCK_MSG_BAD_PARENT_SHA1, "invalid 'parent' line format - bad sha1");
+ if (err)
+ return err;
+ }
buffer += 41;
parent_line_count++;
}
@@ -548,11 +554,17 @@ static int fsck_commit_buffer(struct commit *commit,
const char *buffer,
if (graft) {
if (graft->nr_parent == -1 && !parent_count)
; /* shallow commit */
- else if (graft->nr_parent != parent_count)
- return report(options, &commit->object,
FSCK_MSG_MISSING_GRAFT, "graft objects missing");
+ else if (graft->nr_parent != parent_count) {
+ err = report(options, &commit->object,
FSCK_MSG_MISSING_GRAFT, "graft objects missing");
+ if (err)
+ return err;
+ }
} else {
- if (parent_count != parent_line_count)
- return report(options, &commit->object,
FSCK_MSG_MISSING_PARENT, "parent objects missing");
+ if (parent_count != parent_line_count) {
+ err = report(options, &commit->object,
FSCK_MSG_MISSING_PARENT, "parent objects missing");
+ if (err)
+ return err;
+ }
}
if (!skip_prefix(buffer, "author ", &buffer))
return report(options, &commit->object,
FSCK_MSG_MISSING_AUTHOR, "invalid format - expected 'author' line");
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 07/19] fsck: Make fsck_ident() warn-friendly
When fsck_ident() identifies a problem with the ident, it should still
advance the pointer to the next line so that fsck can continue in the
case of a mere warning.
Signed-off-by: Johannes Schindelin
---
fsck.c | 49 +++--
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/fsck.c b/fsck.c
index 47cb686..8a1eea3 100644
--- a/fsck.c
+++ b/fsck.c
@@ -479,40 +479,45 @@ static int require_end_of_header(const void *data,
unsigned long size,
static int fsck_ident(const char **ident, struct object *obj, struct
fsck_options *options)
{
+ const char *p = *ident;
char *end;
- if (**ident == '<')
+ *ident = strchrnul(*ident, '\n');
+ if (**ident == '\n')
+ (*ident)++;
+
+ if (*p == '<')
return report(options, obj, FSCK_MSG_MISSING_NAME_BEFORE_EMAIL,
"invalid author/committer line - missing space before email");
- *ident += strcspn(*ident, "<>\n");
- if (**ident == '>')
+ p += strcspn(p, "<>\n");
+ if (*p == '>')
return report(options, obj, FSCK_MSG_BAD_NAME, "invalid
author/committer line - bad name");
- if (**ident != '<')
+ if (*p != '<')
return report(options, obj, FSCK_MSG_MISSING_EMAIL, "invalid
author/committer line - missing email");
- if ((*ident)[-1] != ' ')
+ if (p[-1] != ' ')
return report(options, obj,
FSCK_MSG_MISSING_SPACE_BEFORE_EMAIL, "invalid author/committer line - missing
space before email");
- (*ident)++;
- *ident += strcspn(*ident, "<>\n");
- if (**ident != '>')
+ p++;
+ p += strcspn(p, "<>\n");
+ if (*p != '>')
return report(options, obj, FSCK_MSG_BAD_EMAIL, "invalid
author/committer line - bad email");
- (*ident)++;
- if (**ident != ' ')
+ p++;
+ if (*p != ' ')
return report(options, obj, FSCK_MSG_MISSING_SPACE_BEFORE_DATE,
"invalid author/committer line - missing space before date");
- (*ident)++;
- if (**ident == '0' && (*ident)[1] != ' ')
+ p++;
+ if (*p == '0' && p[1] != ' ')
return report(options, obj, FSCK_MSG_ZERO_PADDED_DATE, "invalid
author/committer line - zero-padded date");
- if (date_overflows(strtoul(*ident, &end, 10)))
+ if (date_overflows(strtoul(p, &end, 10)))
return report(options, obj, FSCK_MSG_DATE_OVERFLOW, "invalid
author/committer line - date causes integer overflow");
- if (end == *ident || *end != ' ')
+ if ((end == p || *end != ' '))
return report(options, obj, FSCK_MSG_BAD_DATE, "invalid
author/committer line - bad date");
- *ident = end + 1;
- if ((**ident != '+' && **ident != '-') ||
- !isdigit((*ident)[1]) ||
- !isdigit((*ident)[2]) ||
- !isdigit((*ident)[3]) ||
- !isdigit((*ident)[4]) ||
- ((*ident)[5] != '\n'))
+ p = end + 1;
+ if ((*p != '+' && *p != '-') ||
+ !isdigit(p[1]) ||
+ !isdigit(p[2]) ||
+ !isdigit(p[3]) ||
+ !isdigit(p[4]) ||
+ (p[5] != '\n'))
return report(options, obj, FSCK_MSG_BAD_TIMEZONE, "invalid
author/committer line - bad time zone");
- (*ident) += 6;
+ p += 6;
return 0;
}
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 05/19] fsck (receive-pack): Allow demoting errors to warnings
For example, missing emails in commit and tag objects can be demoted to
mere warnings with
git config receive.fsck.missingemail=warn
The value is actually a comma-separated list.
In case that the same key is listed in multiple receive.fsck.
lines in the config, the latter configuration wins (this can happen for
example when both $HOME/.gitconfig and .git/config contain message type
settings).
As git receive-pack does not actually perform the checks, it hands off
the setting to index-pack or unpack-objects in the form of an optional
argument to the --strict option.
Signed-off-by: Johannes Schindelin
---
builtin/index-pack.c | 4
builtin/receive-pack.c | 17 +++--
builtin/unpack-objects.c | 5 +
fsck.c | 8
fsck.h | 1 +
5 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 87ae9ba..98e14fe 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1633,6 +1633,10 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
} else if (!strcmp(arg, "--strict")) {
strict = 1;
do_fsck_object = 1;
+ } else if (skip_prefix(arg, "--strict=", &arg)) {
+ strict = 1;
+ do_fsck_object = 1;
+ fsck_set_msg_types(&fsck_options, arg);
} else if (!strcmp(arg,
"--check-self-contained-and-connected")) {
strict = 1;
check_self_contained_and_connected = 1;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 94d0571..3afe8f8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -19,6 +19,7 @@
#include "tag.h"
#include "gpg-interface.h"
#include "sigchain.h"
+#include "fsck.h"
static const char receive_pack_usage[] = "git receive-pack ";
@@ -36,6 +37,7 @@ static enum deny_action deny_current_branch =
DENY_UNCONFIGURED;
static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects = -1;
static int transfer_fsck_objects = -1;
+static struct strbuf fsck_msg_types = STRBUF_INIT;
static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
@@ -115,6 +117,15 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (skip_prefix(var, "receive.fsck.", &var)) {
+ if (is_valid_msg_type(var, value))
+ strbuf_addf(&fsck_msg_types, "%c%s=%s",
+ fsck_msg_types.len ? ',' : '=', var, value);
+ else
+ warning("Skipping unknown msg id '%s'", var);
+ return 0;
+ }
+
if (strcmp(var, "receive.fsckobjects") == 0) {
receive_fsck_objects = git_config_bool(var, value);
return 0;
@@ -1490,7 +1501,8 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
if (quiet)
argv_array_push(&child.args, "-q");
if (fsck_objects)
- argv_array_push(&child.args, "--strict");
+ argv_array_pushf(&child.args, "--strict%s",
+ fsck_msg_types.buf);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1508,7 +1520,8 @@ static const char *unpack(int err_fd, struct shallow_info
*si)
argv_array_pushl(&child.args, "index-pack",
"--stdin", hdr_arg, keep_arg, NULL);
if (fsck_objects)
- argv_array_push(&child.args, "--strict");
+ argv_array_pushf(&child.args, "--strict%s",
+ fsck_msg_types.buf);
if (fix_thin)
argv_array_push(&child.args, "--fix-thin");
child.out = -1;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6d17040..7cc086f 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -530,6 +530,11 @@ int cmd_unpack_objects(int argc, const char **argv, const
char *prefix)
strict = 1;
continue;
}
+ if (skip_prefix(arg, "--strict=", &arg)) {
+ strict = 1;
+ fsck_set_msg_types(&fsck_options, arg);
+ continue;
+ }
if (starts_with(arg, "--pack_header=")) {
struct pack_header *hdr;
char *c;
diff --git a/fsck.c b/fsck.c
index 7db81d2..0c7cc26 1006
[PATCH v5 04/19] fsck: Offer a function to demote fsck errors to warnings
There are legacy repositories out there whose older commits and tags
have issues that prevent pushing them when 'receive.fsckObjects' is set.
One real-life example is a commit object that has been hand-crafted to
list two authors.
Often, it is not possible to fix those issues without disrupting the
work with said repositories, yet it is still desirable to perform checks
by setting `receive.fsckObjects = true`. This commit is the first step
to allow demoting specific fsck issues to mere warnings.
The `fsck_set_msg_types()` function added by this commit parses a list
of settings in the form:
missingemail=warn,badname=warn,...
Unfortunately, the FSCK_WARN/FSCK_ERROR flag is only really heeded by
git fsck so far, but other call paths (e.g. git index-pack --strict)
error out *always* no matter what type was specified. Therefore, we need
to take extra care to set all message types to FSCK_ERROR by default in
those cases.
Signed-off-by: Johannes Schindelin
---
fsck.c | 82 +++---
fsck.h | 10 ++--
2 files changed, 87 insertions(+), 5 deletions(-)
diff --git a/fsck.c b/fsck.c
index 4595c7f..7db81d2 100644
--- a/fsck.c
+++ b/fsck.c
@@ -103,13 +103,85 @@ static int fsck_msg_type(enum fsck_msg_id msg_id,
{
int msg_type;
- msg_type = msg_id_info[msg_id].msg_type;
- if (options->strict && msg_type == FSCK_WARN)
- msg_type = FSCK_ERROR;
+ assert(msg_id >= 0 && msg_id < FSCK_MSG_MAX);
+
+ if (options->msg_type)
+ msg_type = options->msg_type[msg_id];
+ else {
+ msg_type = msg_id_info[msg_id].msg_type;
+ if (options->strict && msg_type == FSCK_WARN)
+ msg_type = FSCK_ERROR;
+ }
return msg_type;
}
+static inline int substrcmp(const char *string, int len, const char *match)
+{
+ int match_len = strlen(match);
+ if (match_len != len)
+ return -1;
+ return memcmp(string, match, len);
+}
+
+static int parse_msg_type(const char *str, int len)
+{
+ if (len < 0)
+ len = strlen(str);
+
+ if (!substrcmp(str, len, "error"))
+ return FSCK_ERROR;
+ else if (!substrcmp(str, len, "warn"))
+ return FSCK_WARN;
+ else
+ die("Unknown fsck message type: '%.*s'",
+ len, str);
+}
+
+void fsck_set_msg_type(struct fsck_options *options,
+ const char *msg_id, int msg_id_len,
+ const char *msg_type, int msg_type_len)
+{
+ int id = parse_msg_id(msg_id, msg_id_len), type;
+
+ if (id < 0)
+ die("Unhandled message id: %.*s", msg_id_len, msg_id);
+ type = parse_msg_type(msg_type, msg_type_len);
+
+ if (!options->msg_type) {
+ int i;
+ int *msg_type = xmalloc(sizeof(int) * FSCK_MSG_MAX);
+ for (i = 0; i < FSCK_MSG_MAX; i++)
+ msg_type[i] = fsck_msg_type(i, options);
+ options->msg_type = msg_type;
+ }
+
+ options->msg_type[id] = type;
+}
+
+void fsck_set_msg_types(struct fsck_options *options, const char *values)
+{
+ while (*values) {
+ int len = strcspn(values, " ,|"), equal;
+
+ if (!len) {
+ values++;
+ continue;
+ }
+
+ for (equal = 0; equal < len; equal++)
+ if (values[equal] == '=' || values[equal] == ':')
+ break;
+
+ if (equal == len)
+ die("Missing '=': '%.*s'", len, values);
+
+ fsck_set_msg_type(options, values, equal,
+ values + equal + 1, len - equal - 1);
+ values += len;
+ }
+}
+
__attribute__((format (printf, 4, 5)))
static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -599,6 +671,10 @@ int fsck_object(struct object *obj, void *data, unsigned
long size,
int fsck_error_function(struct object *obj, int msg_type, const char *message)
{
+ if (msg_type == FSCK_WARN) {
+ warning("object %s: %s", sha1_to_hex(obj->sha1), message);
+ return 0;
+ }
error("object %s: %s", sha1_to_hex(obj->sha1), message);
return 1;
}
diff --git a/fsck.h b/fsck.h
index f6f268a..edb4540 100644
--- a/fsck.h
+++ b/fsck.h
@@ -6,6 +6,11 @@
struct fsck_options;
+void fsck_set_msg_type(struct fsck_options *options,
+ const char *msg_id, int msg_id_len,
+ const char *msg_type, int msg_type_len);
+void fsck_set_msg_types(struct fsck_options *options, const char *values);
+
/*
* callback function for fsck_walk
* type is the expected type of the object or OBJ_ANY
@@ -25,10 +30,11 @@ struct fsck_options {
fsck_walk_func walk;
fsck_
[PATCH v5 06/19] fsck: Report the ID of the error/warning
Some legacy code has objects with non-fatal fsck issues; To enable the
user to ignore those issues, let's print out the ID (e.g. when
encountering "missingemail", the user might want to call `git config
--add receive.fsck.missingemail=warn`).
Signed-off-by: Johannes Schindelin
---
fsck.c | 16
t/t1450-fsck.sh | 4 ++--
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 0c7cc26..47cb686 100644
--- a/fsck.c
+++ b/fsck.c
@@ -190,6 +190,20 @@ void fsck_set_msg_types(struct fsck_options *options,
const char *values)
}
}
+static void append_msg_id(struct strbuf *sb, const char *msg_id)
+{
+ for (;;) {
+ char c = *(msg_id)++;
+
+ if (!c)
+ break;
+ if (c != '_')
+ strbuf_addch(sb, tolower(c));
+ }
+
+ strbuf_addstr(sb, ": ");
+}
+
__attribute__((format (printf, 4, 5)))
static int report(struct fsck_options *options, struct object *object,
enum fsck_msg_id id, const char *fmt, ...)
@@ -198,6 +212,8 @@ static int report(struct fsck_options *options, struct
object *object,
struct strbuf sb = STRBUF_INIT;
int msg_type = fsck_msg_type(id, options), result;
+ append_msg_id(&sb, msg_id_info[id].id_string);
+
va_start(ap, fmt);
strbuf_vaddf(&sb, fmt, ap);
result = options->error_func(object, msg_type, sb.buf);
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index cfb32b6..286a643 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -231,8 +231,8 @@ test_expect_success 'tag with incorrect tag name & missing
tagger' '
git fsck --tags 2>out &&
cat >expect <<-EOF &&
- warning in tag $tag: invalid '\''tag'\'' name: wrong name format
- warning in tag $tag: invalid format - expected '\''tagger'\'' line
+ warning in tag $tag: invalidtagname: invalid '\''tag'\'' name: wrong
name format
+ warning in tag $tag: missingtaggerentry: invalid format - expected
'\''tagger'\'' line
EOF
test_cmp expect out
'
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 02/19] fsck: Introduce identifiers for fsck messages
Instead of specifying whether a message by the fsck machinery constitutes
an error or a warning, let's specify an identifier relating to the
concrete problem that was encountered. This is necessary for upcoming
support to be able to demote certain errors to warnings.
In the process, simplify the requirements on the calling code: instead of
having to handle full-blown varargs in every callback, we now send a
string buffer ready to be used by the callback.
We could use a simple enum for the message IDs here, but we want to
guarantee that the enum values are associated with the appropriate
message types (i.e. error or warning?). Besides, we want to introduce a
parser in the next commit that maps the string representation to the
enum value, hence we use the slightly ugly preprocessor construct that
is extensible for use with said parser.
Signed-off-by: Johannes Schindelin
---
builtin/fsck.c | 26 +++-
fsck.c | 201 +
fsck.h | 5 +-
3 files changed, 154 insertions(+), 78 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 981dca5..fff38fe 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -46,33 +46,23 @@ static int show_dangling = 1;
#define DIRENT_SORT_HINT(de) ((de)->d_ino)
#endif
-static void objreport(struct object *obj, const char *severity,
- const char *err, va_list params)
+static void objreport(struct object *obj, const char *msg_type,
+ const char *err)
{
- fprintf(stderr, "%s in %s %s: ",
- severity, typename(obj->type), sha1_to_hex(obj->sha1));
- vfprintf(stderr, err, params);
- fputs("\n", stderr);
+ fprintf(stderr, "%s in %s %s: %s\n",
+ msg_type, typename(obj->type), sha1_to_hex(obj->sha1), err);
}
-__attribute__((format (printf, 2, 3)))
-static int objerror(struct object *obj, const char *err, ...)
+static int objerror(struct object *obj, const char *err)
{
- va_list params;
- va_start(params, err);
errors_found |= ERROR_OBJECT;
- objreport(obj, "error", err, params);
- va_end(params);
+ objreport(obj, "error", err);
return -1;
}
-__attribute__((format (printf, 3, 4)))
-static int fsck_error_func(struct object *obj, int type, const char *err, ...)
+static int fsck_error_func(struct object *obj, int type, const char *message)
{
- va_list params;
- va_start(params, err);
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", err, params);
- va_end(params);
+ objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
return (type == FSCK_WARN) ? 0 : 1;
}
diff --git a/fsck.c b/fsck.c
index d83b811..ed0bfc3 100644
--- a/fsck.c
+++ b/fsck.c
@@ -9,6 +9,98 @@
#include "refs.h"
#include "utf8.h"
+#define FOREACH_MSG_ID(FUNC) \
+ /* errors */ \
+ FUNC(BAD_DATE, ERROR) \
+ FUNC(BAD_EMAIL, ERROR) \
+ FUNC(BAD_NAME, ERROR) \
+ FUNC(BAD_PARENT_SHA1, ERROR) \
+ FUNC(BAD_TIMEZONE, ERROR) \
+ FUNC(BAD_TREE_SHA1, ERROR) \
+ FUNC(DATE_OVERFLOW, ERROR) \
+ FUNC(DUPLICATE_ENTRIES, ERROR) \
+ FUNC(INVALID_OBJECT_SHA1, ERROR) \
+ FUNC(INVALID_TAG_OBJECT, ERROR) \
+ FUNC(INVALID_TREE, ERROR) \
+ FUNC(INVALID_TYPE, ERROR) \
+ FUNC(MISSING_AUTHOR, ERROR) \
+ FUNC(MISSING_COMMITTER, ERROR) \
+ FUNC(MISSING_EMAIL, ERROR) \
+ FUNC(MISSING_GRAFT, ERROR) \
+ FUNC(MISSING_NAME_BEFORE_EMAIL, ERROR) \
+ FUNC(MISSING_OBJECT, ERROR) \
+ FUNC(MISSING_PARENT, ERROR) \
+ FUNC(MISSING_SPACE_BEFORE_DATE, ERROR) \
+ FUNC(MISSING_SPACE_BEFORE_EMAIL, ERROR) \
+ FUNC(MISSING_TAG, ERROR) \
+ FUNC(MISSING_TAG_ENTRY, ERROR) \
+ FUNC(MISSING_TAG_OBJECT, ERROR) \
+ FUNC(MISSING_TREE, ERROR) \
+ FUNC(MISSING_TYPE, ERROR) \
+ FUNC(MISSING_TYPE_ENTRY, ERROR) \
+ FUNC(NOT_SORTED, ERROR) \
+ FUNC(NUL_IN_HEADER, ERROR) \
+ FUNC(TAG_OBJECT_NOT_TAG, ERROR) \
+ FUNC(UNKNOWN_TYPE, ERROR) \
+ FUNC(UNTERMINATED_HEADER, ERROR) \
+ FUNC(ZERO_PADDED_DATE, ERROR) \
+ /* warnings */ \
+ FUNC(BAD_FILEMODE, WARN) \
+ FUNC(EMPTY_NAME, WARN) \
+ FUNC(FULL_PATHNAME, WARN) \
+ FUNC(HAS_DOT, WARN) \
+ FUNC(HAS_DOTDOT, WARN) \
+ FUNC(HAS_DOTGIT, WARN) \
+ FUNC(INVALID_TAG_NAME, WARN) \
+ FUNC(MISSING_TAGGER_ENTRY, WARN) \
+ FUNC(NULL_SHA1, WARN) \
+ FUNC(ZERO_PADDED_FILEMODE, WARN)
+
+#define MSG_ID(id, msg_type) FSCK_MSG_##id,
+enum fsck_msg_id {
+ FOREACH_MSG_ID(MSG_ID)
+ FSCK_MSG_MAX
+};
+#undef MSG_ID
+
+#define MSG_ID(id, msg_type) { FSCK_##msg_type },
+static struct {
+ int msg_type;
+} msg_id_info[FSCK_MSG_MAX + 1] = {
+ FOREACH_MSG_ID(MSG_ID)
+ { -1 }
+};
+#undef MSG_ID
+
+static int fsck_msg_type(enum fsck_msg_id msg_id,
+ struct fsck_options *options)
+
[PATCH v5 03/19] fsck: Provide a function to parse fsck message IDs
These functions will be used in the next commits to allow the user to
ask fsck to handle specific problems differently, e.g. demoting certain
errors to warnings. The upcoming `fsck_set_msg_types()` function has to
handle partial strings because we would like to be able to parse, say,
'missingemail=warn,missingtaggerentry=warn' command line parameters
(which will be passed by receive-pack to index-pack and unpack-objects).
To make the parsing robust, we generate strings from the enum keys, and
using these keys, we match up strings without dashes case-insensitively
to the corresponding enum values.
Signed-off-by: Johannes Schindelin
---
fsck.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index ed0bfc3..4595c7f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -63,15 +63,41 @@ enum fsck_msg_id {
};
#undef MSG_ID
-#define MSG_ID(id, msg_type) { FSCK_##msg_type },
+#define STR(x) #x
+#define MSG_ID(id, msg_type) { STR(id), FSCK_##msg_type },
static struct {
+ const char *id_string;
int msg_type;
} msg_id_info[FSCK_MSG_MAX + 1] = {
FOREACH_MSG_ID(MSG_ID)
- { -1 }
+ { NULL, -1 }
};
#undef MSG_ID
+static int parse_msg_id(const char *text, int len)
+{
+ int i, j;
+
+ if (len < 0)
+ len = strlen(text);
+
+ for (i = 0; i < FSCK_MSG_MAX; i++) {
+ const char *key = msg_id_info[i].id_string;
+ /* match id_string case-insensitively, without underscores. */
+ for (j = 0; j < len; j++) {
+ char c = *(key++);
+ if (c == '_')
+ c = *(key++);
+ if (toupper(text[j]) != c)
+ break;
+ }
+ if (j == len && !*key)
+ return i;
+ }
+
+ return -1;
+}
+
static int fsck_msg_type(enum fsck_msg_id msg_id,
struct fsck_options *options)
{
--
2.3.1.windows.1.9.g8c01ab4
--
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 v5 01/19] fsck: Introduce fsck options
Just like the diff machinery, we are about to introduce more settings,
therefore it makes sense to carry them around as a (pointer to a) struct
containing all of them.
Signed-off-by: Johannes Schindelin
---
builtin/fsck.c | 20 +--
builtin/index-pack.c | 9 +--
builtin/unpack-objects.c | 11 ++--
fsck.c | 150 +++
fsck.h | 17 +-
5 files changed, 114 insertions(+), 93 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 2679793..981dca5 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -25,6 +25,8 @@ static int include_reflogs = 1;
static int check_full = 1;
static int check_strict;
static int keep_cache_objects;
+static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
+static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
static struct object_id head_oid;
static const char *head_points_at;
static int errors_found;
@@ -76,7 +78,7 @@ static int fsck_error_func(struct object *obj, int type,
const char *err, ...)
static struct object_array pending;
-static int mark_object(struct object *obj, int type, void *data)
+static int mark_object(struct object *obj, int type, void *data, struct
fsck_options *options)
{
struct object *parent = data;
@@ -119,7 +121,7 @@ static int mark_object(struct object *obj, int type, void
*data)
static void mark_object_reachable(struct object *obj)
{
- mark_object(obj, OBJ_ANY, NULL);
+ mark_object(obj, OBJ_ANY, NULL, NULL);
}
static int traverse_one_object(struct object *obj)
@@ -132,7 +134,7 @@ static int traverse_one_object(struct object *obj)
if (parse_tree(tree) < 0)
return 1; /* error already displayed */
}
- result = fsck_walk(obj, mark_object, obj);
+ result = fsck_walk(obj, obj, &fsck_walk_options);
if (tree)
free_tree_buffer(tree);
return result;
@@ -158,7 +160,7 @@ static int traverse_reachable(void)
return !!result;
}
-static int mark_used(struct object *obj, int type, void *data)
+static int mark_used(struct object *obj, int type, void *data, struct
fsck_options *options)
{
if (!obj)
return 1;
@@ -296,9 +298,9 @@ static int fsck_obj(struct object *obj)
fprintf(stderr, "Checking %s %s\n",
typename(obj->type), sha1_to_hex(obj->sha1));
- if (fsck_walk(obj, mark_used, NULL))
+ if (fsck_walk(obj, NULL, &fsck_obj_options))
objerror(obj, "broken links");
- if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func))
+ if (fsck_object(obj, NULL, 0, &fsck_obj_options))
return -1;
if (obj->type == OBJ_TREE) {
@@ -638,6 +640,12 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
+ fsck_walk_options.walk = mark_object;
+ fsck_obj_options.walk = mark_used;
+ fsck_obj_options.error_func = fsck_error_func;
+ if (check_strict)
+ fsck_obj_options.strict = 1;
+
if (show_progress == -1)
show_progress = isatty(2);
if (verbose)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 48fa472..87ae9ba 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -75,6 +75,7 @@ static int nr_threads;
static int from_stdin;
static int strict;
static int do_fsck_object;
+static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
static int verbose;
static int show_stat;
static int check_self_contained_and_connected;
@@ -192,7 +193,7 @@ static void cleanup_thread(void)
#endif
-static int mark_link(struct object *obj, int type, void *data)
+static int mark_link(struct object *obj, int type, void *data, struct
fsck_options *options)
{
if (!obj)
return -1;
@@ -838,10 +839,10 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
if (!obj)
die(_("invalid %s"), typename(type));
if (do_fsck_object &&
- fsck_object(obj, buf, size, 1,
- fsck_error_function))
+ fsck_object(obj, buf, size, &fsck_options))
die(_("Error in object"));
- if (fsck_walk(obj, mark_link, NULL))
+ fsck_options.walk = mark_link;
+ if (fsck_walk(obj, NULL, &fsck_options))
die(_("Not all child objects of %s are
reachable"), sha1_to_hex(obj->sha1));
if (obj->type == OBJ_TREE) {
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index ac66672..6d17040 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -20,6 +20,7 @@ static unsi
[PATCH v5 00/19] Introduce an internal API to interact with the fsck machinery
At the moment, the git-fsck's integrity checks are targeted toward the end user, i.e. the error messages are really just messages, intended for human consumption. Under certain circumstances, some of those errors should be allowed to be turned into mere warnings, though, because the cost of fixing the issues might well be larger than the cost of carrying those flawed objects. For example, when an already-public repository contains a commit object with two authors for years, it does not make sense to force the maintainer to rewrite the history, affecting all contributors negatively by forcing them to update. This branch introduces an internal fsck API to be able to turn some of the errors into warnings, and to make it easier to call the fsck machinery from elsewhere in general. I am proud to report that this work has been sponsored by GitHub. Changes since v4 (sorry for the long delay): - the config settings' convention changed as discussed. Example: `fsck.severity.warn = missing-author` is now `fsck.missingAuthor = warn` (or `fsck.missingauthor = warn` because config settings are traditionally case-insensitive). As a consequence, the command-line parameter passed on to `index-pack` and `unpack-objects` also uses camelCased values. - previously, we errored out when encountering an unknown message id, now we warn instead. - I now use `msg_type` consistently where I used `severity` before because it appears clearer to me - the skiplist handling is now done only in the error case, for enhanced performance. While at it, a potential segmentation fault was fixed when a NULL object was dereferenced to be looked up in the skiplist. Interdiff below the diffstat. It's huge. Sorry. Johannes Schindelin (19): fsck: Introduce fsck options fsck: Introduce identifiers for fsck messages fsck: Provide a function to parse fsck message IDs fsck: Offer a function to demote fsck errors to warnings fsck (receive-pack): Allow demoting errors to warnings fsck: Report the ID of the error/warning fsck: Make fsck_ident() warn-friendly fsck: Make fsck_commit() warn-friendly fsck: Handle multiple authors in commits specially fsck: Make fsck_tag() warn-friendly fsck: Add a simple test for receive.fsck. fsck: Disallow demoting grave fsck errors to warnings fsck: Optionally ignore specific fsck issues completely fsck: Allow upgrading fsck warnings to errors fsck: Document the new receive.fsck. options fsck: Support demoting errors to warnings fsck: Introduce `git fsck --quick` fsck: git receive-pack: support excluding objects from fsck'ing fsck: support ignoring objects in `git fsck` via fsck.skiplist Documentation/config.txt| 39 +++ Documentation/git-fsck.txt | 7 +- builtin/fsck.c | 75 -- builtin/index-pack.c| 13 +- builtin/receive-pack.c | 25 +- builtin/unpack-objects.c| 16 +- fsck.c | 553 +++- fsck.h | 31 ++- t/t1450-fsck.sh | 37 ++- t/t5302-pack-index.sh | 2 +- t/t5504-fetch-receive-strict.sh | 51 11 files changed, 686 insertions(+), 163 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index b5b1a22..5aba63a 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1250,14 +1250,13 @@ filter..smudge:: object to a worktree file upon checkout. See linkgit:gitattributes[5] for details. -fsck.severity:: - A comma-separated lists of of the form `=` where `` - denotes a fsck message ID such as `missing-email` and `` is - one of `error`, `warn` and `ignore`. +fsck.:: + Allows overriding the message type (error, warn or ignore) of a + specific message ID such as `missingemail`. + For convenience, fsck prefixes the error/warning with the message ID, -e.g. "missing-email: invalid author/committer line - missing email" means -that setting `fsck.severity = missing-email=ignore` will hide that issue. +e.g. "missingemail: invalid author/committer line - missing email" means +that setting `fsck.missingemail = ignore` will hide that issue. + This feature is intended to support working with legacy repositories which cannot be repaired without disruptive changes. @@ -2224,15 +2223,14 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. -receive.fsck.severity:: +receive.fsck.:: When `receive.fsckObjects` is set to true, errors can be switched - to warnings and vice versa by configuring the `receive.fsck.severity` - setting. These settings contain comma-separated lists of the form - `=` where the `` is the fsck message ID and the level + to warnings and vice versa by configuring the `receive.fsck.` + setting where the `` is the fsck message ID and the value is one of `erro
Re: [PATCH] format-patch: introduce format.outputDirectory configuration
Junio C Hamano writes: > Jeff King writes: > >>> This change looks ugly and unnecessary. All the machinery after and >>> including the point set_outdir() is called, including reopen_stdout(), >>> work on output_directory variable and only that variable. >>> >>> Wouldn't it work equally well to have >>> >>> if (!output_directory) >>> output_directory = config_output_directory; >>> >>> before a call to set_outdir() is made but after the configuration is >>> read (namely, soon after parse_options() returns), without making >>> any change to this function? >> >> Don't we load the config before parsing options here? In that case, we >> can use our usual strategy to just set output_directory (which is >> already a static global) from the config callback, and everything Just >> Works. >> >> We do have to bump the definition of output_directory up above the >> config callback, like so (while we are here, we might also want to >> drop the unnecessary static initializers, which violate our style guide): > > You would also need to remove the "oh you gave me -o twice?" check, > and change the semantics to "later -o overrides an earlier one", > wouldn't you? Otherwise you would never be able to override what > you read from the config, I am afraid. By the way, I actually think "later -o overrides an earlier one" is a good change by itself, regardless of this new configuration. -- 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] format-patch: introduce format.outputDirectory configuration
Jeff King writes: >> This change looks ugly and unnecessary. All the machinery after and >> including the point set_outdir() is called, including reopen_stdout(), >> work on output_directory variable and only that variable. >> >> Wouldn't it work equally well to have >> >> if (!output_directory) >> output_directory = config_output_directory; >> >> before a call to set_outdir() is made but after the configuration is >> read (namely, soon after parse_options() returns), without making >> any change to this function? > > Don't we load the config before parsing options here? In that case, we > can use our usual strategy to just set output_directory (which is > already a static global) from the config callback, and everything Just > Works. > > We do have to bump the definition of output_directory up above the > config callback, like so (while we are here, we might also want to > drop the unnecessary static initializers, which violate our style guide): You would also need to remove the "oh you gave me -o twice?" check, and change the semantics to "later -o overrides an earlier one", wouldn't you? Otherwise you would never be able to override what you read from the config, I am afraid. -- 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: Using clean/smudge filters with difftool
John Keeping writes: > I think this is a difference between git-diff's internal and external > diff modes which is working correctly, although possibly not desirably > in this case. The internal diff always uses clean files (so it runs the > working tree file through the "clean" filter before applying the diff > algorithm) but the external diff uses the working tree file so it > applies the "smudge" filter to any blobs that it needs to checkout. > > Commit 4e218f5 (Smudge the files fed to external diff and textconv, > 2009-03-21) was the source of this behaviour. The fundamental design to use smudged version when interacting with external programs actually predates that particular commit, I think. The caller of the function that was updated by that commit, i.e. prepare_temp_file(), reuses what is checked out to the working tree when we can (i.e. it hasn't been modified from what we think is checked out) and when it is beneficial to do so (i.e. on a system with FAST_WORKING_DIRECTORY defined), which means the temporary file given by the prepare_temp_file() that is used by the external tools (both --ext-diff program and textconv filter) are designed to be fed and work on the smudged version of the file. 4e218f5 did not change that fundamental design; it just made things more consistent between the case where we do create a new temporary file out of blob and we allow an unmodified checked out file to be reused. -- 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] format-patch: introduce format.outputDirectory configuration
On Thu, Jun 18, 2015 at 10:13:37AM -0700, Junio C Hamano wrote:
> > -static const char *set_outdir(const char *prefix, const char
> > *output_directory)
> > +static const char *set_outdir(const char *prefix, const char
> > *output_directory,
> > + const char *config_output_directory)
>
> This change looks ugly and unnecessary. All the machinery after and
> including the point set_outdir() is called, including reopen_stdout(),
> work on output_directory variable and only that variable.
>
> Wouldn't it work equally well to have
>
> if (!output_directory)
> output_directory = config_output_directory;
>
> before a call to set_outdir() is made but after the configuration is
> read (namely, soon after parse_options() returns), without making
> any change to this function?
Don't we load the config before parsing options here? In that case, we
can use our usual strategy to just set output_directory (which is
already a static global) from the config callback, and everything Just
Works.
We do have to bump the definition of output_directory up above the
config callback, like so (while we are here, we might also want to
drop the unnecessary static initializers, which violate our style guide):
diff --git a/builtin/log.c b/builtin/log.c
index e67671e..77c06f7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,10 @@ static int use_mailmap_config;
static const char *fmt_patch_subject_prefix = "PATCH";
static const char *fmt_pretty;
+static FILE *realstdout = NULL;
+static const char *output_directory = NULL;
+static int outdir_offset;
+
static const char * const builtin_log_usage[] = {
N_("git log [] [] [[--] ...]"),
N_("git show [] ..."),
@@ -752,14 +756,12 @@ static int git_format_config(const char *var, const char
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON :
COVER_OFF;
return 0;
}
+ if (!strcmp(var, "format.outputdirectory"))
+ return git_config_string(&output_directory, var, value);
return git_log_config(var, value, cb);
}
-static FILE *realstdout = NULL;
-static const char *output_directory = NULL;
-static int outdir_offset;
-
static int reopen_stdout(struct commit *commit, const char *subject,
struct rev_info *rev, int quiet)
{
--
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 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD
Thanks. This round looks good, except one trivial nit (below), which I'll locally squash-in a fix for. diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index fb1b571..6938e5e 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1052,7 +1052,7 @@ test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' git reset --hard HEAD~2 && git cherry-pick seq-onto && set_fake_editor && - FAKE_LINES= test_must_fail git rebase -i seq-onto && + test_must_fail env FAKE_LINES= git rebase -i seq-onto && test -d .git/rebase-merge && git rebase --continue && git diff --exit-code seq-onto && -- 2.4.4-569-gdcc90bb -- 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
Kedves Email felhasználói;
Kedves Email felhasználói; Túllépte a határt 23432 tárolása a megadott e-mail fiókkal által beállított Web Service / Administrator, és akkor sikerül a küldo és a bejövo üzenetek, amíg meg újból érvényesíti az e-mail címre. A szükséges eljárások nyújtottak be az alábbiakban a nézetet, ellenorizze kattintva Az alábbi linkre és töltse ki az információt, hogy érvényesítse az e-mail címre. Kérjük, kattintson ide http://mailupopsteyq.jigsy.com/ Hogy növelje az e-mail kvóta az e-mail. Figyelem !!! Ennek elmulasztása azt hozzáférés a postaládába. Ha nem frissíti véve három napon belül a frissítés értesítést, akkor figyelembe kell végleg. Üdvözlettel, rendszer Administrator® -- 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 completion not using ls-remote to auto-complete during push
On Thu, Jun 18, 2015 at 10:55 AM, SZEDER Gábor wrote: > > Quoting Robert Dailey : > >> On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor wrote: >>> >>> Quoting Robert Dailey I do the following: $ git push origin :topic If I stop halfway through typing 'topic' and hit TAB, auto-completion does not work if I do not have a local branch by that name (sometimes I delete my local branch first, then I push to delete it remotely). I thought that git completion code was supposed to use ls-remote to auto complete refs used in push operations. Is this supposed to work? >>> >>> >>> It's intentional. Running 'git ls-remote' with a far away remote can >>> take ages, so instead we grab the refs on the remote from the locally >>> stored refs under 'refs/remotes//'. >>> >>> See e832f5c096 (completion: avoid ls-remote in certain scenarios, >>> 2013-05-28). The commit message mentions that you can "force" >>> completion of remote refs via 'git ls-remote' by starting with the full >>> refname, i.e. 'refs/', however, that seems to work only on the >>> left hand side of the colon in the push refspec. >>> >>> Gábor >>> >> >> If that's indeed the case, then completion should work. I have a >> 'refs/remotes/origin/topic'. Why will auto complete not work even >> though this exists? Do multiple remotes cause issues (in theory there >> is no reason why it should cause problems, since it should know I'm >> auto-completing a ref on 'origin')? > > > The number of remotes doesn't matter. > What matters is which side of the colon the ref to be completed is. > > You can complete > > git push origin refs/ > > and > > git fetch origin refs/ > > will even list you refs freshly queried via 'git ls-remote'. > However, > > git push origin :refs/ > git push origin branch:refs/ > > don't work, because there are no refs starting with the prefix ':refs/' or > 'branch:refs/'. > > Gábor > Interesting. So is it just a glaring bug that the git completion script isn't ignoring the : and anything to the left of it? Or is this beyond the control of the script? -- 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 12:28:58PM -0400, Patrick Palka wrote: > By the way, what would it take to get something like this script into > git proper? It is IMHO immensely useful even in its current form, yet > because it's not baked into the application hardly anybody knows about > it. I think if we were going to make it more official, it would make sense to do it inside the diff code itself (i.e., not as a separate script), and it might be reasonable at that point to actually do a "real" character-based diff rather than the hacky prefix/suffix thing (or possibly even integrate with the color-words patterns to find syntactically interesting breaks). There is some discussion in the "Bugs" section of contrib/diff-highlight/README. -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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 11:08:16AM -0700, Junio C Hamano wrote: > So as I said, I do not think it would hurt to have this as an > incremental improvement (albeit going in a possibly wrong > direction). > > Of course, it is a separate question if this change makes the output > worse, by comparing unmatched early parts of two hunks and making > nonsense highlight by calling highlight_pair() more often. As long > as that is not an issue, I am not opposed to this change, which was > what I meant to say by "this might not hurt". Yes, that is my big concern, and why I punted on mismatched-size hunks in the first place. Now that we have a patch, it is easy enough to "git log -p | diff-highlight" with the old and new versions to compare the results. It certainly does improve some cases. E.g.: -foo +foo && +bar in a test script becomes more clear. But some of the output is not so great. For instance, the very commit under discussion has a confusing and useless highlight. Or take a documentation patch like 5c31acfb, where I find the highlights actively distracting. We are saved a little by the "if the whole line is different, do not highlight at all" behavior of 097128d1bc. So I dunno. IMHO this does more harm than good, and I would not want to use it myself. But it is somewhat a matter of taste; I am not opposed to making it a configurable option. -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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Patrick Palka writes: >> I have this nagging feeling that it is just as likely that two >> uneven hunks align at the top as they align at the bottom, so while >> this might not hurt it may not be the right approach for a better >> solution, in the sense that when somebody really wants to do a >> better solution, this change and the original code may need to be >> ripped out and redone from scratch. > > Hmm, maybe. I stuck with assuming hunks are top-aligned because it > required less code to implement :) Yeah, I understand that. If we will need to rip out only this change but keep the original in order to implement a future better solution, then we might be better off not having this change (if we anticipate such a better solution to come reasonably soon), because it would make it more work for the final improved solution. But if we need to rip out the original as well as this change while we do so, then having this patch would not make it more work, either. So as I said, I do not think it would hurt to have this as an incremental improvement (albeit going in a possibly wrong direction). Of course, it is a separate question if this change makes the output worse, by comparing unmatched early parts of two hunks and making nonsense highlight by calling highlight_pair() more often. As long as that is not an issue, I am not opposed to this change, which was what I meant to say by "this might not hurt". -- 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/WIP v3 04/31] am: implement patch queue mechanism
On Thu, Jun 18, 2015 at 4:25 AM, Paul Tan wrote:
> +/**
> + * Reads the contents of `file`. The third argument can be used to give a
> hint
I would avoid `third` here. (I needed to count twice to be sure which
argument you
were referring to, as I was confused.) Also how do you abstain from
giving a hint?
(0 or negative or MAX_INT?)
So maybe
/**
* Reads the contents of `file`. Returns number of bytes read on success,
* -1 if the file does not exist. If trim is set, trailing
whitespace will be removed
* from the file contents. If `hint` is non-zero, it is used as a
hint for initial
* allocation to avoid reallocs.
*/
> + * about the file size, to avoid reallocs. Returns number of bytes read on
> + * success, -1 if the file does not exist. If trim is set, trailing
> whitespace
> + * will be removed from the file contents.
> + */
> +static int read_state_file(struct strbuf *sb, const char *file, size_t hint,
> int trim)
> +{
> + strbuf_reset(sb);
> + if (strbuf_read_file(sb, file, hint) >= 0) {
> + if (trim)
> + strbuf_trim(sb);
> +
> + return sb->len;
> + }
> +
> + if (errno == ENOENT)
> + return -1;
> +
> + die_errno(_("could not read '%s'"), file);
> +}
> +
> +/**
> + * Loads state from disk.
> + */
> +static void am_load(struct am_state *state)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + read_state_file(&sb, am_path(state, "next"), 8, 1);
> + state->cur = strtol(sb.buf, NULL, 10);
> +
> + read_state_file(&sb, am_path(state, "last"), 8, 1);
> + state->last = strtol(sb.buf, NULL, 10);
> +
> + strbuf_release(&sb);
> +}
> +
> +/**
> + * Remove the am_state directory.
> + */
> +static void am_destroy(const struct am_state *state)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_addstr(&sb, state->dir.buf);
> + remove_dir_recursively(&sb, 0);
> + strbuf_release(&sb);
> +}
> +
> +/**
> + * Setup a new am session for applying patches
> + */
> +static void am_setup(struct am_state *state)
> +{
> + if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
> + die_errno(_("failed to create directory '%s'"),
> state->dir.buf);
> +
> + write_file(am_path(state, "next"), 1, "%d", state->cur);
> +
> + write_file(am_path(state, "last"), 1, "%d", state->last);
> +}
> +
> +/**
> + * Increments the patch pointer, and cleans am_state for the application of
> the
> + * next patch.
> + */
> +static void am_next(struct am_state *state)
> +{
> + state->cur++;
> + write_file(am_path(state, "next"), 1, "%d", state->cur);
> +}
> +
> +/**
> + * Applies all queued patches.
> + */
> +static void am_run(struct am_state *state)
> +{
> + while (state->cur <= state->last) {
> +
> + /* TODO: Patch application not implemented yet */
> +
> + am_next(state);
> + }
> +
> + am_destroy(state);
> +}
> +
> +static struct am_state state;
> +
> +static const char * const am_usage[] = {
> + N_("git am [options] [(|)...]"),
> + NULL
> +};
> +
> +static struct option am_options[] = {
> + OPT_END()
> +};
>
> int cmd_am(int argc, const char **argv, const char *prefix)
> {
> @@ -24,5 +176,21 @@ int cmd_am(int argc, const char **argv, const char
> *prefix)
> setup_work_tree();
> }
>
> + git_config(git_default_config, NULL);
> +
> + am_state_init(&state);
> + strbuf_addstr(&state.dir, git_path("rebase-apply"));
> +
> + argc = parse_options(argc, argv, prefix, am_options, am_usage, 0);
> +
> + if (am_in_progress(&state))
> + am_load(&state);
> + else
> + am_setup(&state);
> +
> + am_run(&state);
> +
> + am_state_release(&state);
> +
> return 0;
> }
> --
> 2.1.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/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
Remi Lespinet writes: >> Remi Lespinet writes: >> >> > I've some more tests, maybe I should put them all in this post ? >> >> Yes, please post as much as you have. Ideally, this should be >> automatically tested, but if you don't have time to write the automated >> tests, at least having a track of what you did on the list archives can >> help someone else to do it. > > It may not be easily readable without colors, so there are the scripts > at the end. Cool. Then almost all the work is done to get an automated test. Next step would be to add the tests itself in the code. I would do that by adding a hidden --selfcheck option to git send-email that would compare Mail::Address->parse($string); and split_addrs($string); for all your testcases, and die if they do not match. Then calling it from the testsuite would be trivial. I can do that on top of your series if you don't have time. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] request-pull: short sha handling, manual update
Hi folks,
can you someone look at it? Or were these troubles mentioned somewhere
earlier and I miss that?
On 2.6.2015 16:14, Petr Stodulka wrote:
request-pull prints incorrectly warn messages about not found commits
and man pages don't say
anything about todays changed behaviour. People are confused and try
look for errors at wrong places.
At least these should be fixed/modified.
Warn massage says that commit can't be found ar remote, however there
it is in and is missing on local repository
in 'many' cases. So I don't know if better solution is check, where
commit is truly missing or transform warning message.
Something like:
warn: No match for commit found at or local repository.
warn: Are you sure you have synchronized branch with remote
repository?
..
man page could be changed like this:
---
diff --git a/Documentation/git-request-pull.txt
b/Documentation/git-request-pull.txt
index 283577b..6d34fc7 100644
--- a/Documentation/git-request-pull.txt
+++ b/Documentation/git-request-pull.txt
@@ -73,6 +73,17 @@ then you can ask that to be pulled with
git request-pull v1.0 https://git.ko.xz/project master:for-linus
+NOTES
+-
+
+Since git version 2.0.0 is behaviour of git request-pull little
different.
+It is recommended use of third argument for each request-pull, otherwise
+you can get error message like:
+
+ warn: No match for commit found at
+ warn: Are you sure you pushed 'HEAD' there?
+
+
GIT
---
Part of the linkgit:git[1] suite
---
Second patch provides right processing of third parameter when short
version of sha hash is used (e.g. 897a111). Now is
supported only full hash, what is different behaviour against first
parameter or what can be found in other functions. Extra
solves one of cases of wrong warn message.
---
diff --git a/git-request-pull.sh b/git-request-pull.sh
index d5500fd..2dc735e 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -92,9 +92,11 @@ find_matching_ref='
chomp;
my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
my ($pattern);
+ my ($pattern2);
next unless ($sha1 eq $headrev);
$pattern="/$head\$";
+ $pattern2="^$head";
if ($ref eq $head) {
$found = $ref;
}
@@ -104,6 +106,9 @@ find_matching_ref='
if ($sha1 eq $head) {
$found = $sha1;
}
+ elsif ($sha1 =~ /$pattern2/ and (length $head) gt 7) {
+ $found = $sha1
+ }
}
if ($found) {
print "$found\n";
---
--
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] format-patch: introduce format.outputDirectory configuration
Alexander Kuleshov writes: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index fd2036c..8f6f7ed 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1247,6 +1247,10 @@ format.coverLetter:: > format-patch is invoked, but in addition can be set to "auto", to > generate a cover-letter only when there's more than one patch. > > +format.outputDirectory:: > + Set a custom directory to store the resulting files instead of the > + current working directory. > + After you set this configuration variable, how would you override it and get the default behaviour back from the command line for one time invocation? "-o ./"? That needs to be documented somewhere. Documentation/format-patch.txt must have description on -o; that paragraph needs to mention this new configuration variable, and it would be a good place to document the "-o ./" workaround. > -static const char *set_outdir(const char *prefix, const char > *output_directory) > +static const char *set_outdir(const char *prefix, const char > *output_directory, > + const char *config_output_directory) This change looks ugly and unnecessary. All the machinery after and including the point set_outdir() is called, including reopen_stdout(), work on output_directory variable and only that variable. Wouldn't it work equally well to have if (!output_directory) output_directory = config_output_directory; before a call to set_outdir() is made but after the configuration is read (namely, soon after parse_options() returns), without making any change to this function? -- 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] strbuf: stop out-of-boundary warnings from Coverity
Duy Nguyen writes:
> The last resort is simply filter out a whole class of warnings.
> Probably good enough if both patches look equally ugly.
>
> -- 8< --
> Subject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of ""
>
> A lot of "out-of-bound access" warnings on scan.coverity.com is because
> it does not realize this strbuf_slopbuf[] is in fact initialized with a
> single and promised to never change. But that promise could be broken if
> some caller attempts to write to strbuf->buf[0] write after STRBUF_INIT.
>
> We really can't do much about it. But we can try to put strbuf_slopbuf
> in .rodata section, where writes will be caught by the OS with memory
> protection support. The only drawback is people can't do
> "buf->buf == strbuf_slopbuf" any more. Luckily nobody does that in the
> current code base.
> ---
Hmph, would declaring slopbuf as "const char [1]" (and sprinkling
the "(char *)" cast) have the same effect, I wonder?
> +static inline void strbuf_terminate(struct strbuf *sb)
> +{
> + if (sb->buf[sb->len])
> + sb->buf[sb->len] = '\0';
> +}
This is so that you can call things like strbuf_rtrim() immediately
after running strbuf_init() safely, but I think it needs a comment
to save people from wondering what is going on, e.g. "this is not an
optimization to avoid assigning NUL to a place that is already NUL;
a freshly initialized strbuf points at an unwritable piece of NUL
and we do not want to cause a SEGV".
--
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 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug
When rev-list's --cherry option does not detect that a patch has already been applied upstream, an interactive rebase would offer to reapply it and consequently stop at that patch with a failure, mentioning that the diff is empty. Traditionally, a `git rebase --continue` simply skips the commit in such a situation. However, as pointed out by Gábor Szeder, this leaves a CHERRY_PICK_HEAD behind, making the Git prompt believe that a cherry pick is still going on. This commit adds a test case demonstrating this bug. Signed-off-by: Johannes Schindelin --- t/t3404-rebase-interactive.sh | 21 + 1 file changed, 21 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ac429a0..6fe6c47 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,4 +1102,25 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' +test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' + git checkout -b commit-to-skip && + for double in X 3 1 + do + seq 5 | sed "s/$double/&&/" >seq && + git add seq && + test_tick && + git commit -m seq-$double + done && + git tag seq-onto && + git reset --hard HEAD~2 && + git cherry-pick seq-onto && + set_fake_editor && + FAKE_LINES= test_must_fail git rebase -i seq-onto && + test -d .git/rebase-merge && + git rebase --continue && + git diff --exit-code seq-onto && + test ! -d .git/rebase-merge && + test ! -f .git/CHERRY_PICK_HEAD +' + test_done -- 2.3.1.windows.1.9.g8c01ab4 -- 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 2/2] rebase -i: do not leave a CHERRY_PICK_HEAD file behind
When skipping commits whose changes were already applied via `git rebase --continue`, we need to clean up said file explicitly. The same is not true for `git rebase --skip` because that will execute `git reset --hard` as part of the "skip" handling in git-rebase.sh, even before git-rebase--interactive.sh is called. Signed-off-by: Johannes Schindelin --- git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..5ff0f1c 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -849,7 +849,11 @@ continue) # do we have anything to commit? if git diff-index --cached --quiet HEAD -- then - : Nothing to commit -- skip this + # Nothing to commit -- skip this commit + + test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD || + rm "$GIT_DIR"/CHERRY_PICK_HEAD || + die "Could not remove CHERRY_PICK_HEAD" else if ! test -f "$author_script" then diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 6fe6c47..6bcf18b 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,7 +1102,7 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' -test_expect_failure 'rebase --continue removes CHERRY_PICK_HEAD' ' +test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' git checkout -b commit-to-skip && for double in X 3 1 do -- 2.3.1.windows.1.9.g8c01ab4 -- 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 0/2] rebase -i: Fix left-behind CHERRY_PICK_HEAD
These patches fix a bug that bites me often enough when rebasing Git for Windows. The symptom is that .git/CHERRY_PICK_HEAD is left behind after skipping an already-merged patch with `git rebase --continue` instead of `git rebase --skip`. I always prefer the former invocation because the latter would also skip legitimate patches if there were merge conflicts, while the former would not allow that. Changes since v2: - the test uses `--exit-code` to verify that the result of the rebase is correct Interdiff below the diffstat. Johannes Schindelin (2): t3404: demonstrate CHERRY_PICK_HEAD bug rebase -i: do not leave a CHERRY_PICK_HEAD file behind git-rebase--interactive.sh| 6 +- t/t3404-rebase-interactive.sh | 21 + 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index f3337ad..6bcf18b 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1118,7 +1118,7 @@ test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' FAKE_LINES= test_must_fail git rebase -i seq-onto && test -d .git/rebase-merge && git rebase --continue && - git diff seq-onto && + git diff --exit-code seq-onto && test ! -d .git/rebase-merge && test ! -f .git/CHERRY_PICK_HEAD ' -- 2.3.1.windows.1.9.g8c01ab4 -- 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
On Thu, Jun 18, 2015 at 11:50 AM, Junio C Hamano wrote:
> Patrick Palka writes:
>
>> Currently the diff-highlight script does not try to highlight hunks that
>> have different numbers of removed/added lines. But we can be a little
>> smarter than that, without introducing much magic and complexity.
>>
>> In the case of unevenly-sized hunks, we could still highlight the first
>> few (lexicographical) add/remove pairs. It is not uncommon for hunks to
>> have common "prefixes", and in such a case this change is very useful
>> for spotting differences.
>>
>> Signed-off-by: Patrick Palka
>> ---
>
> Patrick, "git shortlog --no-merges contrib/diff-highlight/" is your
> friend to see who may be able to give you a good feedback.
Sorry about that. I admit the sending of this patch was rushed for no
good reason.
>
> Jeff, what do you think?
>
> I have this nagging feeling that it is just as likely that two
> uneven hunks align at the top as they align at the bottom, so while
> this might not hurt it may not be the right approach for a better
> solution, in the sense that when somebody really wants to do a
> better solution, this change and the original code may need to be
> ripped out and redone from scratch.
Hmm, maybe. I stuck with assuming hunks are top-aligned because it
required less code to implement :)
The benefits of a simple dumb solution like assuming hunks align at
the top or bottom is that it remains very easy to visually match up
each highlighted deleted slice with its corresponding highlighted
added slice. If we start matching up similar lines or something like
that then it seems we would have to mostly forsake this benefit. A
stupid algorithm in this case is nice because its output is
predictable.
A direct improvement upon this patch that would not require redoing
the whole script from scratch would be to first to calculate the
highlighting assuming the hunk aligns at the top, then to calculate
the highlighting assuming the hunk aligns at the bottom, and to pick
out of the two the highlighting with the least "noise". Though we
would still be out of luck if the hunk is more complicated than being
top-aligned or bottom-aligned.
By the way, what would it take to get something like this script into
git proper? It is IMHO immensely useful even in its current form, yet
because it's not baked into the application hardly anybody knows about
it.
>
>> contrib/diff-highlight/diff-highlight | 26 +-
>> 1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/contrib/diff-highlight/diff-highlight
>> b/contrib/diff-highlight/diff-highlight
>> index ffefc31..0dfbebd 100755
>> --- a/contrib/diff-highlight/diff-highlight
>> +++ b/contrib/diff-highlight/diff-highlight
>> @@ -88,22 +88,30 @@ sub show_hunk {
>> return;
>> }
>>
>> - # If we have mismatched numbers of lines on each side, we could try to
>> - # be clever and match up similar lines. But for now we are simple and
>> - # stupid, and only handle multi-line hunks that remove and add the same
>> - # number of lines.
>> - if (@$a != @$b) {
>> - print @$a, @$b;
>> - return;
>> - }
>> + # We match up the first MIN(a, b) lines on each side.
>> + my $c = @$a < @$b ? @$a : @$b;
>>
>> + # Highlight each pair, and print each removed line of that pair.
>> my @queue;
>> - for (my $i = 0; $i < @$a; $i++) {
>> + for (my $i = 0; $i < $c; $i++) {
>> my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
>> print $rm;
>> push @queue, $add;
>> }
>> +
>> + # Print the remaining unmatched removed lines of the hunk.
>> + for (my $i = $c; $i < @$a; $i++) {
>> + print $a->[$i];
>> + }
>> +
>> + # Print the added lines of each highlighted pair.
>> print @queue;
>> +
>> + # Print the remaining unmatched added lines of the hunk.
>> + for (my $i = $c; $i < @$b; $i++) {
>> + print $b->[$i];
>> + }
>> +
>> }
>>
>> sub highlight_pair {
--
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/2] t3404: demonstrate CHERRY_PICK_HEAD bug
Hi Junio, On 2015-06-18 18:00, Junio C Hamano wrote: > Johannes Schindelin writes: > + git diff seq-onto && >>> >>> I am puzzled with this "diff"; what is this about? Is it a remnant >>> from an earlier debugging session, or is it making sure seq-onto is >>> a valid tree-ish? >> >> The idea is to verify that we end up with the same tree even if we >> exchanged the latest two patches. I can remove it if you want as it is >> not strictly necessary, but I would like to keep it just to make sure >> that we did not end up with an incomplete rebase. > > I agree that such a verification is a very good thing to have here. > But you would need to ask "git diff" to signal that it found no > differences with --exit-code or --quiet, I would think. > > Thanks. Whoops! Of course... You want me to re-roll? 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: Using clean/smudge filters with difftool
On Thu, Jun 18, 2015 at 05:39:18PM +0200, Florian Aspart wrote: > 2015-06-18 16:28 GMT+02:00 John Keeping : > > On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote: > >> 2015-06-18 16:11 GMT+02:00 John Keeping : > >> > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote: > >> >> 2015-06-18 15:26 GMT+02:00 John Keeping : > >> >> > [Please don't top-post on this list.] > >> >> > > >> >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: > >> >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber > >> >> >> : > >> >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: > >> >> >> >> I created a clean filter to apply on some files before commiting > >> >> >> >> them. > >> >> >> >> The filter works correctly when I commit the file and is also > >> >> >> >> applied > >> >> >> >> when I usethe iff command line tool. > >> >> >> >> However, when using difftool with meld, the filter is not applied > >> >> >> >> and > >> >> >> >> the different versions of the files are compared without any > >> >> >> >> filtering. > >> >> >> >> > >> >> >> >> Is there a way to apply the clean/smudge filters when comparing > >> >> >> >> the > >> >> >> >> working copy of a file to the HEAD version in a gui diff tool? > >> >> >> >> > >> >> >> >> I'm using git version 2.4.3 under Ubuntu. > >> > > >> > I also realised that the code for file diff is very different from > >> > directory diff do you see any difference between git-difftool acting on > >> > files and with the `--dir-diff` option? > >> > >> No, even with the --dir-diff option, the filter is still not applied. > > > > I have tried to reproduce this and it works as expected for me (i.e. the > > filter is applied) both for file diff and directory diff mode: > > > > $ git config filter.quote.clean "sed -e 's/^> //'" > > $ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'" > > $ git config filter.quote.required true > > > > $ echo '*.quote filter=quote' >>.gitattributes > > $ cat >1.quote < > one > > two > > three > > EOF > > $ git add .gitattributes 1.quote > > $ git commit -m 'Initial commit' > > $ echo four >>1.quote > > > > Now `git-difftool` shows the differences with the filter applied. This can > > be > > seen running with GIT_TRACE: > > > > $ GIT_TRACE=2 git difftool > > 15:26:59.211541 git.c:557 trace: exec: 'git-difftool' > > 15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool' > > 15:26:59.338617 git.c:348 trace: built-in: git 'config' > > '--bool' '--get' 'difftool.trustExitCode' > > 15:26:59.342664 git.c:348 trace: built-in: git 'diff' > > 15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e > > '\''s/^> //'\''' > > 15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e > > '\''s/^> //'\''' 'sed -e '\''s/^> //'\''' > > 15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> > > /n; s/^/> /'\''' > > 15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e > > '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\''' > > 15:26:59.355716 run-command.c:347 trace: run_command: > > 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' > > '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' > > '' '100644' > > 15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' > > '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' > > '100644' '1.quote' '' '100644' > > 15:26:59.370468 git.c:348 trace: built-in: git 'config' > > 'diff.tool' > > 15:26:59.373485 git.c:348 trace: built-in: git 'config' > > 'merge.tool' > > 15:26:59.378402 git.c:348 trace: built-in: git 'config' > > 'difftool.vimdiff.cmd' > > 15:26:59.381424 git.c:348 trace: built-in: git 'config' > > 'mergetool.vimdiff.cmd' > > 15:26:59.386623 git.c:348 trace: built-in: git 'config' > > '--bool' 'mergetool.prompt' > > 15:26:59.390198 git.c:348 trace: built-in: git 'config' > > '--bool' 'difftool.prompt' > > > > I think the first run_command of `sed` is cleaning the working tree file > > to figure out *if* it differs, then the second `sed` is smudging the > > version in the index so that difftool can use it. > > I'm not really understanding what your filter is doing, but I tried > your code on my machine and I get a different result when using diff > and difftool on my machine. It's supposed to be adding "> " to the beginning of lines in the working tree and stripping them in the repo, but I've just realised that the instructions above are wrong since "1.quote" is supposed to have leading "> "'s (although the test I ran before tidying it up for the email was correct). > The diff results give me: > > diff --git a/1.quote b/1.quote > index 4cb29ea..f384549 100644 > --- a/1.quote > +++ b/1.quote > @@ -1,
Re: [PATCH 1/2] t3404: demonstrate CHERRY_PICK_HEAD bug
Johannes Schindelin writes: >>> + git diff seq-onto && >> >> I am puzzled with this "diff"; what is this about? Is it a remnant >> from an earlier debugging session, or is it making sure seq-onto is >> a valid tree-ish? > > The idea is to verify that we end up with the same tree even if we > exchanged the latest two patches. I can remove it if you want as it is > not strictly necessary, but I would like to keep it just to make sure > that we did not end up with an incomplete rebase. I agree that such a verification is a very good thing to have here. But you would need to ask "git diff" to signal that it found no differences with --exit-code or --quiet, I would think. 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: Visualizing merge conflicts after the fact (using kdiff3)
Michael J Gruber writes: > This type of request comes up often (for a reason). I'm wondering > whether we could support it more systematically, either by exposing the > steps above as a command, or by storing the unresolved merge somewhere > (leveraging stash or rerere). Perhaps 'tr/remerge-diff' (on 'pu') is of interest? -- 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 completion not using ls-remote to auto-complete during push
Quoting Robert Dailey : On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor wrote: Quoting Robert Dailey I do the following: $ git push origin :topic If I stop halfway through typing 'topic' and hit TAB, auto-completion does not work if I do not have a local branch by that name (sometimes I delete my local branch first, then I push to delete it remotely). I thought that git completion code was supposed to use ls-remote to auto complete refs used in push operations. Is this supposed to work? It's intentional. Running 'git ls-remote' with a far away remote can take ages, so instead we grab the refs on the remote from the locally stored refs under 'refs/remotes//'. See e832f5c096 (completion: avoid ls-remote in certain scenarios, 2013-05-28). The commit message mentions that you can "force" completion of remote refs via 'git ls-remote' by starting with the full refname, i.e. 'refs/', however, that seems to work only on the left hand side of the colon in the push refspec. Gábor If that's indeed the case, then completion should work. I have a 'refs/remotes/origin/topic'. Why will auto complete not work even though this exists? Do multiple remotes cause issues (in theory there is no reason why it should cause problems, since it should know I'm auto-completing a ref on 'origin')? The number of remotes doesn't matter. What matters is which side of the colon the ref to be completed is. You can complete git push origin refs/ and git fetch origin refs/ will even list you refs freshly queried via 'git ls-remote'. However, git push origin :refs/ git push origin branch:refs/ don't work, because there are no refs starting with the prefix ':refs/' or 'branch:refs/'. Gábor -- 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] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Patrick Palka writes:
> Currently the diff-highlight script does not try to highlight hunks that
> have different numbers of removed/added lines. But we can be a little
> smarter than that, without introducing much magic and complexity.
>
> In the case of unevenly-sized hunks, we could still highlight the first
> few (lexicographical) add/remove pairs. It is not uncommon for hunks to
> have common "prefixes", and in such a case this change is very useful
> for spotting differences.
>
> Signed-off-by: Patrick Palka
> ---
Patrick, "git shortlog --no-merges contrib/diff-highlight/" is your
friend to see who may be able to give you a good feedback.
Jeff, what do you think?
I have this nagging feeling that it is just as likely that two
uneven hunks align at the top as they align at the bottom, so while
this might not hurt it may not be the right approach for a better
solution, in the sense that when somebody really wants to do a
better solution, this change and the original code may need to be
ripped out and redone from scratch.
> contrib/diff-highlight/diff-highlight | 26 +-
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/contrib/diff-highlight/diff-highlight
> b/contrib/diff-highlight/diff-highlight
> index ffefc31..0dfbebd 100755
> --- a/contrib/diff-highlight/diff-highlight
> +++ b/contrib/diff-highlight/diff-highlight
> @@ -88,22 +88,30 @@ sub show_hunk {
> return;
> }
>
> - # If we have mismatched numbers of lines on each side, we could try to
> - # be clever and match up similar lines. But for now we are simple and
> - # stupid, and only handle multi-line hunks that remove and add the same
> - # number of lines.
> - if (@$a != @$b) {
> - print @$a, @$b;
> - return;
> - }
> + # We match up the first MIN(a, b) lines on each side.
> + my $c = @$a < @$b ? @$a : @$b;
>
> + # Highlight each pair, and print each removed line of that pair.
> my @queue;
> - for (my $i = 0; $i < @$a; $i++) {
> + for (my $i = 0; $i < $c; $i++) {
> my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
> print $rm;
> push @queue, $add;
> }
> +
> + # Print the remaining unmatched removed lines of the hunk.
> + for (my $i = $c; $i < @$a; $i++) {
> + print $a->[$i];
> + }
> +
> + # Print the added lines of each highlighted pair.
> print @queue;
> +
> + # Print the remaining unmatched added lines of the hunk.
> + for (my $i = $c; $i < @$b; $i++) {
> + print $b->[$i];
> + }
> +
> }
>
> sub highlight_pair {
--
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: Using clean/smudge filters with difftool
2015-06-18 16:28 GMT+02:00 John Keeping : > On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote: >> 2015-06-18 16:11 GMT+02:00 John Keeping : >> > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote: >> >> 2015-06-18 15:26 GMT+02:00 John Keeping : >> >> > [Please don't top-post on this list.] >> >> > >> >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: >> >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber >> >> >> : >> >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: >> >> >> >> I created a clean filter to apply on some files before commiting >> >> >> >> them. >> >> >> >> The filter works correctly when I commit the file and is also >> >> >> >> applied >> >> >> >> when I usethe iff command line tool. >> >> >> >> However, when using difftool with meld, the filter is not applied >> >> >> >> and >> >> >> >> the different versions of the files are compared without any >> >> >> >> filtering. >> >> >> >> >> >> >> >> Is there a way to apply the clean/smudge filters when comparing the >> >> >> >> working copy of a file to the HEAD version in a gui diff tool? >> >> >> >> >> >> >> >> I'm using git version 2.4.3 under Ubuntu. >> > >> > I also realised that the code for file diff is very different from >> > directory diff do you see any difference between git-difftool acting on >> > files and with the `--dir-diff` option? >> >> No, even with the --dir-diff option, the filter is still not applied. > > I have tried to reproduce this and it works as expected for me (i.e. the > filter is applied) both for file diff and directory diff mode: > > $ git config filter.quote.clean "sed -e 's/^> //'" > $ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'" > $ git config filter.quote.required true > > $ echo '*.quote filter=quote' >>.gitattributes > $ cat >1.quote < one > two > three > EOF > $ git add .gitattributes 1.quote > $ git commit -m 'Initial commit' > $ echo four >>1.quote > > Now `git-difftool` shows the differences with the filter applied. This can be > seen running with GIT_TRACE: > > $ GIT_TRACE=2 git difftool > 15:26:59.211541 git.c:557 trace: exec: 'git-difftool' > 15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool' > 15:26:59.338617 git.c:348 trace: built-in: git 'config' > '--bool' '--get' 'difftool.trustExitCode' > 15:26:59.342664 git.c:348 trace: built-in: git 'diff' > 15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e '\''s/^> > //'\''' > 15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e > '\''s/^> //'\''' 'sed -e '\''s/^> //'\''' > 15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> > /n; s/^/> /'\''' > 15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e > '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\''' > 15:26:59.355716 run-command.c:347 trace: run_command: > 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' > '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' > '' '100644' > 15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' > '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' > '100644' '1.quote' '' '100644' > 15:26:59.370468 git.c:348 trace: built-in: git 'config' > 'diff.tool' > 15:26:59.373485 git.c:348 trace: built-in: git 'config' > 'merge.tool' > 15:26:59.378402 git.c:348 trace: built-in: git 'config' > 'difftool.vimdiff.cmd' > 15:26:59.381424 git.c:348 trace: built-in: git 'config' > 'mergetool.vimdiff.cmd' > 15:26:59.386623 git.c:348 trace: built-in: git 'config' > '--bool' 'mergetool.prompt' > 15:26:59.390198 git.c:348 trace: built-in: git 'config' > '--bool' 'difftool.prompt' > > I think the first run_command of `sed` is cleaning the working tree file > to figure out *if* it differs, then the second `sed` is smudging the > version in the index so that difftool can use it. I'm not really understanding what your filter is doing, but I tried your code on my machine and I get a different result when using diff and difftool on my machine. The diff results give me: diff --git a/1.quote b/1.quote index 4cb29ea..f384549 100644 --- a/1.quote +++ b/1.quote @@ -1,3 +1,4 @@ one two three +four While the diff tool tells me that the repository file is: > one > two > three and my working copy: one two three four In both case the the filters are called twice (cf GIT_TRACE) as in the example your wrote. -- 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 completion not using ls-remote to auto-complete during push
On Thu, Jun 18, 2015 at 6:29 AM, SZEDER Gábor wrote: > Quoting Robert Dailey >> I do the following: >> >> $ git push origin :topic >> >> If I stop halfway through typing 'topic' and hit TAB, auto-completion >> does not work if I do not have a local branch by that name (sometimes >> I delete my local branch first, then I push to delete it remotely). I >> thought that git completion code was supposed to use ls-remote to auto >> complete refs used in push operations. Is this supposed to work? > > It's intentional. Running 'git ls-remote' with a far away remote can > take ages, so instead we grab the refs on the remote from the locally > stored refs under 'refs/remotes//'. > > See e832f5c096 (completion: avoid ls-remote in certain scenarios, > 2013-05-28). The commit message mentions that you can "force" > completion of remote refs via 'git ls-remote' by starting with the full > refname, i.e. 'refs/', however, that seems to work only on the > left hand side of the colon in the push refspec. > > Gábor > If that's indeed the case, then completion should work. I have a 'refs/remotes/origin/topic'. Why will auto complete not work even though this exists? Do multiple remotes cause issues (in theory there is no reason why it should cause problems, since it should know I'm auto-completing a ref on 'origin')? -- 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 report: `git revert` on empty commit fails silently
Hello gitters Steps to reproduce: $ git commit --allow-empty -m ‘test’ $ git revert HEAD The latter fails silently, leaving HEAD pointing at the commit created by the first command. A subsequent `git commit --allow-empty` has the revert message as the default commit message when starting the editor. Hope this is the right place for bugs. Alistair -- 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] mergetools: add config option to disable auto-merge
On Thu, Jun 18, 2015 at 4:43 AM David Aguilar wrote: > > On Wed, Jun 17, 2015 at 10:27:58PM -0400, Mike Rappazzo wrote: > > > > I feel that the auto-merge takes away the context of the changes. > > > > I use araxis merge as my mergetool of choice. I almost always immediately > > undo the auto-merge. After taking a moment to look at each file, I will > > then (usually) use the keyboard shortcut for auto-merge. > > > > It sure would be nice to "set-and-forget" a config variable to remove the > > annoyance of having to undo the auto-merge. I think that this config > > option is reasonable. Perhaps my documentation leaves something to be > > desired. I can take another stab at it. > > If this is the case then I would recommend making it more > granular. Just because Araxis' automerge is undesirable does > not mean that some other tools' automerge is as well. > e.g. the config variable could be "mergetool..automerge" > rather than the top-level "mergetool.automerge" variable. I don't necessarily think that araxis' automerge is bad, but I like to look at the before and after to understand the context of a conflict. I can't imagine that this is a quirk of araxis, but is probably something that exists for any auto-merging tool. The feature doesn't seem to be that widely supported among the other tooling. I only found the three to use such a feature. Since the automerge option is not available on every merge tool, it seems reasonable to use "mergetool..automerge" instead of "merge.automerge". > > > But, as Junio suggested, having a command-line flag to skip the > behavior might be a better first step. Something like, > "git mergetool --no-automerge". > > Most of Git's behavior that can be modified via configuration > can also be modified on the command-line, so exposing this > feature as a flag would probably be a good idea. This makes sense, and if this change is to go forward, I will implement the command line option. > > Even without a config variable, it can still be fire-and-forget > convenient by using a git alias to supply the flag. > > In lieu of any of these features, another option is that you can > override the default command by setting "mergetool.araxis.cmd", > and "git mergetool" will use your definition rather than its > built-in command. We left that escape hatch in for just this > purpose. I guess that if this patch does not go forward, I will have to use this workaround. -- 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/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line
> Remi Lespinet writes:
>
> > I've some more tests, maybe I should put them all in this post ?
>
> Yes, please post as much as you have. Ideally, this should be
> automatically tested, but if you don't have time to write the automated
> tests, at least having a track of what you did on the list archives can
> help someone else to do it.
It may not be easily readable without colors, so there are the scripts
at the end. You can change the tested input by changing lines after
the "cat >.tmplist" line in testall.sh. (There are two scripts
testall.sh and testone.perl).
Here are the tests results:
Input:
Split:
M::A :
Same : Yes
--
Input: Jane
Split: Jane
M::A : Jane
Same : Yes
--
Input: j...@example.com
Split: j...@example.com
M::A : j...@example.com
Same : Yes
--
Input:
Split: j...@example.com
M::A : j...@example.com
Same : Yes
--
Input: Jane
Split: Jane
M::A : Jane
Same : Yes
--
Input: Jane Doe
Split: Jane Doe
M::A : Jane Doe
Same : Yes
--
Input: Jane\ Doe
Split: "Jane\ Doe"
M::A : "Jane \ Doe"
Same : No
--
Input: "Jane"
Split: "Jane"
M::A : "Jane"
Same : Yes
--
Input: "Doe, Jane"
Split: "Doe, Jane"
M::A : "Doe, Jane"
Same : Yes
--
Input: "Doe, Ja"ne
Split: "Doe, Ja ne"
M::A : "Doe, Ja" ne
Same : No
--
Input: "Doe, Katarina" Jane
Split: "Doe, Katarina Jane"
M::A : "Doe, Katarina" Jane
Same : No
--
Input: "Jane@:;\>.,()
Split: "Jane@:;\>.,()
M::A : "Jane@:;\>.,()
Same : Yes
--
Input: Jane@:;\.,()<>Doe
Split: Jane@:
: "\."
: Doe ()
M::A : Jane@:
: \.
: Doe ()
Same : No
--
Input: Jane!#$%&'*+-/=?^_{|}~Doe'
Split: Jane!#$%&'*+-/=?^_{|}~Doe'
M::A : Jane!#$%&'*+-/=?^_{|}~Doe'
Same : Yes
--
Input: ""
Split: ""
M::A : ""
Same : Yes
--
Input: "Jane j...@example.com"
Split: "Jane j...@example.com"
M::A : "Jane j...@example.com"
Same : Yes
--
Input: Jane Doe
Split: Jane Doe
M::A : Jane Doe
Same : Yes
--
Input: Jane Doe < j...@example.com >
Split: Jane Doe
M::A : Jane Doe
Same : Yes
--
Input: Jane @ Doe @ Jane @ Doe
Split: Jane@Doe@Jane@Doe
M::A : Jane@Doe@Jane@Doe
Same : Yes
--
Input: Jane j...@example.com
Split: janej...@example.com
M::A : Jane
: j...@example.com
Same : No
--
Input: Jane Doe
Split: jdoe@example.comJaneDoe
M::A : Jane Doe
Same : No
--
Input: Jane Doe
Split: Jane
M::A : Jane Doe
Same : No
--
Input: "Jane, 'Doe'"
Split: "Jane, 'Doe'"
M::A : "Jane, 'Doe'"
Same : Yes
--
Input: 'Doe, "Jane'
Split: 'Doe
: " Jane'
M::A : 'Doe
: " Jane'
Same : Yes
--
Input: "Jane" "Do"e
Split: "Jane" "Do" e
M::A : "Jane" "Do" e
Same : Yes
--
Input: "Jane' Doe"
Split: "Jane' Doe"
M::A : "Jane' Doe"
Same : Yes
--
Input: "Jane Doe "
Split: "Jane Doe "
M::A : "Jane Doe "
Same : Yes
--
Input: "Jane\" Doe"
Split: "Jane\" Doe"
M::A : "Jane\" Doe"
Same : Yes
--
Input: Doe, jane
Split: Doe
: jane
M::A : Doe
: jane
Same : Yes
--
Input: "Jane Doe
Split: " Jane Doe
M::A : " Jane Doe
Same : Yes
--
Input: "Jane "Kat"a" ri"na" ",Doe"
Split: "Jane Kat a ri na ,Doe"
M::A : "Jane " Kat "a" ri "na" ",Doe"
Same : No
--
Input: Jane Doe
Split: Jane Doe
M::A : Jane
: Doe
Same : No
--
Input: Jane "Doe "
Split: "Jane Doe "
M::A : Jane
: "Doe "
Same : No
--
Input: \"Jane Doe
Split: "\"Jane Doe"
M::A : \ " Jane Doe
Same : No
--
Input: Jane\"\" Doe
Split: "Jane\"\" Doe"
M::A : Jane \ " \ " Doe
Same : No
--
Input: 'Jane 'Doe'
Split: 'Jane 'Doe'
M::A : 'Jane 'Doe'
Same : Yes
--
Input: 'Jane "Katarina\" \' Doe'
Split: "'Jane Katarina\" \' Doe'"
M::A : 'Jane " Katarina \ " \ ' Doe'
Same : No
**
* SCRIPTS PART *
**
testall.sh
#!/bin/sh
cat >.tmplist <
Jane
Jane Doe
Jane\ Doe
"Jane"
"Doe, Jane"
"Doe, Ja"ne
"Doe, Katarina" Jane
"Jane@:;\>.,()
Jane@:;\.,()<>Doe
Jane!#$%&'*+-/=?^_{|}~Doe'
""
"Jane j...@example.com"
Jane Doe
Jane Doe < j...@example.com >
Jane @ Doe @ Jane @ Doe
Jane j...@example.com
Jane Doe
Jane Doe
"Jane, 'Doe'"
'Doe, "Jane'
"Jane" "Do"e
"Jane' Doe"
"Jane Doe "
"Jane\" Doe"
Doe, jane
"Jane Doe
"Jane "Kat"a" ri"na" ",Doe"
Jane Doe
Jane "Doe "
\"Jane Doe
Jane\"\" Doe
'Jane 'Doe'
'Jane "Katarina\" \' Doe'
EOF
cat .tmplist | while read -r line
do
echo "Input: $line"
./testone.perl "$line"
echo --
done
testone.perl
#!/usr/bin/perl
use strict;
use warnings;
use Term::ANSIColor;
use Mail::Address;
use Text::ParseWords;
my $string = $ARGV[0];
sub split_addr
Re: Using clean/smudge filters with difftool
On Thu, Jun 18, 2015 at 04:17:52PM +0200, Florian Aspart wrote: > 2015-06-18 16:11 GMT+02:00 John Keeping : > > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote: > >> 2015-06-18 15:26 GMT+02:00 John Keeping : > >> > [Please don't top-post on this list.] > >> > > >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: > >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber : > >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: > >> >> >> I created a clean filter to apply on some files before commiting > >> >> >> them. > >> >> >> The filter works correctly when I commit the file and is also applied > >> >> >> when I usethe iff command line tool. > >> >> >> However, when using difftool with meld, the filter is not applied and > >> >> >> the different versions of the files are compared without any > >> >> >> filtering. > >> >> >> > >> >> >> Is there a way to apply the clean/smudge filters when comparing the > >> >> >> working copy of a file to the HEAD version in a gui diff tool? > >> >> >> > >> >> >> I'm using git version 2.4.3 under Ubuntu. > > > > I also realised that the code for file diff is very different from > > directory diff do you see any difference between git-difftool acting on > > files and with the `--dir-diff` option? > > No, even with the --dir-diff option, the filter is still not applied. I have tried to reproduce this and it works as expected for me (i.e. the filter is applied) both for file diff and directory diff mode: $ git config filter.quote.clean "sed -e 's/^> //'" $ git config filter.quote.smudge "sed -e '/^> /n; s/^/> /'" $ git config filter.quote.required true $ echo '*.quote filter=quote' >>.gitattributes $ cat >1.quote <>1.quote Now `git-difftool` shows the differences with the filter applied. This can be seen running with GIT_TRACE: $ GIT_TRACE=2 git difftool 15:26:59.211541 git.c:557 trace: exec: 'git-difftool' 15:26:59.211674 run-command.c:347 trace: run_command: 'git-difftool' 15:26:59.338617 git.c:348 trace: built-in: git 'config' '--bool' '--get' 'difftool.trustExitCode' 15:26:59.342664 git.c:348 trace: built-in: git 'diff' 15:26:59.344857 run-command.c:347 trace: run_command: 'sed -e '\''s/^> //'\''' 15:26:59.345383 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''s/^> //'\''' 'sed -e '\''s/^> //'\''' 15:26:59.351077 run-command.c:347 trace: run_command: 'sed -e '\''/^> /n; s/^/> /'\''' 15:26:59.351605 run-command.c:195 trace: exec: '/bin/sh' '-c' 'sed -e '\''/^> /n; s/^/> /'\''' 'sed -e '\''/^> /n; s/^/> /'\''' 15:26:59.355716 run-command.c:347 trace: run_command: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '' '100644' 15:26:59.356191 run-command.c:195 trace: exec: 'git-difftool--helper' '1.quote' '/tmp/SUEySx_1.quote' '4cb29ea38f70d7c61b2a3a25b02e3bdf44905402' '100644' '1.quote' '' '100644' 15:26:59.370468 git.c:348 trace: built-in: git 'config' 'diff.tool' 15:26:59.373485 git.c:348 trace: built-in: git 'config' 'merge.tool' 15:26:59.378402 git.c:348 trace: built-in: git 'config' 'difftool.vimdiff.cmd' 15:26:59.381424 git.c:348 trace: built-in: git 'config' 'mergetool.vimdiff.cmd' 15:26:59.386623 git.c:348 trace: built-in: git 'config' '--bool' 'mergetool.prompt' 15:26:59.390198 git.c:348 trace: built-in: git 'config' '--bool' 'difftool.prompt' I think the first run_command of `sed` is cleaning the working tree file to figure out *if* it differs, then the second `sed` is smudging the version in the index so that difftool can use it. -- 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: Using clean/smudge filters with difftool
2015-06-18 16:11 GMT+02:00 John Keeping : > On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote: >> 2015-06-18 15:26 GMT+02:00 John Keeping : >> > [Please don't top-post on this list.] >> > >> > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: >> >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber : >> >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: >> >> >> Hi everyone, >> >> >> >> >> >> I created a clean filter to apply on some files before commiting them. >> >> >> The filter works correctly when I commit the file and is also applied >> >> >> when I usethe iff command line tool. >> >> >> However, when using difftool with meld, the filter is not applied and >> >> >> the different versions of the files are compared without any >> >> >> filtering. >> >> >> >> >> >> Is there a way to apply the clean/smudge filters when comparing the >> >> >> working copy of a file to the HEAD version in a gui diff tool? >> >> >> >> >> >> I'm using git version 2.4.3 under Ubuntu. >> >> >> >> >> >> Best, >> >> >> Florian >> >> > >> >> > Are you saying that "difftool" compares an uncleaned working tree file >> >> > with a cleaned blob? That would be a bug in either difftool or the way >> >> > we feed difftool. >> >> > >> >> yes in this case "difftool" compares an uncleaned working tree file >> >> with a cleaned blob. I did not try the smudge filter to see if it >> >> applied in difftool. >> >> >> >> I think the problem comes from the way difftool is feeded, since I >> >> also had this problem when setting an external tool for the diff in >> >> the gitconfig file. >> >> >> >> However, I'm not sure if this is a bug or it is designed to be so. >> >> If the external tool changes a cleaned working tree file during the >> >> diff, then by saving this file the result of the cleaning filter would >> >> also be saved in the working tree. >> > >> > How is your filter configured? Is it using a simple pattern (e.g. >> > "*.c") or is it using a file path? >> > >> > git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder >> > if the prefix means that the attribute specification does not match the >> > temporary file that difftool produces, so no filter is applied. >> >> It is using a simple pattern: >> *.ipynb filter=clean_ipynb > > I also realised that the code for file diff is very different from > directory diff do you see any difference between git-difftool acting on > files and with the `--dir-diff` option? No, even with the --dir-diff option, the filter is still not applied. -- 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: Using clean/smudge filters with difftool
On Thu, Jun 18, 2015 at 03:51:25PM +0200, Florian Aspart wrote: > 2015-06-18 15:26 GMT+02:00 John Keeping : > > [Please don't top-post on this list.] > > > > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: > >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber : > >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: > >> >> Hi everyone, > >> >> > >> >> I created a clean filter to apply on some files before commiting them. > >> >> The filter works correctly when I commit the file and is also applied > >> >> when I usethe iff command line tool. > >> >> However, when using difftool with meld, the filter is not applied and > >> >> the different versions of the files are compared without any > >> >> filtering. > >> >> > >> >> Is there a way to apply the clean/smudge filters when comparing the > >> >> working copy of a file to the HEAD version in a gui diff tool? > >> >> > >> >> I'm using git version 2.4.3 under Ubuntu. > >> >> > >> >> Best, > >> >> Florian > >> > > >> > Are you saying that "difftool" compares an uncleaned working tree file > >> > with a cleaned blob? That would be a bug in either difftool or the way > >> > we feed difftool. > >> > > >> yes in this case "difftool" compares an uncleaned working tree file > >> with a cleaned blob. I did not try the smudge filter to see if it > >> applied in difftool. > >> > >> I think the problem comes from the way difftool is feeded, since I > >> also had this problem when setting an external tool for the diff in > >> the gitconfig file. > >> > >> However, I'm not sure if this is a bug or it is designed to be so. > >> If the external tool changes a cleaned working tree file during the > >> diff, then by saving this file the result of the cleaning filter would > >> also be saved in the working tree. > > > > How is your filter configured? Is it using a simple pattern (e.g. > > "*.c") or is it using a file path? > > > > git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder > > if the prefix means that the attribute specification does not match the > > temporary file that difftool produces, so no filter is applied. > > It is using a simple pattern: > *.ipynb filter=clean_ipynb I also realised that the code for file diff is very different from directory diff do you see any difference between git-difftool acting on files and with the `--dir-diff` option? -- 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: Using clean/smudge filters with difftool
2015-06-18 15:26 GMT+02:00 John Keeping : > [Please don't top-post on this list.] > > On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: >> 2015-06-18 14:31 GMT+02:00 Michael J Gruber : >> > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: >> >> Hi everyone, >> >> >> >> I created a clean filter to apply on some files before commiting them. >> >> The filter works correctly when I commit the file and is also applied >> >> when I usethe iff command line tool. >> >> However, when using difftool with meld, the filter is not applied and >> >> the different versions of the files are compared without any >> >> filtering. >> >> >> >> Is there a way to apply the clean/smudge filters when comparing the >> >> working copy of a file to the HEAD version in a gui diff tool? >> >> >> >> I'm using git version 2.4.3 under Ubuntu. >> >> >> >> Best, >> >> Florian >> > >> > Are you saying that "difftool" compares an uncleaned working tree file >> > with a cleaned blob? That would be a bug in either difftool or the way >> > we feed difftool. >> > >> yes in this case "difftool" compares an uncleaned working tree file >> with a cleaned blob. I did not try the smudge filter to see if it >> applied in difftool. >> >> I think the problem comes from the way difftool is feeded, since I >> also had this problem when setting an external tool for the diff in >> the gitconfig file. >> >> However, I'm not sure if this is a bug or it is designed to be so. >> If the external tool changes a cleaned working tree file during the >> diff, then by saving this file the result of the cleaning filter would >> also be saved in the working tree. > > How is your filter configured? Is it using a simple pattern (e.g. > "*.c") or is it using a file path? > > git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder > if the prefix means that the attribute specification does not match the > temporary file that difftool produces, so no filter is applied. It is using a simple pattern: *.ipynb filter=clean_ipynb -- 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: Using clean/smudge filters with difftool
[Please don't top-post on this list.] On Thu, Jun 18, 2015 at 03:15:38PM +0200, Florian Aspart wrote: > 2015-06-18 14:31 GMT+02:00 Michael J Gruber : > > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: > >> Hi everyone, > >> > >> I created a clean filter to apply on some files before commiting them. > >> The filter works correctly when I commit the file and is also applied > >> when I usethe iff command line tool. > >> However, when using difftool with meld, the filter is not applied and > >> the different versions of the files are compared without any > >> filtering. > >> > >> Is there a way to apply the clean/smudge filters when comparing the > >> working copy of a file to the HEAD version in a gui diff tool? > >> > >> I'm using git version 2.4.3 under Ubuntu. > >> > >> Best, > >> Florian > > > > Are you saying that "difftool" compares an uncleaned working tree file > > with a cleaned blob? That would be a bug in either difftool or the way > > we feed difftool. > > > yes in this case "difftool" compares an uncleaned working tree file > with a cleaned blob. I did not try the smudge filter to see if it > applied in difftool. > > I think the problem comes from the way difftool is feeded, since I > also had this problem when setting an external tool for the diff in > the gitconfig file. > > However, I'm not sure if this is a bug or it is designed to be so. > If the external tool changes a cleaned working tree file during the > diff, then by saving this file the result of the cleaning filter would > also be saved in the working tree. How is your filter configured? Is it using a simple pattern (e.g. "*.c") or is it using a file path? git-difftool uses `git checkout-index --all --prefix=$dir/` and I wonder if the prefix means that the attribute specification does not match the temporary file that difftool produces, so no filter is applied. -- 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: Using clean/smudge filters with difftool
Hi Michael, yes in this case "difftool" compares an uncleaned working tree file with a cleaned blob. I did not try the smudge filter to see if it applied in difftool. I think the problem comes from the way difftool is feeded, since I also had this problem when setting an external tool for the diff in the gitconfig file. However, I'm not sure if this is a bug or it is designed to be so. If the external tool changes a cleaned working tree file during the diff, then by saving this file the result of the cleaning filter would also be saved in the working tree. 2015-06-18 14:31 GMT+02:00 Michael J Gruber : > Florian Aspart venit, vidit, dixit 16.06.2015 16:11: >> Hi everyone, >> >> I created a clean filter to apply on some files before commiting them. >> The filter works correctly when I commit the file and is also applied >> when I usethe iff command line tool. >> However, when using difftool with meld, the filter is not applied and >> the different versions of the files are compared without any >> filtering. >> >> Is there a way to apply the clean/smudge filters when comparing the >> working copy of a file to the HEAD version in a gui diff tool? >> >> I'm using git version 2.4.3 under Ubuntu. >> >> Best, >> Florian > > Are you saying that "difftool" compares an uncleaned working tree file > with a cleaned blob? That would be a bug in either difftool or the way > we feed difftool. > > Michael > -- 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: Visualizing merge conflicts after the fact (using kdiff3)
Hi Micha, On 2015-06-18 14:26, Michael J Gruber wrote: > Johannes Schindelin venit, vidit, dixit 16.06.2015 11:43: > >> To list all the merge commits in the current branch, I would use the >> command-line: >> >> ```bash >> git rev-list --author="My Colleague" --parents HEAD | >> sed -n 's/ .* .*//p' >> ``` >> >> (i.e. listing all the commits with their parents, then filtering just the >> ones having more than one parent, which would include octopus merges if your >> history has them.) > > :) > > "--merges" (aka "--min-parents=2") is your friend here. Learnt something! Thanks, 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: Using clean/smudge filters with difftool
Florian Aspart venit, vidit, dixit 16.06.2015 16:11: > Hi everyone, > > I created a clean filter to apply on some files before commiting them. > The filter works correctly when I commit the file and is also applied > when I usethe iff command line tool. > However, when using difftool with meld, the filter is not applied and > the different versions of the files are compared without any > filtering. > > Is there a way to apply the clean/smudge filters when comparing the > working copy of a file to the HEAD version in a gui diff tool? > > I'm using git version 2.4.3 under Ubuntu. > > Best, > Florian Are you saying that "difftool" compares an uncleaned working tree file with a cleaned blob? That would be a bug in either difftool or the way we feed difftool. Michael -- 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: Visualizing merge conflicts after the fact (using kdiff3)
Johannes Schindelin venit, vidit, dixit 16.06.2015 11:43: > Hi Eric, > > On 2015-06-16 03:17, Eric Raible wrote: >> I'm running 1.9.5.msysgit.1, but this is a general git question... >> >> Upon returning from a vacation, I was looking at what people had been >> up to, and discovered on merge in which a colleague had resolved a merge >> incorrectly. It turns out that he has pushed *many* merges over the past >> year which had conflicts in my code, and now I don't trust any of them. >> >> So naturally I want to check each of them for correctness. >> >> I know about "git log -p -cc SHA -- path", but it really doesn't >> show just the conflicts so there's just too much noise in that output. >> >> I use kdiff3 to resolve conflicts, so I'm looking for a way to >> visualize these already-resolved conflicts with that tool. >> As I said, there are many merges, so the prospect of checking >> out each sha, doing the merge, and then comparing the results >> is completely untenable. >> >> Can anyone help? Surely other people have wanted to review how >> conflicts were resolved w/out looking at the noise of unconflicted >> changes, right? > > If I was walking in your shoes, I would essentially recreate the merge > conflicts and then use "git diff " with the resolved merge in > your current history. > > Something like this: > > ```bash > mergecommit=$1 > > # probably should verify that the working directory is clean, yadda yadda > > # recreate merge conflicts on an unnamed branch (Git speak: detached HEAD) > git checkout $mergecommit^ > git merge $mergecommit^2 || > die "This merge did not have any problem!" > > # compare to the actual resolution as per the merge commit > git diff $mergecommit > ``` This type of request comes up often (for a reason). I'm wondering whether we could support it more systematically, either by exposing the steps above as a command, or by storing the unresolved merge somewhere (leveraging stash or rerere). > To list all the merge commits in the current branch, I would use the > command-line: > > ```bash > git rev-list --author="My Colleague" --parents HEAD | > sed -n 's/ .* .*//p' > ``` > > (i.e. listing all the commits with their parents, then filtering just the > ones having more than one parent, which would include octopus merges if your > history has them.) :) "--merges" (aka "--min-parents=2") is your friend here. > > Hopefully this gives you good ideas how to proceed. > > Ciao, > Johannes > Michael -- 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
