[JENKINS-48831] Fix core-asset jar generation #4
Conversation
pom.xml
Outdated
| <parent> | ||
| <groupId>org.jenkins-ci.plugins</groupId> | ||
| <artifactId>plugin</artifactId> | ||
| <version>2.30</version> |
There was a problem hiding this comment.
Not strictly related, but 2.30 is now 7 months outdated: https://github.com/jenkinsci/plugin-pom/blob/master/CHANGELOG.md
There was a problem hiding this comment.
Would prefer to do upgrades in a separate PR. I just wanted to get it working with this.
js-module-base/pom.xml
Outdated
| @@ -80,12 +82,12 @@ | |||
| <version>1.8</version> | |||
| <executions> | |||
| <execution> | |||
| <phase>initialize</phase> | |||
| <phase>package</phase> | |||
There was a problem hiding this comment.
One of them ... there were multiple issues.
js-module-base/pom.xml
Outdated
| <configuration> | ||
| <target> | ||
| <!-- Use the pluginId as the asset namespace. --> | ||
| <copy todir="${project.build.directory}/assets-wrapper/assets/${project.artifactId}/jsmodules"> | ||
| <fileset dir="${project.basedir}/src/main/webapp/jsmodules"/> | ||
| <fileset dir="${basedir}/src/main/webapp/jsmodules"/> |
There was a problem hiding this comment.
Why? IIRC you are supposed to use project.basedir, not basedir.
There was a problem hiding this comment.
Same as above ... project.basedir didn't work (no files were copied to the expected location) while basedir did work.
There was a problem hiding this comment.
So this is the error ....
${project.basedir} expression not supported during profile activation, use ${basedir} instead
js-module-base/pom.xml
Outdated
| @@ -70,7 +72,7 @@ | |||
| <id>copy-jsmodules</id> | |||
| <activation> | |||
| <file> | |||
| <exists>${project.basedir}/src/main/webapp/jsmodules</exists> | |||
| <exists>${basedir}/src/main/webapp/jsmodules</exists> | |||
There was a problem hiding this comment.
Yes, that's what I originally had but it failed and maven (3.3.9 in my case) logged a warning instructing to change to use just "basedir".
I'll go back and reproduce the error and see if it's a version thing etc.
js-module-base/pom.xml
Outdated
| <groupId>org.jenkins-ci.ui</groupId> | ||
| <artifactId>jenkins-js-libs</artifactId> | ||
| <version>2.0.0-SNAPSHOT</version> | ||
| <relativePath>../pom.xml</relativePath> |
There was a problem hiding this comment.
So, could this POM material just be moved up into the root POM to simplify things?
There was a problem hiding this comment.
I was tempted to do that but decided to leave for another PR. Yes, I think it should work all in the top level pom.
|
Will address comments once I'm back - out sick atm with flu. |
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
This def did not work when the pom was in the subdir.
To test bundle generation happens properly and so fail the build if core asset generation goes wrong again.
tfennelly
force-pushed
the
JENKINS-48831
branch
from
57826df
to
eef9ced
Compare
| core using a URL of the form "<resURL>/assets/<namespace>/<resourcePath>". | ||
| --> | ||
| <profile> | ||
| <id>copy-core-assets</id> |
There was a problem hiding this comment.
This all smells like it would be easier wrapped in a Maven plugin... not to mention a faster build.
In fact this looks like a custom lifecycle would be even better
There was a problem hiding this comment.
Ah will ya stop mess'n. I'll not be doing that :)
There was a problem hiding this comment.
I do agree it is ugly, but not worth writing a plugin for it.
|
@jglick @daniel-beck I think this is ok now. |
[JENKINS-48831] Fix core-asset jar generation # Conflicts: # bootstrap/pom.xml # js-module-base/pom.xml
See JENKINS-48831.
Also fixing the version of all plugins to one version. Doing different versions in a multi-module setup was dumb. We might want to split out into individual repos at some point.
@jenkinsci/code-reviewers @reviewbybees