ceylon / ceylon-module-resolver

DEPRECATED
Apache License 2.0
23 stars 9 forks source link

Require that maven module names contain `:` character #59

Closed FroMage closed 8 years ago

FroMage commented 11 years ago

The following is the old description, a new better plan has been laid out in the comments of requiring : in the module name.

This would allow us to:

This is a very cross-project issue, but since it is mostly about maven and resolving, I'm filing it here.

I suppose the maven annotation would be declared in the ceylon.interop.java module, which you'd need to import to use it.

WDYT?

davidfestal commented 11 years ago

+1 Andwhat about adding an optional parameter to the annotation with an alternate repository if not in maven central ?

quintesse commented 11 years ago

if it turns out this is really required

I really don't think there's any other way, but we can also add an extra point:

what about adding an optional parameter

I don't think that's a good idea because it would make our dependencies very fragile. Host name changes, Maven artifacts change repositories, etc

But I don't doubt that at some point we'll get a way to configure a list of possible Maven respositories to try. Maybe in a build system or whatever, but I don't think it's a good idea to put it in the module file itself. (I do understand that for now this means that if a Maven artifact is not in the central repository you will have to add it manually)

alesj commented 11 years ago

What about rather adding e.g. type attribute to @Import?

@Import(type = ImportType.MAVEN)

quintesse commented 11 years ago

@alesj I'm not sure what you have in mind exactly? The import (without @) is a language keyword and cannot have any attributes. That's why it was suggested to add it as an annotation: maven import "foo.bar" 1.2.3

alesj commented 11 years ago

Ah, ok, I see. (I need to update my Ceylon knowledge ...)

FroMage commented 11 years ago

All Maven issues move to 1.0

FroMage commented 11 years ago

I think a better fix would be to require that Maven modules use : in the name, since it fixes lots of issues we have WRT duplicate maven modules that are foo.bar and foo:bar, which means we get both modules loaded at the same time ATM.

This would fix all these issues, unfortunately I feel we can't do this in time for 1.0 and we have to accept that 1.0 will have crappy Maven module support.

sgalles commented 10 years ago

I guess this is related to this issue. This does not work : import "org.eclipse.jetty.orbit.javax.servlet" "3.0.0.v201112011016"; (the name of the module is javax.servlet)

rimmington commented 10 years ago

Same issue with org.hibernate.hibernate-jpa-2.1-api, where the artefact is hibernate-jpa-2.1-api. Weird versioning aside, I suspect this is an issue for a number of other Maven artefacts.

Is there any kind of temporary workaround for this issue?

quintesse commented 10 years ago

Sorry for not responding to this sooner @sgalles and @rimmington , but this is already supported for quite a while! You just put a : between the group id and artifact id, like this:

import "org.eclipse.jetty.orbit:javax.servlet" "3.0.0.v201112011016";

This issue is just about making that colon required instead of optional like it is now. (Btw, when using that colon putting the module name in quotes is required!)

sgalles commented 10 years ago

OK ! Good to know ! Thanks @quintesse

quintesse commented 10 years ago

Btw, I don't really like the idea to make the colon required and using that to detect if we're dealing with maven, I'd rather prefer to make it explicit with some kind of annotation (who knows what other repository types could make use of the colon separator).

FroMage commented 10 years ago

Moving again.

FroMage commented 8 years ago

This has been fixed a while ago.