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

Rollup of various changes on my side #26

Merged
merged 20 commits into from
Mar 19, 2018

Conversation

promovicz
Copy link
Contributor

This series of patches contains both some cleanups and some functional enhancements:

  • Support for overriding the Java version
  • Support for an archive JAR file
  • "Cleaner" handling of JCKit

Would you be willing to accept these as-is or should I separate the functional changes from the rest?

Copy link
Owner

@martinpaljak martinpaljak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing license header from file (MIT)

@martinpaljak
Copy link
Owner

All in all they look like good cleanups, give me some time to go through them.

Regarding File vs Path - there was something specific that made me decide to stick to Path-s, IIRC related to weird chars and Windows.

@@ -287,6 +289,10 @@ public void setExport(String msg) {
output_exp = msg;
}

public void setJar(String msg) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing in README?

@martinpaljak martinpaljak merged commit daec8c4 into martinpaljak:master Mar 19, 2018
@promovicz
Copy link
Contributor Author

Regarding File vs Path: I'll be investigating this and try to find out what your problem was. All in all Path is the more modern API. Too bad it conflicts with ant so badly. File seemed to yield simpler code, but maybe that was a mistake.

@martinpaljak
Copy link
Owner

Maybe it was a brainfart. I Usually leave a comment at the troublesome point, but as I do not see one, maybe it did not exist or got resolved. Thank you for the cleanup!

@martinpaljak
Copy link
Owner

Right. Try to run in a folder with space in the name.

@promovicz
Copy link
Contributor Author

Yuk. Sounds like it will affect many users. I will try to figure this out tomorrow. Also forgot documenting another option so this needs another PR anyway.

@promovicz
Copy link
Contributor Author

Actually couldn't leave this alone and took a look. The cause is not the use of File but how the files get passed as arguments to ant. Working on solution.

@martinpaljak
Copy link
Owner

FYI: After giving it some thought, I removed javaversion from <javacard>, as this feels like a very specific tweak, which should not "pollute" the absolute minimum required possible configurable, which is the javacard kit.

@gitfineon
Copy link

Hey @martinpaljak , tried to port some build stuff to ant-javacard but one project requires javac -source 1.6 -target 1.6. I was very pleased finding an option for that in your README.md but then found this comment.

Is there another way to force version? Because I get unsupported class file format of version 52.0. and this is exactly what happens when I remove those javac options in the old build file. My goal is to port without touching sources. But I'll take a look the "class file format" ... if it is a minor change.

@martinpaljak
Copy link
Owner

@gitfineon could you please clarify your scope and open a new issue ?

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.

None yet

3 participants