Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1876] artifacts named '*.jar' should not be treated as modules. #803

Merged
merged 1 commit into from
Oct 27, 2014

Conversation

ryenus
Copy link
Contributor

@ryenus ryenus commented Sep 24, 2014

this patch ensure local '.jar' artifacts are copied to 'lib/'
instead of extracted to 'modules/'.

see http://play.lighthouseapp.com/projects/57987-play-framework/tickets/1876

this patch ensure local '.jar' artifacts are copied to 'lib/'
instead of extracted to 'modules/'.
@cloudbees-pull-request-builder

play-1-3-x-pull-requests #285 FAILURE
Looks like there's a problem with this pull request

@ryenus
Copy link
Contributor Author

ryenus commented Sep 24, 2014

could anyone please help resolve the pull request builder? it keeps failing with merge error, maybe the jenkins build workspace need clean up as a pre-build step?

@Notalifeform
Copy link
Contributor

@ryenus I'm not sure what's happening - I'll look into it

@ryenus
Copy link
Contributor Author

ryenus commented Sep 25, 2014

@Notalifeform,

The problem is like this:

  • I have play installed to /path/to/play
  • and I have a foo-module at /path/to/play/framework/modules/foo-module, example
  • and inside the module it has a local dependency included, like cli-parser-1.1.jar in the example
  • and ~/.ivy2/cache is cleared or renamed.
  • last, I run play deps /path/to/play/framework/modules/foo-module

The expected result is that the included jar file should be copied to the lib directory of foo-module.

While the actual result is that the jar file is extracted to the modules directory of foo-module.

Looking into the code it's because the condition below is matched because the jar in this case is inside /path/to/play/framework/modules, thus is treated as a play module:

String frameworkPath = new File(framework, "modules").getCanonicalPath();
isPlayModule = artifact.getArtifactOrigin().getLocation().startsWith(frameworkPath);

HTH, thank you

@xael-fry
Copy link
Member

@ryenus Why do you need to run deps in the module. We generally run deps in the application?
Do you have the same result when runnning deps in an application with dependency to your module?

@Notalifeform
Copy link
Contributor

@ryenus I was referring to the cloudbees failures - I didn't really look at your PR itself yet.

@ryenus
Copy link
Contributor Author

ryenus commented Sep 26, 2014

@xael-fry, run deps in the application only created links in the application modules directory.

In our case, one of our custom module have third party dependencies and somehow running deps in the application directory did nothing about the module dependencies unless I move the module dependencies into the application. That's why I need to run deps in the module directory.

I agree that in most cases a module being worked on should be put outside of the framework/modules directory, that's very true for published modules, but not so convenient for private modules in our case.

But I do expect jar files should be just copied to lib instead of extracted to modules directory in either cases.

@Notalifeform, thanks for the clarification, may be the jenkins workspace need a cleanup/hard-reset?

pepite added a commit that referenced this pull request Oct 27, 2014
[#1876] artifacts named '*.jar' should not be treated as modules.
@pepite pepite merged commit 3b7cfff into playframework:1.3.x Oct 27, 2014
@xael-fry
Copy link
Member

Merged in master and 1.3.x
Thanks

@angryziber
Copy link
Contributor

Unfortunately, our application doesn't work anymore with this change applied.
Some of our modules are now treated as jar files and not resolved correctly.

@pepite
Copy link
Contributor

pepite commented Oct 29, 2014

ok will revert the changes or fix it.

@angryziber
Copy link
Contributor

Example of our dependencies.yml that doesn't work:

require:
- play 1.3+
- play -> pdf 0.9
- play -> guice 1.2
- play -> secure
- play -> excel 1.2.3
- play-cms -> cms 1.3.4
- play-tests -> tests 3.9.4
- play-logger -> logger 1.2.2
- play-press -> press 2.2.5
- com.google.guava -> guava 18.0
repositories:
- codeborne:
type: http
artifact: http://repo.codeborne.com/[organization]/[module]-[revision].zip
contains:
- play-cms
- play-logger
- play-tests
- play-press

Modules that are resolved using this http resolver are no longer resolved...

@pepite
Copy link
Contributor

pepite commented Oct 29, 2014

Thanks you. Will look into it tonight

@angryziber
Copy link
Contributor

It seems that ivy downloads zip files from custom repository as jar files for some reason and the code and that's why DependenciesManager used to check for artifact.getArtifactOrigin().getLocation().endsWith(".zip"), which produces the correct result. The new code that just returns in case of local name *.jar doesn't check the artifact origin anymore

@angryziber
Copy link
Contributor

A friendly reminder :-)

@pepite
Copy link
Contributor

pepite commented Nov 3, 2014

Sorry did not have time to look at it. It's on my todo and I hope to have a bit of time tonight.

@xael-fry
Copy link
Member

xael-fry commented Nov 4, 2014

Revert for now, will work on the issue later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants