vletoux / GidsApplet

Generic Identity Device Specification Applet
GNU General Public License v3.0
102 stars 38 forks source link

Build cleanup #24

Closed rpavlik closed 10 months ago

rpavlik commented 10 months ago
vletoux commented 10 months ago

While this seems awesome work, I cannot accept it as the CI is indeed impacted. You can see the (basic) script at the root of the repo

rpavlik commented 10 months ago

Right, sorry I should have left this as draft until I fixed CI.

vletoux commented 10 months ago

Trying to test this PR. Unfortunately, I'm not using VSCode (my Eclipse environement could not be reproduced anymore)

I'm falling at adding the java source to VSCode image

As the option "Add Folder to Java Source Path" is missing

Copying below some screenshots to prepare a change for the main readme to explain how to build the code from the source

  1. install VSCode

image

  1. Install java coding pack : go to https://aka.ms/vscode-java-installer-win and run the installer image

  2. Install JDK Once VSCode reloaded, install a JDK image

    Select 11 Lts image

image

I'm then falling at compiling the project

vletoux commented 10 months ago

You have to click on "run" to force the debugger to be asked (in this case "java") A bunch of files is then downloaded.

You can then see the option "add source" image

vletoux commented 10 months ago

the ext/sdks is also empty I don't know how to load this from submodule in VSCode (will need to be documented for newbies)

vletoux commented 10 months ago

(I've downloaded the source code as .zip from your local branch)

martinpaljak commented 10 months ago

I could set up a somewhat boilerplate maven + ant-javacard + jcardsim setup, with a very small ant build (I use ant only for airgapped applet building) that should be standard import to intellij or eclipse and pipe the tests through jcardsim.

rpavlik commented 10 months ago

@martinpaljak that might be nice, that's definitely beyond my abilities. See this branch for the furthest-improved version of this build system I've made: https://github.com/vletoux/GidsApplet/pull/26

(there are two variants generated from the build - both of them include the "main" sources, plus one other dir. It picks a config implementation. In the PR linked above, I have the tests running against jcardsim and passing.)

I just rebased the whole stack of branches and force-pushed, also adding a commit that I think should fix the ci? Been a while since I used travis.

On windows, I used:

Unfortunately by default GitHub does not include submodules in downloaded archive files, which is why the ext directory is missing things. I think they have a setting you can enable that now?

vletoux commented 10 months ago

I've installed the VSCode extension to load ant image That loads an "Ant Target runner" at the bottom left. I can know select the target and build the code

I got a complain about requesting JDK version 17 (I've only installed JDK version 11 as part of my "if should run on older version" policy) image

I'm able to add the unit test by clicking on the "lab" icon at the left and selecting "Junit" image

I've been able to compile most of the java files by adding "add source folder" and adding manually the .jar as reference. I think the following file should be added to the PR: .vscode/settings.json

{
    "java.project.sourcePaths": [
        "src/test",
        "src/main",
    "src/import2048",
    ],
    "java.project.referencedLibraries": [
        "ext/sdks/jc305u3_kit/lib/*.jar",
        "test-lib\\**\\*.jar"
    ]
}

But unfortunately I'm unable to compile the test files because of the missing simulator import and some libraries such as javax.xml.bind

I've read the CI and this is ok for me.

We are close !

What's missing for me is the instructions to build for VSCode as this is what you are using and fixing that unit test compile / run. And fixing that instructions for ant to build OR changing ant to match the framework.

Sorry, that's a long time I did not play with javacard (more used to Active Directory audits and visual studio - not the code version)

vletoux commented 10 months ago

the test-lib directory may have also be added in .gitignore This is the directory created by the vscode test extension to download the junit lib

vletoux commented 10 months ago

Changed JDK from version 17 to version 11 and still having this error message image

image

(code clean / recompiled & vscode closed and restarted)

rpavlik commented 10 months ago

If you look at #25 I got the tests to build by vendoring or downloading the test dependencies at build time.

Yeah unfortunately, to build the javacard applet itself you need 8 or 11, 17 is too new even when using the 3.0.5 kit to build for 2.2.1, and part of what I love about this applet is the wide card support :) However, the VS Code extension uses 17, so I think it actually itself uses 17 to build the tests and run them locally when I am on my Linux machine.

The maven thing might be handy still since the VS code Java extension can import Maven directly, as can other stuff. Apparently tooling support for Ant is lesser, presumably it's a little too flexible?

Whether or not to ship vscode settings files is a controversy because some of the contents tends to be user preference rather than project configuration. However, it might make sense here.

OK I added them.

vletoux commented 10 months ago

setting JAVA_HOME fixes the ant build image

image

vletoux commented 10 months ago

Last thing: make tests build

I've seen reference inside settings.json (in .vscode) about jcardsim: ext/jcardsim Did you added it as a submodule ? I don't see any update there.

vletoux commented 10 months ago

After manually adding the jcardsim.jar in ext, the tests don't compile yet. I'm falling at loading the following imports: image

I think module-info.java is missing (java9 novelty) with requires java.smartcardio; Not sure about it as I was working with java8 previously

rpavlik commented 10 months ago

the tests are building properly in #26, I split the PR up. Don't waste time redoing what I did, just grab the "head" branch :) Each time I make a change here I've been actually doing the dev on that branch, then rebasing and updating intermediate branches.

This is a screenshot of the history in gitk - each of those branches is one of the PRs I made. Unfortunately github is even worse than gitlab at handling dependent PRs.

image

I haven't been able to fix that javax.smartcardio red squiggle in the editor, but it does build and debug the tests properly. (Adding module-info only made it worse, in fact.) That's how I did the research into the failing test - by stepping through the code in the vscode java debugger. (I don't see a javax.xml squiggle, though, that is fixed in my latest code.) You may need to choose "Restart java language server" after making a change for vscode to notice it.