Maven 1.x Eclipse Plugin

handling source attachments (patch)

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9
  • Labels:
    None
  • Number of attachments :
    5

Description

Actually maven repository doesn't contain source artifacts, however it would be nice if eclipse users could have a way to handle source for jars and have them added to eclipse .classpath (needed for debug and javadocs).

The attached path let users store sources in the local maven repository in the same way other artifacts are managed. Through the modification of the jar path the plugin will look for the sources file and, if existing, it will add them to the .classpath file.

The position of src files is controlled by 2 plugin properties: maven.eclipse.src.dir (directory for source artifact type) and maven.eclipse.src.extension (extension for files). These are temporarely managed as eclipse plugin properties till there is a standard maven default for them.

As an example, using the default values for these properties:
MAVEN_REPO/eclipse/jars/eclipse-ui-3.0.0.jar
will be mapped to
MAVEN_REPO/eclipse/src/eclipse-ui-3.0.0.zip

If the source zip is not available, it will not be added to the classpath file, so it will not cause any problem to users who don't have sources in their local repository.

  1. File
    MPECLIPSE-48_updated.diff
    14/Oct/04 5:13 PM
    5 kB
    fabrizio giustina
  2. File
    MPECLIPSE-48.3_with_plugin_tests.diff
    15/Oct/04 4:26 AM
    12 kB
    fabrizio giustina
  3. File
    MPECLIPSE-48.4_added_tag.diff
    15/Oct/04 11:01 AM
    11 kB
    fabrizio giustina
  4. File
    MPECLIPSE-48.5_tag_after_emmanuel_commit.diff
    15/Oct/04 11:19 AM
    7 kB
    fabrizio giustina
  5. File
    MPECLIPSE-48.diff
    12/Oct/04 4:37 PM
    4 kB
    fabrizio giustina

Issue Links

depends upon

Improvement - An improvement or enhancement to an existing feature or task. MPECLIPSE-26 Add a property to dependencies to link to source

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
fabrizio giustina added a comment -

cvs diff (also includes an update to documentation)

Show
fabrizio giustina added a comment - cvs diff (also includes an update to documentation)
Hide
Permalink
fabrizio giustina added a comment -

this is an updated version of the patch.
Updated after the MPECLIPSE-47 commit, fixes an error in the previous patch, and contains the fix for MPECLIPSE-49 too

Show
fabrizio giustina added a comment - this is an updated version of the patch. Updated after the MPECLIPSE-47 commit, fixes an error in the previous patch, and contains the fix for MPECLIPSE-49 too
Hide
Permalink
fabrizio giustina added a comment -

updated again, also adding plugin tests for MPECLIPSE-47 and MPECLIPSE-49.
Note that actually paths for junit/cactus in test maven.xml have windows specific file separator "\" so probably tests will fail under Linux (this was already present before this patch).

Show
fabrizio giustina added a comment - updated again, also adding plugin tests for MPECLIPSE-47 and MPECLIPSE-49 . Note that actually paths for junit/cactus in test maven.xml have windows specific file separator "\" so probably tests will fail under Linux (this was already present before this patch).
Hide
Permalink
David Eric Pugh added a comment -

fabrizio,

Since you are on a role, do you feel like seeing if some of this code:

+ <!-- check cactus sources -->
+ <j:set var="mappedsrc" value="cactus/$

{maven.eclipse.src.dir}

/cactus-$

{depVersion}.${maven.eclipse.src.extension}"/>
+ <u:file var="checkSrcExist" name="${maven.repo.local}/${mappedsrc}" />
+ <j:choose>
+ <j:when test="${checkSrcExist.exists()}">
+ <classpathentry kind="var" path="MAVEN_REPO/cactus/jars/cactus-${depVersion}

.jar" sourcepath="MAVEN_REPO/$

{mappedsrc}

"/>
+ </j:when>
+ <j:otherwise>
+ <classpathentry kind="var" path="MAVEN_REPO/cactus/jars/cactus-$

{depVersion}

.jar"/>
+ </j:otherwise>
+ </j:choose>

Can be converted into a tag? Something like <eclipse:checksource>? I can help you with this, but a good example is the artifact plugin. It seems like we are duplicating the same jelly over and over, and while it's good stuff, it makes it harder to read. I'll go ahead though and commit what you've already supplied.

Show
David Eric Pugh added a comment - fabrizio, Since you are on a role, do you feel like seeing if some of this code: + <!-- check cactus sources --> + <j:set var="mappedsrc" value="cactus/$ {maven.eclipse.src.dir} /cactus-$ {depVersion}.${maven.eclipse.src.extension}"/> + <u:file var="checkSrcExist" name="${maven.repo.local}/${mappedsrc}" /> + <j:choose> + <j:when test="${checkSrcExist.exists()}"> + <classpathentry kind="var" path="MAVEN_REPO/cactus/jars/cactus-${depVersion} .jar" sourcepath="MAVEN_REPO/$ {mappedsrc} "/> + </j:when> + <j:otherwise> + <classpathentry kind="var" path="MAVEN_REPO/cactus/jars/cactus-$ {depVersion} .jar"/> + </j:otherwise> + </j:choose> Can be converted into a tag? Something like <eclipse:checksource>? I can help you with this, but a good example is the artifact plugin. It seems like we are duplicating the same jelly over and over, and while it's good stuff, it makes it harder to read. I'll go ahead though and commit what you've already supplied.
Hide
Permalink
David Eric Pugh added a comment -

