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

Fix Description of -J #199

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ijrsvt
Copy link

@ijrsvt ijrsvt commented Jan 26, 2024

The -J parameter of sjsonnet appears to prefer **left-hand** files over right-hand files. This change in behavior happened between 0.3.0and0.3.1`, so I think it makes sense to change the documentation, not the behavior.[1]

I think that the change happened in #107, but I'm not 100% sure if I'm reading the Scala correctly. The previous implementation seems to prepend values to the jpath variable [2].

[1]:
Create the following files:

/tmp/a/file.jsonnet {value: "a"}
/tmp/b/file.jsonnet {value: "b"}
/tmp/c/file.jsonnet {value: "c"}

and ~/dumb.jsonnet:

local file = import "file.jsonnet";

{
whoami : file.value
}

Run the following commands

curl -L https://github.com/databricks/sjsonnet/releases/download/0.3.0/sjsonnet.jar  > /tmp/sjsonnet30.jar; chmod +x /tmp/sjsonnet30.jar
curl -L https://github.com/databricks/sjsonnet/releases/download/0.3.1/sjsonnet.jar  > /tmp/sjsonnet31.jar; chmod +x /tmp/sjsonnet31.jar

$ /tmp/sjsonnet31.jar /home/ian.rodney/dumb.jsonnet -J /tmp/c/ -J /tmp/a/ -J /tmp/b/ 
{
   "whoami": "c"
}
$ /tmp/sjsonnet30.jar /home/ian.rodney/dumb.jsonnet -J /tmp/c/ -J /tmp/a/ -J /tmp/b/ 
{
   "whoami": "b"
}

[2]:

    Arg[Config, String](
      "jpath", Some('J'),
      "Specify an additional library search dir (right-most wins)",
      (c, v) => c.copy(jpaths = v :: c.jpaths)
    ),

@ianrodney-db
Copy link

Hey @lihaoyi-databricks Could you PTAL at this?

@lihaoyi-databricks
Copy link
Contributor

@carl-db do you think you can help take a look at this? The main question is whether to update the docs to align with the current behavior, or update the current behavior to align with the docs (and I presume the upstream implementation)

IMO updating the behavior to match the docs/upstream is the correct thing to do, but that may have some migration cost/risk, so it's something you guys should probably make the decision for since you own our jsonnet tooling overall

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.

3 participants