unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#32295] [PATCH] gnu: clojure: Fix index generation.
@ 2018-07-28  6:08 Gábor Boskovits
  2018-07-28  6:29 ` Gábor Boskovits
  2018-07-29 10:01 ` bug#32295: " Gábor Boskovits
  0 siblings, 2 replies; 5+ messages in thread
From: Gábor Boskovits @ 2018-07-28  6:08 UTC (permalink / raw)
  To: 32295; +Cc: Gábor Boskovits

* gnu/packages/java.scm (clojure)[arguments]: Add phase 'fix-manifest-classpath to remove
offending directory entry making indexing fail.
---
 gnu/packages/java.scm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm
index 2ba6d76ca..0ff92c763 100644
--- a/gnu/packages/java.scm
+++ b/gnu/packages/java.scm
@@ -1862,6 +1862,11 @@ new Date();"))
                   "test-generative-src"
                   "tools-namespace-src"))
                #t))
+           (add-after 'unpack 'fix-manifest-classpath
+             (lambda _
+               (substitute* "build.xml"
+                 (("<attribute name=\"Class-Path\" value=\".\"/>") ""))
+               #t))
            ;; The javadoc target is not built by default.
            (add-after 'build 'build-doc
              (lambda _
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [bug#32295] [PATCH] gnu: clojure: Fix index generation
  2018-07-28  6:08 [bug#32295] [PATCH] gnu: clojure: Fix index generation Gábor Boskovits
@ 2018-07-28  6:29 ` Gábor Boskovits
  2018-07-29  3:05   ` Alex ter Weele
  2018-07-29 10:01 ` bug#32295: " Gábor Boskovits
  1 sibling, 1 reply; 5+ messages in thread
From: Gábor Boskovits @ 2018-07-28  6:29 UTC (permalink / raw)
  To: 32295

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

I could use some help from someone more familiar with clojure uses.

On current master clojure does not build, as the manifest contains
Class-Path: .
and jar -i throws exception in phase 'generate-jar-indices.

The patch I proposed removes the classpath from the manifest. Do you think
this can interfere with normal uses of clojure?

Another possible solution would be to delete the index generation phase,
but I'm not sure about the implications here either.

WDYT?

[-- Attachment #2: Type: text/html, Size: 614 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#32295] [PATCH] gnu: clojure: Fix index generation
  2018-07-28  6:29 ` Gábor Boskovits
@ 2018-07-29  3:05   ` Alex ter Weele
  2018-07-29  6:38     ` Gábor Boskovits
  0 siblings, 1 reply; 5+ messages in thread
From: Alex ter Weele @ 2018-07-29  3:05 UTC (permalink / raw)
  To: Gábor Boskovits; +Cc: 32295

Gábor Boskovits <boskovits@gmail.com> writes:

> I could use some help from someone more familiar with clojure uses.
>
> On current master clojure does not build, as the manifest contains
> Class-Path: .
> and jar -i throws exception in phase 'generate-jar-indices.
>
> The patch I proposed removes the classpath from the manifest. Do you
> think this can interfere with normal uses of clojure?
>

Gábor,

I applied your patch and did the following:

$ guix environment guix -- make -j4 && ./pre-inst-env guix environment --ad-hoc icedtea clojure
...
$ java -jar $GUIX_ENVIRONMENT/share/java/clojure-1.9.0.jar
Clojure 1.9.0
user=> (inc 0)
1
user=> (clojure.set/difference #{1 2} #{1})
ClassNotFoundException clojure.set  java.net.URLClassLoader.findClass (URLClassLoader.java:381)
user=> (require 'clojure.set)
nil
user=> (clojure.set/difference #{1 2} #{1})
#{2}

Constructing the environment caused Clojures's test suite to run with no
failures. Based on a cursory test, I think that Clojure is working as
expected.

I also looked into what the removed line
means. https://ant.apache.org/manual/Tasks/jar.html explains that
<manifest> in the "jar" task specifies an inline
manifest. https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#classpath
explains the meaning of a Class-Path in a manifest. So, I believe that
means that at runtime, JARs in the same directory as the Clojure JAR
will be available on the classpath.

I believe that the <attribute name="Class-Path" value="."/> line was
added in this commit
https://github.com/clojure/clojure/commit/36868a7734f15c51eb1831aa9d72a14544496987#diff-2cccd7bf48b7a9cc113ff564acd802a8R30. The
age of the commit and the commit message lead me to believe that it was
added as a convenience during the early development of Clojure and may
be removable upstream now. I may ask about that on the Clojure mailing
list.

> Another possible solution would be to delete the index generation
> phase, but I'm not sure about the implications here either.
>
> WDYT?

I also tried this and did the same as above. Clojure's test suite ran
again and had no failures. However, generate-jar-indices in (guix build
ant-build-system) has a docstring which suggests that the phase is
necessary to prevent garbage collection of the dependencies of the JAR
file. So it would seem like your patch is the best way to go ☺

P.S.: Gábor, thanks for your work on Java packages recently! With a
maven-build-system, I think I can bootstrap leiningen, which I'd really
like to see in the distribution.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#32295] [PATCH] gnu: clojure: Fix index generation
  2018-07-29  3:05   ` Alex ter Weele
@ 2018-07-29  6:38     ` Gábor Boskovits
  0 siblings, 0 replies; 5+ messages in thread
From: Gábor Boskovits @ 2018-07-29  6:38 UTC (permalink / raw)
  To: alex.ter.weele; +Cc: 32295

[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]

Alex ter Weele <alex.ter.weele@gmail.com> ezt írta (időpont: 2018. júl.
29., V, 5:05):

> Gábor Boskovits <boskovits@gmail.com> writes:
>
> > I could use some help from someone more familiar with clojure uses.
> >
> > On current master clojure does not build, as the manifest contains
> > Class-Path: .
> > and jar -i throws exception in phase 'generate-jar-indices.
> >
> > The patch I proposed removes the classpath from the manifest. Do you
> > think this can interfere with normal uses of clojure?
> >
>
> Gábor,
>
> I applied your patch and did the following:
>
> $ guix environment guix -- make -j4 && ./pre-inst-env guix environment
> --ad-hoc icedtea clojure
> ...
> $ java -jar $GUIX_ENVIRONMENT/share/java/clojure-1.9.0.jar
> Clojure 1.9.0
> user=> (inc 0)
> 1
> user=> (clojure.set/difference #{1 2} #{1})
> ClassNotFoundException clojure.set  java.net.URLClassLoader.findClass
> (URLClassLoader.java:381)
> user=> (require 'clojure.set)
> nil
> user=> (clojure.set/difference #{1 2} #{1})
> #{2}
>
> Constructing the environment caused Clojures's test suite to run with no
> failures. Based on a cursory test, I think that Clojure is working as
> expected.
>
>
Thanks for the review, I will apply this patch then.


> I also looked into what the removed line
> means. https://ant.apache.org/manual/Tasks/jar.html explains that
> <manifest> in the "jar" task specifies an inline
> manifest.
> https://docs.oracle.com/javase/8/docs/technotes/guides/jar/jar.html#classpath
> explains the meaning of a Class-Path in a manifest. So, I believe that
> means that at runtime, JARs in the same directory as the Clojure JAR
> will be available on the classpath.
>
> I believe that the <attribute name="Class-Path" value="."/> line was
> added in this commit
>
> https://github.com/clojure/clojure/commit/36868a7734f15c51eb1831aa9d72a14544496987#diff-2cccd7bf48b7a9cc113ff564acd802a8R30.
> The
> age of the commit and the commit message lead me to believe that it was
> added as a convenience during the early development of Clojure and may
> be removable upstream now. I may ask about that on the Clojure mailing
> list.
>
> > Another possible solution would be to delete the index generation
> > phase, but I'm not sure about the implications here either.
> >
> > WDYT?
>
> I also tried this and did the same as above. Clojure's test suite ran
> again and had no failures. However, generate-jar-indices in (guix build
> ant-build-system) has a docstring which suggests that the phase is
> necessary to prevent garbage collection of the dependencies of the JAR
> file. So it would seem like your patch is the best way to go ☺
>
> P.S.: Gábor, thanks for your work on Java packages recently! With a
> maven-build-system, I think I can bootstrap leiningen, which I'd really
> like to see in the distribution.
>
>

[-- Attachment #2: Type: text/html, Size: 3982 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#32295: gnu: clojure: Fix index generation.
  2018-07-28  6:08 [bug#32295] [PATCH] gnu: clojure: Fix index generation Gábor Boskovits
  2018-07-28  6:29 ` Gábor Boskovits
@ 2018-07-29 10:01 ` Gábor Boskovits
  1 sibling, 0 replies; 5+ messages in thread
From: Gábor Boskovits @ 2018-07-29 10:01 UTC (permalink / raw)
  To: 32295-done

[-- Attachment #1: Type: text/plain, Size: 68 bytes --]

Fixed on master as commit 62196859227a91a206deceaae2829d9c32308347.

[-- Attachment #2: Type: text/html, Size: 337 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-07-29 10:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-28  6:08 [bug#32295] [PATCH] gnu: clojure: Fix index generation Gábor Boskovits
2018-07-28  6:29 ` Gábor Boskovits
2018-07-29  3:05   ` Alex ter Weele
2018-07-29  6:38     ` Gábor Boskovits
2018-07-29 10:01 ` bug#32295: " Gábor Boskovits

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).