Bug 530117 - Don't version control jar-in-jar-loader.zip
Summary: Don't version control jar-in-jar-loader.zip
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.8.2   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-22 04:44 EST by Andrey Loskutov CLA
Modified: 2020-08-25 04:41 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Loskutov CLA 2018-01-22 04:44:57 EST
Follow up on bug 530108. It is highly surprising to see generated stuff added to the git. This leads to such cases where the code was changed but the fix is actually not working.

Ideally the zip should be generated on maven build out of sources. I'm not a maven expert, so I can't contribute here, so hope someone with more maven knowledge can look at this. May be just run ant from maven.
Comment 1 Lars Vogel CLA 2020-02-04 04:00:48 EST
+1, having this zip file reappear again and again in the Git staging view is annoying.
Comment 2 Lars Vogel CLA 2020-02-04 04:01:14 EST
Noopur, can this file be excluded from version control?
Comment 3 Eclipse Genie CLA 2020-02-04 09:23:41 EST
New Gerrit change created: https://git.eclipse.org/r/157145
Comment 4 Stephan Herrmann CLA 2020-02-04 09:32:15 EST
(In reply to Eclipse Genie from comment #3)
> New Gerrit change created: https://git.eclipse.org/r/157145

How's this:

15:27:47 [INFO] --- maven-antrun-plugin:1.8:run (jar-in-jar-loader) @ org.eclipse.jdt.ui ---
15:27:47 [INFO] Executing tasks
15:27:47 
15:27:47 main:
15:27:47 
15:27:47 compile-jar-in-jar-loader:
15:27:47     [mkdir] Created dir: /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/temp
15:27:47     [javac] /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/customBuildCallbacks.xml:71: warning: 'includeantruntime' was not set, defaulting to build.sysclasspath=last; set to false for repeatable builds
15:27:47     [javac] Compiling 5 source files to /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/temp

15:27:47     [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.6
15:27:47     [javac] Note: /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/jar in jar loader/org/eclipse/jdt/internal/jarinjarloader/JarRsrcLoader.java uses unchecked or unsafe operations.
15:27:47     [javac] Note: Recompile with -Xlint:unchecked for details.
15:27:47     [javac] 1 warning
15:27:47       [zip] Building zip: /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/jar-in-jar-loader.zip
15:27:47    [delete] Deleting directory /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/temp
15:27:47 [INFO] Executed tasks

It's using the existing customBuildCallbacks.xml.

The result also looks fine: https://ci.eclipse.org/jdt/job/eclipse.jdt.ui-Gerrit/ws/org.eclipse.jdt.ui/jar-in-jar-loader.zip
Comment 5 Stephan Herrmann CLA 2020-02-04 09:57:37 EST
Probably scripts/build_jar-in-jar-loader.xml is more appropriate these days. Patch set #2.

Interestingly, #1 worked with this warning:

15:27:47     [javac] /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/customBuildCallbacks.xml:71: warning: 'includeantruntime' was not set, defaulting to build.sysclasspath=last; set to false for repeatable builds

whereas #2 failed with this error:

15:46:41 [ERROR] /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/scripts/build_jar-in-jar-loader.xml:22: Unable to find a javac compiler;
15:46:41 [ERROR] com.sun.tools.javac.Main is not on the classpath.
15:46:41 [ERROR] Perhaps JAVA_HOME does not point to the JDK.
15:46:41 [ERROR] It is currently set to "/opt/tools/java/oracle/jdk-8/1.8.0_202/jre"
15:46:41 [ERROR] around Ant part ...<ant antfile="scripts/build_jar-in-jar-loader.xml"/>... @ 4:55 in /home/jenkins/agent/workspace/eclipse.jdt.ui-Gerrit/org.eclipse.jdt.ui/target/antrun/build-main.xml

assumably due to includeAntRuntime="no" in scripts/build_jar-in-jar-loader.xml

Retrying with includeAntRuntime="yes"
Comment 6 Stephan Herrmann CLA 2020-02-04 12:42:08 EST
(In reply to Stephan Herrmann from comment #5)
> Retrying with includeAntRuntime="yes"

not the solution. but putting ${java.home}/../lib/tools.jar on the systemPath via a maven plugin dependency does the trick.
Comment 8 Stephan Herrmann CLA 2020-02-04 14:16:28 EST
(In reply to Eclipse Genie from comment #7)
> Gerrit change https://git.eclipse.org/r/157145 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/
> ?id=1304727e7122fad4aa013a196ba021a1b5f8cd95

Released for 4.15 M3, let's hope it doesn't cause issues in production builds ...
Comment 9 Andrey Loskutov CLA 2020-02-04 14:22:39 EST
Cool to have it generated, but does maven checks the content of that jar for equality? 

Background : since usually jars contain timestamps of files in their zip entries, jars built from the same source would be always "different" (this is at least my personal ant experience). Different in maven build usually means, it may trigger a warning that build is incorrect because it produced "different" bits from same sources.

Thoughts?
Comment 10 Stephan Herrmann CLA 2020-02-04 14:44:03 EST
(In reply to Andrey Loskutov from comment #9)
> Cool to have it generated, but does maven checks the content of that jar for
> equality? 

compare against what? Since it's no longer in git, maven will not normally know about the previous jar.
 
> Background : since usually jars contain timestamps of files in their zip
> entries, jars built from the same source would be always "different" (this
> is at least my personal ant experience). Different in maven build usually
> means, it may trigger a warning that build is incorrect because it produced
> "different" bits from same sources.
> 
> Thoughts?

Maybe that's why it was committed to git in the first place. Feel free to revert.
Comment 11 Stephan Herrmann CLA 2020-02-04 14:52:51 EST
Another idea: revert the change and instead just remove the ExternalToolBuilder that triggers ant to re-created the zip. Everybody making changes to the sources of the JarInJarLoader would manually invoke ant and commit the zip. I see 4 relevant code changes in this area in its entire lifetime. So invoking ant four times in 10+ years doesn't sound like a problem to me :)
Comment 12 Andrey Loskutov CLA 2020-02-04 15:19:53 EST
(In reply to Stephan Herrmann from comment #10)
> (In reply to Andrey Loskutov from comment #9)
> > Cool to have it generated, but does maven checks the content of that jar for
> > equality? 
> 
> compare against what? Since it's no longer in git, maven will not normally
> know about the previous jar.

We have this "baseline" comparison, that compares build artifacts with the previous I build and I believe it is configured to replace built artifacts with the baseline version depending on some conditions (but I don't know exactly which). And I believe related to that was a check that there shouldn't be "different" content if the inputs hasn't changed. We will see it on second build, in which we have no changes in the bundle.
Comment 13 Noopur Gupta CLA 2020-02-05 00:22:46 EST
This caused I20200204-1800 failure:
(https://ci.eclipse.org/releng/view/Builds/job/I-build-4.15/92/console)

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:1.8:run (jar-in-jar-loader) on project org.eclipse.jdt.ui: Execution jar-in-jar-loader of goal org.apache.maven.plugins:maven-antrun-plugin:1.8:run failed: Plugin org.apache.maven.plugins:maven-antrun-plugin:1.8 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:1.6.0 at specified path /opt/public/common/java/openjdk/jdk-11/../lib/tools.jar -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :org.eclipse.jdt.ui

I will revert the patch and restart the build.
Comment 14 Eclipse Genie CLA 2020-02-05 00:24:05 EST
New Gerrit change created: https://git.eclipse.org/r/157177
Comment 16 Noopur Gupta CLA 2020-02-05 00:25:20 EST
(In reply to Eclipse Genie from comment #15)
> Gerrit change https://git.eclipse.org/r/157177 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=dddfacf6f018e856a8698b3e7a8f1bf3493acdd2
Reverted.
Comment 17 Andrey Loskutov CLA 2020-02-05 05:29:23 EST
(In reply to Stephan Herrmann from comment #11)
> Another idea: revert the change and instead just remove the
> ExternalToolBuilder that triggers ant to re-created the zip. Everybody
> making changes to the sources of the JarInJarLoader would manually invoke
> ant and commit the zip. I see 4 relevant code changes in this area in its
> entire lifetime. So invoking ant four times in 10+ years doesn't sound like
> a problem to me :)

Probably the best idea. See also bug 262746 whee it was added.

While playing with that, I've noticed that at least in my setup build_jar-in-jar-loader.xml build file generates different zip content because even if ant runs with Java 8, it uses javac from PATH that is in my case Java 11, and this one creates an additional JarRsrcLoader$1.class inside the zip file.

This doesn't look good. 

Shouldn't we use ecj instead of arbitrary javac in the user path?
Comment 18 Stephan Herrmann CLA 2020-02-05 17:35:45 EST
(In reply to Noopur Gupta from comment #16)
> (In reply to Eclipse Genie from comment #15)
> > Gerrit change https://git.eclipse.org/r/157177 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=dddfacf6f018e856a8698b3e7a8f1bf3493acdd2
> Reverted.

Thanks, Noopur, for cleaning up after me.

(In reply to Andrey Loskutov from comment #17)
> (In reply to Stephan Herrmann from comment #11)
> > Another idea: revert the change and instead just remove the
> > ExternalToolBuilder that triggers ant to re-created the zip. Everybody
> > making changes to the sources of the JarInJarLoader would manually invoke
> > ant and commit the zip. I see 4 relevant code changes in this area in its
> > entire lifetime. So invoking ant four times in 10+ years doesn't sound like
> > a problem to me :)
> 
> Probably the best idea. See also bug 262746 whee it was added.
> 
> While playing with that, I've noticed that at least in my setup
> build_jar-in-jar-loader.xml build file generates different zip content
> because even if ant runs with Java 8, it uses javac from PATH that is in my
> case Java 11, and this one creates an additional JarRsrcLoader$1.class
> inside the zip file.
> 
> This doesn't look good. 
> 
> Shouldn't we use ecj instead of arbitrary javac in the user path?

I think we can separate these issues, no?

Remove the ExternalToolBuilder from .project, but keep ".externalToolBuilders/Build Jar in Jar Loader.launch" for manual invocation.
=> Do this now, original problem solved, generation of the zip unchanged.

Check if JarRsrcLoader$1.class created by javac 11 is a problem
=> Follow-up bug.

WDYT?
Comment 19 Stephan Herrmann CLA 2020-02-22 05:15:45 EST
I don't have plans to again break builds this late in the cycle :)