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

Allow blank tsaurl value #71

Closed
tresf opened this issue Dec 17, 2019 · 4 comments
Closed

Allow blank tsaurl value #71

tresf opened this issue Dec 17, 2019 · 4 comments

Comments

@tresf
Copy link

tresf commented Dec 17, 2019

Ant tasks are cumbersome in their own right, but one thing that's particularly frustrating about ant is the lack of good conditional support in a target.

As a side-effect, ugly patterns like this emerge...

<!-- Handle missing property using "unless" -->
<target name="my-target-condition-1" unless="some.property">
   <!-- ... --> 
</target>
<!-- Handle existing property using "if" -->
<target name="my-target-condition-2" if="some.property">
   <!-- ... --> 
</target>
<!-- Wrap conditional property into depends -->
<target name="my-target" depends="my-target-condition-1,my-target-condition-2">
   <!-- ... --> 
</target>

Although this pattern is a side-effect of ant (not jsign specifically), handling the parameter's lib-side can avoid this...

For example... if a build system has two build configurations:

  • Self-signed build system
  • Trusted CA-signed build system

It would make sense to have a way to provide the ant-task an empty value (e.g. for tsaurl=) instead of two separate targets.

Expanding this to JSign:

<target name="sign-exe">
   <taskdef name="jsign" classname="net.jsign.JsignTask" classpath="path/to/jsign-3.0-SNAPSHOT.jar"/>

   <jsign file="my-program.exe"
      name="My Program"
      url="https://ebourg.github.io/jsign/"
      keystore="my-keystore.pfx"
      alias="my-alias"
      storepass="P@ssw0rd"
      keypass="P@ssw0rd"
      #### a blank to skip timestamping? ####
      tsaurl="${signing.tsaurl}"
   />
</target>
  • Since (i assume) the JSign API aims to be strict, it may warrant a soft warning, which ant users can ignore.
  • If a blank value is undesired for strictness purposes, perhaps an intentional flag to ignore (e.g. "-1", "ignore", etc).

Note, this same problem occurs for many command-line invocations; ant users are used to it. If the request is closed as wontfix, I would completely understand. 😄

@ebourg
Copy link
Owner

ebourg commented Dec 18, 2019

I don't mind ignoring blank values for the tsaurl parameter, but in your example if ${signing.tsaurl} isn't defined it won't be blank, it'll be the string "${signing.tsaurl}".

For the command line I don't think it's possible to ignore a blank value, because the parser expects a parameter after --tsaurl.

@tresf
Copy link
Author

tresf commented Dec 18, 2019

in your example if ${signing.tsaurl} isn't defined it won't be blank, it'll be the string "${signing.tsaurl}".

Good catch. For this, I would use a property getter/setter.

<!-- blank if not already set -->
<condition property="signing.tsaurl" value="">
   <not>
      <isset property="signing.tsaurl"/>
   </not>
</condition>

... or less obvious... because... ant 😆

<!-- blank if not already set -->
<property name="signing.tsaurl" value=""/>

For the command line I don't think it's possible to ignore a blank value, because the parser expects a parameter after --tsaurl.

This would depend on invocation, but '' or "" would work, no?

@ebourg
Copy link
Owner

ebourg commented Dec 18, 2019

This would depend on invocation, but '' or "" would work, no?

I don't know, this has to be tested.

Would you want to submit a PR implementing this? I can review and merge it into the upcoming version 3.0. I think a simple change in SignerHelper will do it.

@tresf
Copy link
Author

tresf commented Dec 18, 2019

I have the code staged here master...tresf:patch-1.

As I'm using it, I have some conflicts with its behavior...

  1. My <!-- blank if not already set --> technique is flawed, it modifies parameters that may be used elsewhere.
  2. Your point it'll be the string "${signing.tsaurl}". means I still need additional variables and/or conditional logic.
  3. Ant's own signjar task behaves the same way.

For these reasons, I'm inclined to say we're breaking a -- albeit very ugly -- ant paradigm and we should leave it as-is. 🙁

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

No branches or pull requests

2 participants