boot-clj / boot

Build tooling for Clojure.
https://boot-clj.github.io/
Eclipse Public License 1.0
1.75k stars 180 forks source link

push tasks :file and :pom options don't search in fileset #633

Open martinklepsch opened 7 years ago

martinklepsch commented 7 years ago

The code to find the jar and pom file given to a push tasks uses io/file which essentially prevents you from specifying a file in the fileset. This in turn requires you to output things into target to make things work.

Proposal: We search for both files in the following order:

Also exceptions around some of this stuff could be improved. If there's a realistic chance to get this into 2.7.2 I'd be very happy to start working on this ASAP.

Pinging @Deraen @arichiardi @SevereOverfl0w for opinions :)

martinklepsch commented 7 years ago

Reproduction snippet:

boot -d hiccup pom -p testing.org/xxx -v 0.0.1 uber jar push --repo "clojars" --file "project.jar"
arichiardi commented 7 years ago

Totally agree we should look into the fileset first.

martinklepsch commented 7 years ago

If you fix the above snippet by using the target task you still run into the issue that there most likely will be multiple pom.xml

boot -d hiccup -C pom -p testing.org/xxx -v 0.0.1 uber jar target push --repo "clojars" --file "target/project.jar"
Writing pom.xml and pom.properties...
Adding uberjar entries...
Writing project.jar...
Writing target dir(s)...
clojure.lang.ExceptionInfo: Multiple jar entries match: .*/pom.xml
    data: {:file
           "/var/folders/tt/hdgn6rc92pv68rscfj8jn8nh0000gn/T/boot.user8395542443886862570.clj",
           :line 13}
       java.lang.Exception: Multiple jar entries match: .*/pom.xml
           boot.pod/find-in-jarfile       pod.clj:  121
                   boot.pod/pom-xml       pod.clj:  205
               boot.pod/pom-xml-map       pod.clj:  417
     boot.task.built-in/fn/fn/fn/fn  built_in.clj:  871
     boot.task.built-in/fn/fn/fn/fn  built_in.clj:  303
     boot.task.built-in/fn/fn/fn/fn  built_in.clj:  717
     boot.task.built-in/fn/fn/fn/fn  built_in.clj:  594
     boot.task.built-in/fn/fn/fn/fn  built_in.clj:  433
                boot.core/run-tasks      core.clj:  938
                  boot.core/boot/fn      core.clj:  948
clojure.core/binding-conveyor-fn/fn      core.clj: 1938
                                ...
martinklepsch commented 7 years ago

This in turn means that you will need to supply the path to the pom.xml manually. If you make a typo this is the error:

 clojure.lang.ExceptionInfo: entry
    data: {:file
           "/var/folders/tt/hdgn6rc92pv68rscfj8jn8nh0000gn/T/boot.user6605178178184909072.clj",
           :line 13}
java.lang.NullPointerException: entry
                                ...
                   boot.pod/pom-xml       pod.clj:  206
               boot.pod/pom-xml-map       pod.clj:  417
martinklepsch commented 7 years ago

So currently what you need to do is this:

boot -d hiccup -C \ 
pom -p foo/bar -v 0.0.1 \
uber \
jar \
target \
push --repo "clojars" --file "target/project.jar" --pom "target/META-INF/maven/foo/bar/pom.xml" 
arichiardi commented 7 years ago

So as we just discovered, the :project option to jar makes pom understand where the pom is with no external help.

It looks a bit magic and I am now wondering if pom reads jar options to know that or something else is going on. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

arichiardi commented 7 years ago

The :project value is employed by jar in order to sift poms https://github.com/boot-clj/boot/blob/master/boot/core/src/boot/task/built_in.clj#L842 so it is important there

martinklepsch commented 7 years ago

The push task is able to properly find the jar so supplying :file isn't needed as often.

If you build uberjars you will need to supply the pom.xml path however. You can do that by passing the pom option but maybe it's better to just take the :project option as it's done for the jar task and find the pom.xml according to the project name.

martinklepsch commented 7 years ago

Thinking more about this the core really is that there are multiple pom files in the fileset once you use uber maybe instead of introducing yet another option we could do something to prevent that issue before it comes up.

martinklepsch commented 7 years ago

So a solution that does not require any code changes is passing a proper :exclude set to the uber task this:

(uber :exclude #{ #"(?i)^META-INF/INDEX.LIST$"
                  #"(?i)^META-INF/[^/]*\.(MF|SF|RSA|DSA)$"
                  #"META-INF/maven/" })
arichiardi commented 7 years ago

Yeah after you said that I was thinking of sifting but the above looks better. In any case this is probably something that needs to be documented very well somewhere

martinklepsch commented 7 years ago

Maybe it should be the default? I don't see why (by default) you'd want multiple pom.xml inside your uberjar.