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

adding jar offset rewriting and support for preamble scripts from files #7

Merged
merged 1 commit into from
Jan 16, 2018

Conversation

mbjarland
Copy link
Contributor

@mbjarland mbjarland commented Jan 15, 2018

After realizing that the original lein-bin repository was dead and failing to find this one for months, I finally did. This PR is replicating the following PR I did in that repo a while back:

Raynes/lein-bin#29

The original comment from that PR pasted below for reference (in case the original lein-bin repo goes away etc).

TLDR: rewriting jar files the way this lib currently does (prepending data to the jar) leaves the jar file integrity compromised. You can see this by issuing:

$> unzip -l bad_prelude.zip 
Archive:  bad_prelude.zip
warning [bad_prelude.zip]:  317 extra bytes at beginning or within zipfile
  (attempting to process anyway)
  Length      Date    Time    Name
...
$>

on a jar file created with lein-binplus. We can see that the file is processed anyway, but unzip gives us a warning. Other tools such as zipdetails does not fare so well:

$> zipdetails bad_prelude.zip 
No Central Directory found
$>

this PR uses clj-zip-meta to rewrite the jar-internal offsets so that the jar file integrity is regained.

Should be noted that clj-zip-meta is not as performant as I would have liked. It can take a few seconds to rewrite the offsets. I know how to increase performance of clj-zip-meta, just haven't had the time. Still, I would rather have it correct and a few seconds slow than incorrect and fast : )

I also added the ability to use external prelude scripts (from files) as some of the preludes I've come up with (drip jvm launcher support for exapmle) turned out to be so complex I would rather not hard code them into the project.clj file.

I submit this pull request in all humbleness and I have to say I'm quite happy to find a maintained fork of lein-bin on github.

Original Pull Request Comment

First of all, thank you for this library. I have been looking for a way to do this for ages and this really solves the problem in an elegant and once-and-for-all way.

With that being said, the current state of this library still left me with two items missing from my wish list of perfection:

  • jar file meta data offsets. The current method of just appending text to the front of the jar file leaves the jar file integrity compromised as none of the jar/zip internal offsets are accurate after the prepend. We would like to rewrite the resulting jar file zip meta data offsets so that tools such as unzip would still consider the zip file valid (as in with valid offsets. Try adding a longer preamble script with the current implementation and then either executing it with java or just running unzip -l on the resulting jar).
  • the ability to use custom preamble scripts to be able to optionally support launch accelerators such as drip. Or whatever insanity users of this library come up with.

I'm somewhat new to clojure and solving this problem led me down quite a journey. In (not so) short:

  • look around and realize there is no library on the jvm to read the zip file meta data as specified by the zip file specification. I'm not talking about java's ZipFile and friends which only deal with entries and have no concept of things like central directories or local headers as defined in the zip file specification.
  • be dissappointed and meditate over the un-reality of the fact that the base distribution unit of all java appliactions is jar files and we have nothing that can actually read them properly.
  • pick yourself up and boldly charge into the frey. Zip files. I mean...how hard can it be?
  • figure out which binary format library to use in clojure. The top contenders seemed to be octet and buffy, but there are others. Try them all out and try to implement zip files in them.
  • realize that none of them solve the problem the way I needed to. As none of them solve the problem out of the box, settle for the simplicity of octet. It has a clean and terse specification dsl.
  • understand the zip file format and how the offset calculations work and why adding a prelude without fixing the offsets breaks the zip file.
  • realize that octet was missing support for the custom length field pattern (ref lengths, see the linked octet pull request for details) used in the zip file specification and it was thus not possible to write a spec in octet for the zip file format. Could not get buffy to work either. Count to ten.
  • write a somewhat extensive pull request to octet adding the required support for reference length fields.
  • wait for pull request to be accepted. Thank you @niwinz for accepting it.
  • with the proper tooling for writing a specification for zip files finally in place in octet, write the library missing from the jvm: clj-zip-meta. Yay, we can now read zip/jar file meta data and have it returned as clojure data structures.
  • iterate...publish to clojars.
  • make changes to this library now using the ability to rewrite zip offsets using the newly written lib. Also add custom preamble support.
  • fight with command line parsing issues for an eternity to get optional (as in, use if installed) drip support working.
  • create this pull request

apologies for the rambling. It was some journey and I would like to thank the writers of this library for writing something so good that it was worth the above trouble to improve on.

If you deem this change acceptable, great. If not, this still solves my local problem so again, many thanks for this library.

@mbjarland mbjarland changed the title adding jar offset rewriting and support for custom preamble scripts adding jar offset rewriting and support for preamble scripts from files Jan 15, 2018
@BrunoBonacci
Copy link
Owner

thanks @mbjarland for this PR, I will review/merge it hopefully tomorrow.

@BrunoBonacci BrunoBonacci merged commit 59e8a25 into BrunoBonacci:master Jan 16, 2018
@BrunoBonacci
Copy link
Owner

thanks @mbjarland, the PR is now merged and available in clojars as lein-binplus 0.6.4
https://clojars.org/lein-binplus/versions/0.6.4

@mbjarland
Copy link
Contributor Author

that was quick work! Many thanks. I have a couple of lein templates waiting for this release.

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.

2 participants