Hi @seancorfield, I've been working on a new feature to add license info and text generation out of https://spdx.dev/. License is EPL-1.0 by default but can be changed with a :license/id option, eg with the value "MIT". I implemented this on top of @jr0cket ’s template project (see https://github.com/fmjrey/project-templates/blob/licenses/src/practicalli/licenses.clj and accompanying https://github.com/practicalli/project-templates/pull/37) but then realized it makes sense to have this part of deps-new instead. Happy to move the PR to deps-new, let me know what you think.
Actually the license info is already in the SPDX jar, so I amended my code to set an environment variable which controls the use of that included resource instead of downloading it from their API. I validated this was not recreating any local cache in my .cache dir, and there's no perceptible delay in creating a new project. See https://github.com/fmjrey/project-templates/blob/licenses/src/practicalli/licenses.clj, which also applies str to the passed :license/id.
A nice to have would be a config option to set :license-store to either :jar or :cache, but that's more of a clj-spdx thing to have. So is the jar solution acceptable to you and shall I go ahead with a PR in deps-new?
I imagine https://github.com/seancorfield/deps-new/blob/develop/src/org/corfield/new/impl.clj#L213 would be the place to add the :license/* options, right?
I'm out at a late lunch but will respond in detail when I get home.
Yup, preprocess-options looks like the right place to merge the :license/* stuff. With some conditional bindings above to fetch the non-EPL-1.0 license stuff if needed or set up the defaults. Sounds like fetching the EPL-1.0 license would be cheap, based on the JAR, since that contains everything?
Reading from the jar means reading some JSON out of resource files. That's definitely fast, for EPL or any other in there.
Getting it from the API is probably best going forward. Peter Monks who wrote clj-spdx seems to be confident that not calling init! would make the cache incremental, see https://github.com/pmonks/clj-spdx/issues/22. We need to validate that however, until then I'd rather be safe with the jar data.
It's getting late here, I've pushed my untested changes on https://github.com/fmjrey/deps-new if you are curious to see what it looks like, I'll certainly push force over it though unless I'm totally brilliant.
Go ahead and rework the PR against deps-new as a draftWill do
> One ask I have is that the :licence/id value is coerced with str so that :license/id MIT is accepted (the symbol coerced to "MIT")
Yes, I was wondering about why the new param was not like the others.
> I'd be a little concerned about additional "weight" for the tool and any runtime impact
Me too, though I wanted to start the discussion after my successful attempt yesterday.
The cache is about 22Mb on my .cache directory. So I guess the one time cost at first run is at least what it takes to download that amount, but I suspect a bit more by my own experience and past discussions. Digging a bit, it seems clj-spdx forces the download of the cache, see https://github.com/spdx/Spdx-Java-Library/issues/193 engaged by clj-spdx author about creating the cache because the underlying official java library is always slow to start up. I suppose the main use cases for SPDX require all licenses, e.g to automate dependency checks. For our case it makes more sense to gradually build a cache as requests come in, though I fear in all cases the official java lib downloads the whole listing. So I've posted https://github.com/pmonks/clj-spdx/issues/22#issuecomment-3971863181 on clj-spdx to inquire a bit.
> I do like the idea that folks could create projects under whatever license they want, out of the box, without getting EPL 1.0 and having to change it if they don't like that
So you mean you like the EPL-1.0 default right?Right, yes, the default should stay the same. And maybe an optimization would be the keep the EPL LICENSE file in deps-new under resources and use that when :license/id is not provided, and only invoke the SPDX stuff when :license/id is provided?
makes sense
Interesting. I'd be a little concerned about additional "weight" for the tool and any runtime impact, although deps-new already depends on tools.build and that drags in a lot of stuff so maybe that's not really a concern...
Go ahead and rework the PR against deps-new as a draft and we can discuss specifics inline there. One ask I have is that the :licence/id value is coerced with str so that :license/id MIT is accepted (the symbol coerced to "MIT") as well as :license/id '"MIT"', since string quoting for -T usage is a bit of a pain. That's how deps-new treats most existing arguments.
In the PR summary, can you also provide some notes on what sort of additional run time it adds? (maybe it's negligible, I've no idea)
I do like the idea that folks could create projects under whatever license they want, out of the box, without getting EPL 1.0 and having to change it if they don't like that -- and then being able to get rid of all the apologetic text in the README and elsewhere about how the default is EPL but you can replace the license with whatever you want 🙂
I updated my fork again, with the fix we talked about: spaces and atom removed, added slf-4j-nop, and amended CHANGELOG. Weird, all the strings in licenses.clj have been surrounded by spaces, no idea how.
I'll devise some unit tests tomorrow.
As expected I just pushed-force some corrections on my fork which should work just fine. Before submitting a PR I would like to highlight the following:
1. The use of license data from the SPDX jar is not a long-term solution. Sooner or later someone is bound to need a more recent data set. For now this replaces current behavior, nothing is lost, and we gain an easy way out of the default EPL-1.0.
2. Going forward we need a way to get license info from the SPDX API without a noticeable delay in creating a project. I suggest we provide a new :licenses template option which defaults to :jar but may be set to :cache or :full-cache. The first one will set the org.spdx.useJARLicenseInfoOnly environment variable to true and the others won't, and the call to init! that takes time on first run only happens with the third one. Alternative: we default to :cache and hope for the best, reverting if needed, what do you think?
3. Some licenses text have placeholders for things like year and owner. It's not the case of EPL-1.0, but as an example the MIT license text contains Copyright (c) <year> <copyright holders>. The SPDX specs describe these as https://spdx.github.io/spdx-spec/v3.0.1/annexes/license-matching-guidelines-and-templates/#guideline-replaceable-text and seems to have moved on https://spdx.github.io/spdx-spec/v3.0.1/annexes/license-matching-guidelines-and-templates/#legacy-text-template-format. Based on the spec wording and skimming through the API I don't believe SPDX at present is intended for generating license files, but instead for comparing and validating conformity with a template. Also I don't believe there is any standardized set of placeholders. I've added a comment in the README reflecting that.
4. The inclusion of clj-spdx makes the create task output the classic SLF4J warnings about using NOP logger. I believe this needs to be corrected in clj-spdx by using slf4j-nop instead of slf4j-api . I've told the author so hopefully he'll check and make a release?
5. TODO before the PR: add some test cases and amend CHANGELOG .
Oh another caveat I forgot to mention in point 2 above: there is no easy way to load from API and then revert to loading from jar. For that you either have to call direct the SPDX API (and forgo the use of the official cache, you'll need you own separate one), or you just have to rerun the tool with another :licenses option.
Two Qs:
• Does the system property org.spdx.useJARLicenseInfoOnly really have a space before and after as part of the name?
• Given that id->license is only called once per run of the tool, is the init-called? atom needed? Or are you thinking of library usage, where someone might potentially generate multiple projects per run?
Otherwise, yup, looks pretty good.
Re: points above:
1. How often does the data set change, and wouldn't the JAR get republished with an update for those changes?
2. :licenses logic looks good (modulo my Q about init-called? above.
3. A note in the README is fine for this -- users have to make various changes anyway in all generated projects.
4. Since clj-spdx is a library it shouldn't add the nop impl. I'd be inclined to add it to deps-new directly -- anyone using deps-new as a library should add :exclusions if they don't want the nop impl (maybe add a note in the README).
5. Yup. I've been a bit lax about tests here since some of it is hard to write tests for.
1. not sure where these spaces are coming from 2. good point, just a defensive habit I suppose, we can let go of the atom 1. a few releases per year https://mvnrepository.com/artifact/org.spdx/java-spdx-library/versions 2. 👍 3. 👍 4. Oh so the advice I gave to clj-spdx is wrong then. I won't be on my computer until tomorrow evening. Can you please give more guidance to Peter Monk in https://github.com/pmonks/clj-spdx/issues/22? 5. the famous tdd debate...
4. No. Peter has me blocked on GitHub so I can't interact with any of his projects -- and he notes he's on Discord but he has me blocked there too. I think part of why he's no longer on Slack here is because he doesn't want to interact with me here. But feel free to ask him to unblock me so I can offer my thoughts 🙂 or else you will be forever passing on any thoughts from me!
I pushed some unit tests on my fork, and writing these made me change the logic slightly:
1. it throws when a license is not found, which I think is better than silently generating some empty files/fields
2. it now also provides a :licenses entry in the map returned, as it allows the test to verify :jar is really set as default
I think that should be ready for a PR, what do you think?
Both of those sound good so, sure, let's move to a full PR!