feat: implement version pinning for JBang aliases#2433
feat: implement version pinning for JBang aliases#2433maxandersen wants to merge 1 commit intojbangdev:mainfrom
Conversation
maxandersen
force-pushed
the
version-pinning
branch
from
20f4243 to
dce593f
Compare
| * Version requested by user via alias:version syntax. Transient because it's | ||
| * not stored in catalog files - only used at runtime for version resolution. | ||
| */ | ||
| public transient String requestedVersion; |
There was a problem hiding this comment.
this is not needed - just calculate on need to know basis.
maxandersen
force-pushed
the
version-pinning
branch
from
dce593f to
9ccae05
Compare
Refactor alias version parsing/pinning to avoid side effects during lookup and apply version/property logic in the resolver layer with proper context. - Add AliasRef for side-effect-free parsing of alias:version@catalog - Add AliasVersionPinner for property-first and pattern-based pinning - Stop resolving properties inside Alias.resolve() - Keep version pinning from propagating through alias chains - Update docs examples and fix alias-chain behavior; format examples as tables - Adjust tests to avoid network/WireMock dependencies Made-with: Cursor
maxandersen
force-pushed
the
version-pinning
branch
from
9ccae05 to
f2a5da0
Compare
|
@maxandersen , the logic that is required is described in the following Python/Java like pseudocode: import org.apache.commons.text.StringSubstitutor
String scriptRef = "camel:4.17.0@apache/camel"
# alias object can be null.
# version is an empty string or a valid version string ("1.0.2")
(Alias alias, String version) = getAlias(scriptRef)
if not alias:
return
# Set version to the "default-version" property
# if version is empty. "default-version" might
# not be defined in which case it is also set by
# default to the empty string.
if version.length() == 0:
version = alias.defaultVersion()
if version.length() > 0:
# replace all instances of "${version}" in string values
# inside the alias record with the value of version.toString()
# Convert the alias object to its JSON representation
jsonText0 = alias.toJson()
# Create a version HashMap
versionMap = {"version": version}
# Replace all instances of "${version}" with version.toString()
sub = StringSubstitutor(versionMap)
jsonText1 = sub.replace(jsonText0)
# Create a new alias object from the version expanded JSON text
alias = Alias.fromJson(jsonText1)
# Continue using alias objectUsing the following {
"aliases": {
"camel": {
"default-version": "4.18.0",
"script-ref": "dsl/camel-jbang/camel-jbang-main/dist/CamelJBang.java",
"properties": {
"camel.jbang.version": "${version}",
"camel-kamelets.version": "${version}"
}
}
}
} |
|
@wfouche I don't know or follow at all what that code is trying to do. Please show explicit cases that you believe is failing with this PR? like: Given the following catalog: These aliases should resolve as this: a:1.0 -> ... |
|
@maxandersen , after our meeting today, I have the missing context that I needed to understand the reasoning behind the approach you follow in the PR. I'll test this and see how well it works, and provide feedback. |
|
Using the following jbang-catalog.json file {
"aliases" : {
"junit" : {
"script-ref" : "org.junit.platform:junit-platform-console-standalone:6.0.3",
"description" : "Launch the JUnit Platform from the console"
},
"cli" : {
"script-ref" : "org.junit.platform:junit-platform-console-standalone:6.0.3",
"description" : "Launch the JUnit Platform from the console"
}
}
}I ran commands: Versioning of GAVs works! However, this feels a bit unreal to me. :-) I'll test a script next. |
|
I've created three scripts: jbang-catalog.json {
"aliases" : {
"h": {
"script-ref": "./hello/${jbang.app.version:}/hello.java"
}
}
}Testing versioning functionality @maxandersen , it works very well. I like it. All that I found that is missing, is that JBang do not pass property Updated test program: ///usr/bin/env jbang "$0" "$@" ; exit $?
import java.lang.System;
public class hello {
public static void main(String... args) {
System.out.println("Hello World, 1.0.0");
String version = System.getProperty("jbang.app.version");
if (version != null) {
System.out.println("JBang App Version: " + version);
} else {
System.out.println("Property 'jbang.app.version' is not defined.");
}
}
}I also tested with The following alias could not be resolved: |
|
In summary, there are two issues to resolve:
With these two issues resolved, I think we'll have a great and useful versioning solution. |
Why/what is the usecase for the app to know what jbang.app.version was used on the command line? That is not really the business of jbang to tell an app what version it is ? There is zero guarantees that version relates to the application version. I get it could be nice to have but it is not something to rely and thus not a fan of exposing it.
I assume you mean this error: can you give me a repo/branch with what you have setup because this direct pointing to a jbang-catalog.json seems irrelevant...I assume its more about that the jbang.app.version is not replaced in case of no alias version specified? |
I've changed my mind. Don't set the property in the app JVM. |
https://github.com/wfouche/jbang/tree/dev/version-tests/tests {
"hw": {
"script-ref": "hello@./apps/hello/1.0.0/jbang-catalog.json"
},
"hw2": {
"script-ref": "hello@./apps/hello/${jbang.app.version:1.0.0}/jbang-catalog.json"
}
}Issue 1
|
|
Issue 2 "camel": {
"script-ref": "./CamelJBang.java"
}
|
|
'It is perfectly valid to have a script-ref for a versioned alias to be just "./CamelJBang.java".' I dont see how. That is not a versioned alias. So no way to apply version Alias to it. |
The "how" is that we just use the version number specified for the alias in jbang.app.version to find the correct //DEPS to load. ///usr/bin/env jbang "$0" "$@" ; exit $?
//JAVA 21+
//REPOS central=https://repo1.maven.org/maven2,apache-snapshot=https://repository.apache.org/content/groups/snapshots/
//DEPS org.apache.camel:camel-bom:${jbang.app.version:4.18.0}@pom
//DEPS org.apache.camel:camel-jbang-core:${jbang.app.version:4.18.0}
//DEPS org.apache.camel.kamelets:camel-kamelets:${jbang.app.version:4.18.0}
package main;
import org.apache.camel.dsl.jbang.core.commands.CamelJBangMain;
public class CamelJBang {
public static void main(String... args) {
CamelJBangMain.run(args);
}
}This does not work, but it should:
This works (simulating what we expect from alias versioning):
|
|
Ok that is not apps jvm environment. That's when the script it build into a jar. You'll need to pass the value via properties as you showed before. The app version is not automatically "inherited" |
|
I hope we can change this. We don't necessarily want a separate Java file for each version of an app. Below is how JBang support for Camel is defined today. How do you propose that this be changed so that
will work with a release of JBang that support alias versioning? https://camel.apache.org/manual/camel-jbang.html {
"catalogs": {},
"aliases": {
"camel": {
"script-ref": "dsl/camel-jbang/camel-jbang-main/dist/CamelJBang.java",
"description": "Run Apache Camel routes easily"
}
}
}///usr/bin/env jbang "$0" "$@" ; exit $?
//JAVA 21+
//REPOS central=https://repo1.maven.org/maven2,apache-snapshot=https://repository.apache.org/content/groups/snapshots/
//DEPS org.apache.camel:camel-bom:${camel.jbang.version:4.18.0}@pom
//DEPS org.apache.camel:camel-jbang-core:${camel.jbang.version:4.18.0}
//DEPS org.apache.camel.kamelets:camel-kamelets:${camel-kamelets.version:4.18.0}
package main;
import org.apache.camel.dsl.jbang.core.commands.CamelJBangMain;
/**
* Main to run CamelJBang
*/
public class CamelJBang {
public static void main(String... args) {
CamelJBangMain.run(args);
}
} |
|
This is the chicken-egg thing I talked about - the alias itself is not versioned its the "thing" the alias points to that needs to be versioned; and when you point to a file relatively in a main branch...you are implicitly saying "give me whatever is the latest". So you need to pick it you want whatever is latest or a specific named version. So your alias could point to: Do note that in the first one the version number is actually "camel-4.18.1" which might not be what you are after but that is the version name used. You can do But now you are having the main repo define an alias in the same place as what this alias points to - it is not that clean and im not seeing how/what we can change without making it harder for everything else. |
Agreed. The I don't understand why we don't view CamelJbang.java file as a "template" file to be used to create a version specific file to be compiled by javac. The alias script-ref points to "template" file CamelJBang.java which is parameterized with property jbang.app.version. Just update JBang set set property "jbang.app.version" in the JBang JVM (not the App JVM) and then the property will be available when "template" file CamelJBang.java is processed by JBang. The locking mechanism that you submitted in another PR I guess will be needed to ensure that apps installed in this way are not updated. |
|
It already is treated as a "template file" - it's done when you build. The thing is that I don't see a consistent way we can be ok applying version to script files without having a version context. That might be we need to such thing as default version but given i already showed you multiple ways to do it without it could you tell me why those are not applicable/sufficient ? |
|
My concern is that versioning Camel/JBang is a bit cumbersome, but it works. To support versions 4.17.0, 4.18.0 and 4.18.1, I need to create three distinct CamelJBang.java files: 
They are all the same except for the hard-coded version number they each contain. camel/4.17.0/CamelJBang.java ....
//DEPS org.apache.camel:camel-bom:4.17.0@pom
//DEPS org.apache.camel:camel-jbang-core:4.17.0
//DEPS org.apache.camel.kamelets:camel-kamelets:4.17.0
....camel/4.18.0/CamelJBang.java ....
//DEPS org.apache.camel:camel-bom:4.18.0@pom
//DEPS org.apache.camel:camel-jbang-core:4.18.0
//DEPS org.apache.camel.kamelets:camel-kamelets:4.18.0
....camel/4.18.1/CamelJBang.java ....
//DEPS org.apache.camel:camel-bom:4.18.1@pom
//DEPS org.apache.camel:camel-jbang-core:4.18.1
//DEPS org.apache.camel.kamelets:camel-kamelets:4.18.0
....jbang-catalog.json "camel": {
"script-ref": "./camel/${jbang.app.version:4.18.1}/CamelJBang.java"
}The 4.18.1 CamelJBang.java example shows that one version number is not always sufficient to meet alll //DEPS requirements. In this case camel-kamelets did not have a 4.18.1 version, and had to remain at 4.18.0 although the other camel JARs do use version 4.18.1. This is a convincing counter example, that shows that versioning as it is currently implemented provide the flexibility required to address issues like this. But it makes me wonder if one should not provide "hints" to the user of what "versions" are available. To that end a "versions" property that can be queried would be really useful. "camel": {
"script-ref": "./camel/${jbang.app.version:4.18.1}/CamelJBang.java",
"versions": {
"4.18.1": "Recommended stable version",
"4.17.0": "Previous stable version",
"4.9.0": "Legacy version"
}
}
Getting back to my opening sentence. No concerns anymore. |
|
The only remaining issue I'm aware of is fixing:
And then, optionally, adding a "versions" property. :-) |
you write as you create these in the same branch - but you don't have to; you already have that file in multiple versions in the tagged versions at https://github.com/apache/camel/commits/main/dsl/camel-jbang/camel-jbang-main/dist/CamelJBang.java If anything I would just make an alias and skip cameljbang.java completely. It is not doing anything besides calling a main method. |
I'm not a superfan, but I can see how it could be useful for querying/documentation. Open separate issue and lets see.
No, I don't see why you need to do that. Just use property replacements or different aliases. Your own suggested ways for it required these too. But again, as I said multiple times you are trying to do something I consider extremely niche of enabling multiple version support for something (i.e. CamelJBang) by putting a source file inside the same source repo of the thing you try to version. That is always going to be messy. |
what specifcially generated that error? |
"hw": {
"script-ref": "hello@./apps/hello/1.0.0/jbang-catalog.json"
},
"hw2": {
"script-ref": "hello@./apps/hello/${jbang.app.version:1.0.0}/jbang-catalog.json"
}Works
Fails
|
@maxandersen , this is the only issue that needs to be resolved before this feature can be released. :-) |
2 participants
Implements
alias:version@catalogsyntax to pin versions at invocation time without modifying catalog files.Try and fix with the syntax suggested in #1979
Examples:
Implementation
Version replacement cascade:
${jbang.app.version:default}) - takes precedenceAs in, if alias has property replacement we will NOT do automagic replacement because
can't know user really want the replacement .
group:artifact:version→group:artifact:newversion)alias@org/repo/ref→alias@org/repo/version)Did this with the thinking that if version replacement not possible its better we fail - but of course if jbang.app.version is included we 'll have to just trust the catalog writer.
For example:
{ "aliases": { "quarkus": { "script-ref": "io.quarkus:quarkus-cli-${jbang.app.version:3.0.0}:3.7.0" } } }Running
jbang quarkus:3.5.0resolves toio.quarkus:quarkus-cli-3.5.0:3.7.0Feedback Needed
Hard error vs warning: Currently throws
ExitExceptionwhen version cannot be applied to unknown URL patterns. Is this the right behavior (least surprise) or should it warn and continue?GAV detection heuristic: Uses colon counting (2+ colons = GAV) and dot detection in first segment (groupId pattern). Edge cases where this might fail?
Catalog reference version pinning: For registered catalog names (e.g.,
tool:1.0@mycatalogwheremycatalogis a named catalog), currently errors since we can't modify the catalog URL. Should this workdifferently?
Documentation
Added complete "Version Pinning" section to
docs/modules/ROOT/pages/alias_catalogs.adocwith syntax, examples, and all replacement strategies.