Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 367581 - do not generate new version qualifier unless there are actual code changes
Summary: do not generate new version qualifier unless there are actual code changes
Status: CLOSED FIXED
Alias: None
Product: CBI
Classification: Technology
Component: build help (show other bugs)
Version: 1.0   Edit
Hardware: All All
: P2 normal (vote)
Target Milestone: ---   Edit
Assignee: Igor Fedorenko CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 362883 370707
Blocks: 372792
  Show dependency tree
 
Reported: 2011-12-27 10:43 EST by Igor Fedorenko CLA
Modified: 2012-07-24 16:47 EDT (History)
12 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Fedorenko CLA 2011-12-27 10:43:40 EST
As discussed on cbi-dev [1], CBI build should not generate new version qualifier unless there are actual code changes.

There are two distinct cases.

When reproducing historical builds, eg, Indigo SR1, the build is expected to produce identical results (ideally, byte-to-byte identical) and should use the same version qualifier as the original build. This implies that version qualifier is embedded in either project sources, map files or some other versioned metadata.

When building a service release, do not generate new version qualifier for bundles and features that did not change compared to baseline build. This Tycho enhancement request is tracked as bug 362883.


[1] http://dev.eclipse.org/mhonarc/lists/cbi-dev/msg00029.html
Comment 1 Igor Fedorenko CLA 2012-02-06 07:04:12 EST
I've created separate bug 370707 to track work related to reproducible build version qualifiers
Comment 2 Andrea Ross CLA 2012-02-10 15:05:52 EST
increasing priority on this ticket
Comment 3 David Williams CLA 2012-02-22 13:59:40 EST
To add a tiny bit, currently sometimes the qualifier doesn't change even if "the code" does change ... the "old" bundle with same qualifier is used instead ... depending on what is mean by "the code". Currently this happens for example, if there are trivial changes to some file, such as "build date" or perhaps even a change to manifest that is deemed "not important enough" or another example if the "timestamp" of signed jars change (via TS Authority). 

I don't know how the project uses "priorities", but I'd count this as a "P1" ... for the simple reason that it is hard to compare builds without it, so other problems or differences might not be noticeable until this is fixed. 

Just my 2 cents.
Comment 4 Igor Fedorenko CLA 2012-03-16 20:21:43 EDT
From the another informative/interesting discussion on cbi-dev [2], I believe there are two possible approaches to implement this in cbi build

=== derive build qualifier from last project commit

Although this approach is likely workable, I believe it has a number of unwanted implications that need to be considered

* Tycho build configuration can be inherited from parent pom.xml (or parent of the parent and so on). Changes to the inherited configuration can result in significant change to project build output even when project sources did not change.
* .qualifier expansion in feature.xml may require new feature qualifier even when feature project itself did not change. It is likely possible to determine if feature qualifier change is needed at build time, so should not be a problem.
* Even without change to either the source or the inherited configuration, build results can still change is some rare/exotic cases when remote repository contents changes in certain ways (new version of a class has different value of a "public static final int" field, for example).


The feature qualifier expansion is particularly bad because it will affect pretty much every feature which includes changed bundles. I believe the only reliable way to solve this is to compare feature to the previous/baseline version, but maybe there are other ways.

The other two problems are probably less severe and can be worked-around by manually adding a commit to affected projects... but this is error prone and likely result is false-negatives, when old bundles are used even when there are meaningful changes.


=== compare bundle and feature contents to "baseline" version

The idea here is to compare build results of "this" build to baseline version. If there are no changes, use the old version. Of course, comparison must ignore differences in version qualifier, which maybe tricky in some cases. This approach, however, may only produce false-positives, i.e. new version is picked when there are no meaningful changes. Personally, I believe this is a better trade-off, but I am open to discussion.

[2] http://dev.eclipse.org/mhonarc/lists/cbi-dev/msg00092.html
Comment 5 Paul Webster CLA 2012-03-19 15:32:50 EDT
I'd like to make sure we separate what we need to do for plugins from what we need to do for features.

The plugins need to have their qualifier bumped when there is a good reason to re-build them.  95% of the time, it's because of a source change.  Currently in the SDK we've dealt with the other 5% by manually causing the qualifier to bump, for example a consumed constant is changed in a dependent project or a change in the SDK compile configuration.

Features currently in the SDK use a qualifier that's generated in 2 pieces, a date (based on when the feature changed) and a suffix that is generated from the versions/qualifiers of all of the contained plugins.  So change the feature source bumps the date qualifier and change any of the contained plugins bumps the suffix.  It generally works, although there are still open bugs (loss of accuracy can creep in, etc).

Even after we solve the plugin qualifier calculations we'll have more to do in the feature one.

