After a bit more investigation, I found
that we can change the git show command to include -U1 (which
means show only 1 line of surrounding context). This will limit
the amount of times 'context' matters.
So I'm sure the next question is, why can't we simply -U0 and
show no surrounding context, before passing into git patch-id?
Well, it seems when I do that, the results of git patch-id are
several lines... which breaks the current implementation.
However, it does come up with identical results for git patch-id
if done that way, at least for the first segments of every line.
[rob@rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git
show -U0 --minimal dec2f114af3acce10c5c9d3ee6043016e5322142 | git
patch-id
42be57b63f6d863843d63e954fcda5a8e589fb8c
dec2f114af3acce10c5c9d3ee6043016e5322142
30045b1ac7faf3b7a1cfbabc4b2d9d665545813b
0000000000000000000000000000000000000000
d6fabb7f604aa2ca3a066027d55e7c1251355a38
0000000000000000000000000000000000000000
94bfa60cb7c6e65908dd45c52e2c232be4449d30
0000000000000000000000000000000000000000
[rob@rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git
show -U0 --minimal 904fea9f59c54e612aac05ae347126c618fe695d | git
patch-id
42be57b63f6d863843d63e954fcda5a8e589fb8c
904fea9f59c54e612aac05ae347126c618fe695d
30045b1ac7faf3b7a1cfbabc4b2d9d665545813b
0000000000000000000000000000000000000000
d6fabb7f604aa2ca3a066027d55e7c1251355a38
0000000000000000000000000000000000000000
94bfa60cb7c6e65908dd45c52e2c232be4449d30
0000000000000000000000000000000000000000
For now, though the current implementation will be broken by using
-U0 because we do not expect multiple line output. I'll test a bit
more and see if I can improve this for next time.
On 04/20/2016 11:35 AM, Martin Malina wrote:
Great idea, Rob! I would also appreciated if there was a JIRA for
each component as Nick suggested.
There are 30 reports for integration tests. Looking
at a few, it was either just a change in a pom or a false
positive.
So I wonder why that is.
Take these two as an example:
Their patch-id is different:
$ git show dec2f114|git patch-id
9b70b5088668467b18419301adacb703d26411cc
dec2f114af3acce10c5c9d3ee6043016e5322142
$ git show 904fea9f5|git patch-id
1748ab1b8d0835a3a6b3d45fc0c57bb4fa24e2a3
904fea9f59c54e612aac05ae347126c618fe695d
Now let's see what seems to be different:
$ diff <(git show 904fea9f5) <(git show
dec2f114)
1c1
< commit 904fea9f59c54e612aac05ae347126c618fe695d
---
> commit dec2f114af3acce10c5c9d3ee6043016e5322142
8c8
< index 0942ef1..ff618cd 100644
---
> index face7e9..40b4e4e 100644
17c17
< Bundle-Version: 4.4.0.qualifier
---
> Bundle-Version: 4.3.0.qualifier
143c143
< index ff53ec5..54a64ee 100644
---
> index fbccb09..aae2c25 100644
You see, there is nothing different - apart from
checksums, it's the bundle version. But that is outside of the
changes - that is just there for context, not part of the
commits. Any idea why the patch-id is different?
-Martin
On 04/18/2016 05:05 AM, Max
Rydahl Andersen wrote:
great initiative - may I suggest you do a PR for your
script and put it at https://github.com/jbosstools/jbosstools-build-ci/tree/jbosstools-4.3.x/util
where we have other such utilities.
Also, might be easier if you put the run info into
jira since the output is quite hard to read/use here
from mail.
btw. it looks like this tool actually is very good at
finding the false positives - i.e. change in pom.xml
where it just changes version seem to be something you
could filter out somehow ?
First, I disagree that these are false-positives ;)
These are commits that are in maintenance that aren't in
master.
And, in all honesty, I think it'd be a mistake to simply
filter them out. I think it's much better to list the
false positive and let the component owner use his
judgment whether the patch needs further inspection or
not. Such simple version-changes will be very very easy
for a human to spot as irrelevant to master. This may
cause the repository owner to waste 2-3 seconds for each
commit, but it guarantees that every possible unmatched
commit is found.
I think this is much better and safer than possibly
adding some logic to filter out version changes and
later discover that the logic was wrong and so it was
hiding legitimately missing commits from being shown.
Also, since we're not inspecting the patches at all, but
rather comparing patch-id's, it would make the script
much much more complicated.
Il meglio è nemico del bene - Perfect is
the enemy of good
https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good
_______________________________________________
jbosstools-dev mailing list
jbosstools-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jbosstools-dev