Skip to content

Gradle cleanup #39

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

adricasti
Copy link

Hi Bryan, I like your project and since the tools for working with Java Cards are not frequently updated I removed dependencias since they tend to become technical debt. The build.gradle file is now simpler and I changed the README to help newcomers.

Copy link
Owner

@BryanJacobs BryanJacobs left a comment

Choose a reason for hiding this comment

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

I appreciate the contribution, but I've left a few review comments. I could see my way to merging something like this but it would need to remove a bit less of the existing functionality. As it stands this is less a "cleanup" and more a "rewrite".

Especially, it still needs to be possible to build a jcardsim jar with zero javacard dependencies present on the build machine, and it needs to be IMpossible to build an applet that relies on a jcardsim-only function.

build.gradle Outdated
jcdk files('../jckit/lib/api_classic-3.0.5.jar')

// Main and test dependencies use JCardSim
implementation files('../jcardsim-3.0.5.jar')
Copy link
Owner

Choose a reason for hiding this comment

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

JCardSim shouldn't be a dependency for the javacard applet. It's only used for testing builds.

@adricasti
Copy link
Author

Overall, my main goal was to get rid of the klinec javacard plugin, and ideally be able to use Visual Studio Code with the Java Extension Pack. From the command line everything is working fine, but in Visual Studio Code I still have a nagging issue in AppletBasicTest.java where VSCode is unable to resolve import javax.smartcardio. But it still can run the tests, and build from the Gradle plugin, is annoying to have that warning though.

Copy link
Owner

@BryanJacobs BryanJacobs left a comment

Choose a reason for hiding this comment

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

Almost there now, but there should be some way to build with your choice of version of the JCDK without editing the sources.

@adricasti adricasti requested a review from BryanJacobs March 4, 2025 10:12
@BryanJacobs
Copy link
Owner

Sorry to be so late responding, but this still doesn't appear to work properly with SDKs other than the last login-required one from Oracle.

FIDO2Applet on  main [?⇡] via 🅶 v8.2 via ☕ v1.8.0 via 🐍 v3.13.2 
❯ ls $JC_HOME                                                                                                                                
api_export_files  bin  classic_simulator  COPYRIGHT.html  document.css  legal  lib  RELEASENOTES-CL.html  shared  Uninstaller                

FIDO2Applet on  main [?⇡] via 🅶 v8.2 via ☕ v1.8.0 via 🐍 v3.13.2 
❯ ./gradlew build

> Task :classesCap FAILED
[...]/FIDO2Applet/src/main/java/us/q3q/fido2/ResidentKeyData.java:3: error: package javacard.framework does not exist
import javacard.framework.ISO7816;

I'd love to merge this but it needs to be able to build the javacard applet when using older SDKs. This fails as written because of three problems:

  1. the libraries are located at lib/api_classic.jar, but this code assumes they're located at lib/api_classic-<version>.jar. This can be fixed with a fallback like the below:
diff --git a/build.gradle b/build.gradle
index bb8c56f..6e4c46f 100644
--- a/build.gradle
+++ b/build.gradle
@@ -13,9 +13,14 @@ configurations {
     jcdk
 }
 
+var jcdk_jar = new File("${System.getenv('JC_HOME')}/lib/api_classic-${JavaCardVersion}.jar")
+if (!jcdk_jar.exists()) {
+    jcdk_jar = new File("${System.getenv('JC_HOME')}/lib/api_classic.jar")
+}
+
 dependencies {
     // JCDK dependencies for CAP generation
-    jcdk files("${System.getenv('JC_HOME')}/lib/api_classic-${JavaCardVersion}.jar")
+    jcdk files(jcdk_jar)
     
     // Test dependencies use JCardSim
     implementation 'com.klinec:jcardsim:3.0.6.0'
  1. Older SDKs' converter binary does not support the -target argument.

  2. Older SDKs do not support Java Class file format version 51.

I don't see cleaning up as being more important than being portable to a wider variety of build environments, so I'd rather not lose support for older tools just to reduce the size of the build.gradle file. If you can ensure this works with both older and newer SDKs I'd be glad to merge it. I really do appreciate the contribution, but what's being improved (the size and "cleanness" of the gradle file) needs to be greater than what's being lost (compatibility).

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