PW
Comment 6 Igor Fedorenko CLA 2012-03-19 17:30:29 EDT
Does the current feature qualifier suffix generation guarantee that the new version is "newer" than the old one according to p2 version comparison rules?
Comment 7 Paul Webster CLA 2012-03-19 19:28:45 EDT
(In reply to comment #6)
> Does the current feature qualifier suffix generation guarantee that the new
> version is "newer" than the old one according to p2 version comparison rules?

Yes, although I'm not sure how.  The algorithm is fairly reliable, I don't recall a lot of problems reported on it (although it would be in PDE Build).

PW
Comment 8 David Williams CLA 2012-03-20 02:22:06 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > Does the current feature qualifier suffix generation guarantee that the new
> > version is "newer" than the old one according to p2 version comparison rules?
> 
> Yes, although I'm not sure how.  The algorithm is fairly reliable, I don't
> recall a lot of problems reported on it (although it would be in PDE Build).
> 

"fairly" is relative, I guess. :) bug 208143 is the one that talks about this issue. In short, it does not "guarantee" it, it could if the "hash codes" would be incredibility long ... but, there are limits, so sometimes "truncated" ... and in some of those cases is "equal to" previous ones, even though content has changed. 

We had LOTS of problems with this in WTP. increasing suffix size (I forget we went to 32 or 42 or something ... and that helped a lot. But, still, "the comparator" is what eventually "saved us" ... at least, allows us to "see" the error and correct it via a manual tag. 

So, the idea of fixing all those feature suffix problems by "detecting change" does sound worth considering. I can not think of any cases where a feature would change in some minor way that didn't matter. Even "formatting" and "whitespace" is usually significant. Less sure about line endings :) 

But, bundles are a bit different. There, I know for a fact we have lots of bundles with "about.ini" files that just get "current date" and in those cases we do NOT want the qualifier to change, if we've made no other changes. Those dates in about.ini are displayed in some dialog somewhere about "plugin info". 

I admit we could probably "live without it" and go in and change all our bundles to not have those files or dates, or set them in some other fashion ... but, that's just one small, obvious example. I know of 2 or 4 others that are more complicated and debatable.
Comment 9 David Williams CLA 2012-03-20 02:39:11 EDT
One more concern about "compare bundle to baseline version". As you know, that's hard to do ... each has differences, say in part of the signing signatures that are not significant, often even changes in exact "byte code layout" of java code that has no effect at all. 

So, true, the p2 comparator is great at "detecting differences" when the "qualifiers are the same" ... that's what it was designed for. I'm a little concerned about using that in "reverse", as it wasn't really designed for that. 

It might work ... but ... its a little different problem ... it would, for example, be used on each and every bundle every build to look for differences, whereas now, to wildly guess, I'd say it only has to look at 25% or 50% that "haven't changed" since the others already have different qualifiers, it does not need to worry about those ... so, hasn't really been designed or tested for the kind of use-case you are describing. 

So, I'm torn ... I like automation ... but also like the developer to be primarily in control and the tools just help.

Adding Olivier to CC list, as he created the java/jar comparator and maybe he'd have some opinion if it could be used as easily in "reverse" of what it now is, that is, if there is some fundamental difference in the direction of these use cases.
Comment 10 Paul Webster CLA 2012-03-20 08:29:01 EDT
I too don't want to see this approach become overly complicated.  While there are some risks (listed below) using the source is extremely deterministic, both in reproducibility and when asked the question "where is that coming from?".

(In reply to comment #4)
> === derive build qualifier from last project commit
> 
> Although this approach is likely workable, I believe it has a number of
> unwanted implications that need to be considered
> 
> * Tycho build configuration can be inherited from parent pom.xml (or parent of
> the parent and so on). Changes to the inherited configuration can result in
> significant change to project build output even when project sources did not
> change.

We live with this today.  A change in the core compiler (when we upgrade basebuilder) can require that bundles be re-tagged.

> * .qualifier expansion in feature.xml may require new feature qualifier even
> when feature project itself did not change. It is likely possible to determine
> if feature qualifier change is needed at build time, so should not be a
> problem.

This is functionality that we need.  See comment #5 thread.

> * Even without change to either the source or the inherited configuration,
> build results can still change is some rare/exotic cases when remote repository
> contents changes in certain ways (new version of a class has different value of
> a "public static final int" field, for example).

We live with this today, and have to re-tag.

The 1st and 3rd risks do happen (although very rarely).  But I believe they're risks we should accept to gain the benefit of a simple pattern: the qualifier can be derived from the source repo ... no other information or repos (git or p2) necessary.

PW
Comment 11 Paul Webster CLA 2012-03-20 08:33:26 EDT
(In reply to comment #8)
> But, bundles are a bit different. There, I know for a fact we have lots of
> bundles with "about.ini" files that just get "current date" and in those cases
> we do NOT want the qualifier to change, if we've made no other changes. Those
> dates in about.ini are displayed in some dialog somewhere about "plugin info". 
> 

In PDE build this is accomplished by modifying the source after it is checked out by PDE build.  Would we hook this into a pre-compile  step (although this changes the source in the working directory) or a replace-token-when-resource-copied step?

Although correctly generating the about.* information in branding plugins should be taken care of in a separate bug.

PW
Comment 12 Igor Fedorenko CLA 2012-03-20 09:54:14 EDT
PDE/Build does not support the notion of configuration inheritance. Parent pom.xml is the recommended way to control build options from one place. While project-level last commit was good-enough indication of "real" bundle change, I am concerned this will not be the case for Tycho and will result in higher rate of false negative, when manual intervention will be required to force the system to ship new version of bundles.

But, in any case, I think Eclipse Platform developers made their decision, so "derive build qualifier from last project commit" is the approach we'll be implementing in CBI build.
Comment 13 Igor Fedorenko CLA 2012-04-04 00:09:40 EDT
here is the outline of required changes

* Introduce extension point in Tycho BuildQualifierMojo that will allow custom logic to determine build timestamp.
* Implement jgit-based implementation of the extension point from the item above that will use project last commit's timestamp as the build timestamp.
* Embed project last commit's timestamp either in project jar file or p2 metadata. This will be used to calculate feature version qualifier (see below).
* For bundle projects, version qualifier will be rendered from the build timestamp using existing Tycho formatting configuration.
* For feature projects, build timestamp is selected is the latest timestamp of the feature project itself and any features/bundles included in the feature.
* Implement new Tycho goal that will compared bundle and feature jars to a baseline version and fail the build if new and baseline version have the equal version but different contents. When this happens, developers are expected to make artificial changes to changed projects to force new version qualifier.

Please note that since this approach fully derives version qualifiers from project sources, it also covers requirements of reproducible build version qualifiers from bug 370707
Comment 14 Paul Webster CLA 2012-04-04 11:45:44 EDT
Igor, would it still be possible to specify the qualifier in a pom.xml directly for one plugin (if one bundle wanted to opt-out of the auto-generation for some reason)?

PW
Comment 15 David Williams CLA 2012-04-04 12:07:55 EDT
(In reply to comment #13)
> here is the outline of required changes
> 
> ... and fail the build if new and baseline version have the equal
> version but different contents. When this happens, developers are expected to
> make artificial changes to changed projects to force new version qualifier.
> 

I'm not sure "failing the build" is the right thing to do. The current comparator often finds differences that are not really significant and can safely be ignored. In other cases it finds problems and it would be clearly a "known problem" that should fail a build (or, fail a unit test). And, there's a third category which are differences that are not known to be significant or not and would take some investigation. 

I know the Platform currently has unit tests to do that sort of testing (but am not familar with details). In WTP, I set it up to sort th wheat from the chafe and print different reports for the main cases, such as, just for examples, see reports for WTP M6:  

full log from comparator: 

http://download.eclipse.org/webtools/downloads/drops/R3.4.0/S-3.4.0M6-20120319200442/testResults/comparatorAllMessages.log.txt

messages which are known to be NOT significant: 

http://download.eclipse.org/webtools/downloads/drops/R3.4.0/S-3.4.0M6-20120319200442/testResults/comparatorExcludedMessages.log.txt

messages which were unexpected, and normally cause for alarm: (this list is normally empty, but in this case, a recent change in the compiler did a better job of removing "dead code" or similar, and we knew it'd be changing again soon so thought ok to not address for M6. 

http://download.eclipse.org/webtools/downloads/drops/R3.4.0/S-3.4.0M6-20120319200442/testResults/comparatorUnexpectedMessages.log.txt

I hope these concrete examples help explain what I mean. 

Thanks,
Comment 16 Igor Fedorenko CLA 2012-04-04 12:18:53 EDT
(In reply to comment #14)
> Igor, would it still be possible to specify the qualifier in a pom.xml directly
> for one plugin (if one bundle wanted to opt-out of the auto-generation for some
> reason)?
> 
> PW

Good question. Forcing version qualifier in pom.xml is not a problem but I am not sure about implications of doing this for features that include such bundles. I think it will still be okay if feature version qualifier is calculated based on timestamps of all included plugins/features regardless how their version qualifiers are calculated, but I need to think little more about it.
Comment 17 Paul Webster CLA 2012-04-12 09:35:40 EDT
I did a scan of the SDK qualifiers, and currently there are only 2 sets of oddities outside of the timestamp pattern:

org.junit_3.8.2.v3_8_2_v20100427-1100/
org.junit_4.8.2.v4_8_2_v20110321-1705/

org.eclipse.swt_3.8.0.v3827.jar
org.eclipse.swt.gtk.linux.x86_64_3.8.0.v3827.jar

I've brought that up with our Eclipse Project Planning Council

PW
Comment 18 Andrea Ross CLA 2012-07-09 21:29:31 EDT
setting to CBI 1.0
Comment 19 Andrea Ross CLA 2012-07-24 11:29:36 EDT
Igor, is this bug solved?
Comment 20 Andrea Ross CLA 2012-07-24 16:47:41 EDT
Discussed and agreed this bug could be closed-fixed.