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

[uber] Some dependencies require their pom.properties file #278

Closed
martinklepsch opened this issue Aug 22, 2015 · 10 comments
Closed

[uber] Some dependencies require their pom.properties file #278

martinklepsch opened this issue Aug 22, 2015 · 10 comments
Milestone

Comments

@martinklepsch
Copy link
Member

Postal looks up it's version at runtime be looking at META-INF/maven/com.draines/postal/pom.properties which is excluded by the uberjar task by default:

#{#"(?i)^META-INF/[^/]*\.(MF|SF|RSA|DSA)$"
  #"^((?i)META-INF)/.*pom\.(properties|xml)$"
  #"(?i)^META-INF/INDEX.LIST$"}

Postal function for reference:

(defn pom-version []
  (let [pom "META-INF/maven/com.draines/postal/pom.properties"
        props (doto (Properties.)
                (.load (-> pom io/resource io/input-stream)))]
    (.getProperty props "version")))
@martinklepsch
Copy link
Member Author

In this regard the docstring for the uber task are also a bit confusing.

Conj MATCH onto the set of regexes that paths must not match.

makes one think that the things mentioned as default will be the "base set" for the mentioned conj operation.

@martinklepsch
Copy link
Member Author

I’m thinking we should include all pom.properties files but that would require a more strict rule to find the right .properties file in the fileset. Currently the following is used, causing my jar to build as tools.namespace 😄

(->> (core/output-files fileset)
                 (map core/tmp-file)
                 (core/by-name ["pom.properties"])
                 first)

(source)

@martinklepsch martinklepsch changed the title Uberjar's with postal break [uber] Some dependencies require their pom.properties file Aug 22, 2015
@martinklepsch
Copy link
Member Author

There's drewr/postal#66 tracking this in postal. I'm not sure if reaching for pom.properties is something jars are "allowed" to do. Someone who knows please step up :)

@micha
Copy link
Contributor

micha commented Sep 3, 2015

Regarding the uber docstring: how about

Conj MATCH onto the set of regexes that paths must not match, replacing the default set.

@micha
Copy link
Contributor

micha commented Sep 3, 2015

@martinklepsch The only thing it's getting from the pom.properties file is the info it uses to generate the name of the jar file. You can use the -f option to the jar task to name your uberjar something other than tools.namespace.jar :)

@martinklepsch
Copy link
Member Author

Right! How did I not check task options here...?
The docstring suggestion sounds good to me.
Guess we can close this then?

@micha
Copy link
Contributor

micha commented Sep 3, 2015

@martinklepsch i think it's still a bug...

I think the jar task is trying to be too smart when it sniffs for pom.properties files just to make a pretty jar name. I think the jar should just be called "project.jar" unless you give the -f option.

Similarly, I think the install and push tasks aren't being smart enough--they should accept an optional --pom option where you can specify the specific pom.xml file they should use to install/deploy when there are multiple poms in the jar.

So I think you're right, we should remove the pom.properties regex from the default excludes and modify some tasks. Does that seem right?

@martinklepsch
Copy link
Member Author

I think the jar task is trying to be too smart when it sniffs for pom.properties files just to make a pretty jar name. I think the jar should just be called "project.jar" unless you give the -f option.

👍

Similarly, I think the install and push tasks aren't being smart enough--they should accept an optional --pom option where you can specify the specific pom.xml file they should use to install/deploy when there are multiple poms in the jar.

Makes sense 👍

So I think you're right, we should remove the pom.properties regex from the default excludes and modify some tasks. Does that seem right?

If the jar task outputs project.jar by default seems fine. What other tasks would need to be modified?

@Deraen
Copy link
Contributor

Deraen commented Oct 5, 2015

This should work with the default options.

@alandipert alandipert added this to the 2.3.5 milestone Oct 5, 2015
micha added a commit that referenced this issue Nov 15, 2015
- Jar task produces jar with default name (project.jar) when multiple
  poms are found in the jar and the --file option isn't given.
- Install and push tasks throw exception when multiple poms are found in
  the jar and the --pom option isn't given.
@Deraen
Copy link
Contributor

Deraen commented Dec 15, 2015

This is fixed in master.

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

No branches or pull requests

4 participants