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

The clj-i18n .jar is distributed as an AOT-compiled lib #47

Closed
vemv opened this issue Feb 13, 2021 · 8 comments
Closed

The clj-i18n .jar is distributed as an AOT-compiled lib #47

vemv opened this issue Feb 13, 2021 · 8 comments
Labels

Comments

@vemv
Copy link

vemv commented Feb 13, 2021

Hi!

The distributed clj-i18n artifact is AOT-compiled:

image

This is known as a source of issues (ref) and would appreciate if newer releases were source-only.

Thanks - V

@vemv vemv added the bug label Feb 13, 2021
@vemv
Copy link
Author

vemv commented Feb 16, 2021

Hi @jonathannewman, sorry to bother. What do you think of this issue?

I verified locally that this project's lein test is able to succeed without :aot.

A specific problem of AOT is that it couples bytecode to a specific Clojure compiler. e.g. there's no particular guarantee that .class bytecode emitted by Clojure 1.x will be able to be consumed by Clojure 1.y

@jonathannewman
Copy link
Contributor

Thanks @vemv I think it makes sense to remove the AOT compilation. If you put up a PR against the repo, I'd be happy to review it.

@vemv
Copy link
Author

vemv commented Feb 17, 2021

Hi again, thanks for the response!

The only necessary changes are removing :aot and :main from project.clj. Out of that, make followed by lein test will pass.

I'd rather not open a PR because of its trivial size, are you ok with that?

@jonathannewman
Copy link
Contributor

I'd prefer a PR for several reasons:

  • It gives an opportunity for other contributors to weigh in on the merit of the work and to make suggestions that we may not be aware of
  • It helps assure that we have a good history of the changes made and why they are made
  • It makes it easier to revert the work if issues arise

I realize that the change may seem trivial from an implementation standpoint, but it is in use as part of Puppet's Enterprise product and thus has a required level of rigor.

I appreciate your understanding, and thanks again for bringing it up.

@vemv
Copy link
Author

vemv commented Feb 18, 2021

Thanks! I agree.

To prevent any IP concerns from getting in the way, could you open the PR? You can cc/ me in the PR itself and would be happy to provide any missing info, rationale, etc.

@jonathannewman
Copy link
Contributor

@vemv see #48

@vemv
Copy link
Author

vemv commented Feb 18, 2021

Thanks! I subscribed to its notifications.

@jonathannewman
Copy link
Contributor

0.9.1 should be on clojars now with the change. Thanks for the submission @vemv ! Going to close this since we have resolved the issue.

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

No branches or pull requests

2 participants