I ditched the property specifing the directory name "/src".. I don't think its needed, and I think our solution is only temporary. I reorganized the docs and added a section on the homepage about the linking of source directoires. Basically stating that "this implementation is for now until Maven properly deals with source artifacts".

Please review, as I had some changes conflicting with your changes, so I had to apply them.

Congrats on the unit tests! They caught at least one merge problem that I made. I appreciate these very clean patches.

Show
David Eric Pugh added a comment - I ditched the property specifing the directory name "/src".. I don't think its needed, and I think our solution is only temporary. I reorganized the docs and added a section on the homepage about the linking of source directoires. Basically stating that "this implementation is for now until Maven properly deals with source artifacts". Please review, as I had some changes conflicting with your changes, so I had to apply them. Congrats on the unit tests! They caught at least one merge problem that I made. I appreciate these very clean patches.
Hide
Permalink
fabrizio giustina added a comment -

I should have done this from the beginning:
added an <eclipse:write-classpath-entry> tag which is used to resolve sources and to actually write down the classpath element to the file.

Also fixes the use of file.separator (in windows sometime \ and sometime /). I still couldn't find a way to use the $

{file.separator}

variable in x:set (in plugin tests), so tests will continue to fail on linux...

Please check the removal of dependency handle too: AFAIK this was already not needed since the pluginVar tag was already used. I replaced pluginVar with get and removed the dependency handle (not sure about this, the plugin should still work in maven < 1.0 ?)

Show
fabrizio giustina added a comment - I should have done this from the beginning: added an <eclipse:write-classpath-entry> tag which is used to resolve sources and to actually write down the classpath element to the file. Also fixes the use of file.separator (in windows sometime \ and sometime /). I still couldn't find a way to use the $ {file.separator} variable in x:set (in plugin tests), so tests will continue to fail on linux... Please check the removal of dependency handle too: AFAIK this was already not needed since the pluginVar tag was already used. I replaced pluginVar with get and removed the dependency handle (not sure about this, the plugin should still work in maven < 1.0 ?)
Hide
Permalink
fabrizio giustina added a comment -

ouch, emmanuel has been faster and clever than me in fixing the file separator issue...
so here it is the diff with the <eclipse:write-classpath-entry> tag merged with emmanuel fix

Show
fabrizio giustina added a comment - ouch, emmanuel has been faster and clever than me in fixing the file separator issue... so here it is the diff with the <eclipse:write-classpath-entry> tag merged with emmanuel fix
Hide
Permalink
David Eric Pugh added a comment -

Did you pick up the changes that Emmanuel Venisse added?

Show
David Eric Pugh added a comment - Did you pick up the changes that Emmanuel Venisse added?
Hide
Permalink
David Eric Pugh added a comment -

I have applied and committed the patch. However, i think this introduced a bug.. the src/java directory is being added multiple times now...

Show
David Eric Pugh added a comment - I have applied and committed the patch. However, i think this introduced a bug.. the src/java directory is being added multiple times now...
Hide
Permalink
David Eric Pugh added a comment -

Discussion on mailing list resolved my last concern.

Show
David Eric Pugh added a comment - Discussion on mailing list resolved my last concern.
Hide
Permalink
Andreas Schönknecht added a comment -

Maven does contain some support for src.zip's already. When running the "dist" goals, a src.zip is built, named <artifactId>-<version>-src.zip.
This files is stored in the remote repository in a directory called "distributions". It would be nice if the Eclipse-Plugin would look there instead of the local repository, so I dont have to manually copy and rename them. Perhaps the "generate-classpath"-Goal could copy all needed src.zips from the remote repository distributions-directory to the local repository.
Before version 1.9 of Eclipse-Plug I worked as followed:
1. Edit the project.xml
2. Run eclipse:generate-classpath
3. Attach Sources manually in Eclipse via a Classpath variable that points to our remote-repository and expanding it to the src.zip in the distributions folder.

Show
Andreas Schönknecht added a comment - Maven does contain some support for src.zip's already. When running the "dist" goals, a src.zip is built, named <artifactId>-<version>-src.zip. This files is stored in the remote repository in a directory called "distributions". It would be nice if the Eclipse-Plugin would look there instead of the local repository, so I dont have to manually copy and rename them. Perhaps the "generate-classpath"-Goal could copy all needed src.zips from the remote repository distributions-directory to the local repository. Before version 1.9 of Eclipse-Plug I worked as followed: 1. Edit the project.xml 2. Run eclipse:generate-classpath 3. Attach Sources manually in Eclipse via a Classpath variable that points to our remote-repository and expanding it to the src.zip in the distributions folder.
Hide
Permalink
David Eric Pugh added a comment -

That is actually a good point about the src created by the dist goal. A patch would be appreciated. I was mostly interested in getting a barebones implementation in.

Show
David Eric Pugh added a comment - That is actually a good point about the src created by the dist goal. A patch would be appreciated. I was mostly interested in getting a barebones implementation in.

People

  • Assignee:
    David Eric Pugh
    Reporter:
    fabrizio giustina
Vote (1)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: