Cleanup how to override SBT + make help documentation update

[abejgonzalez]

Related issue:

Type of change: other enhancement

Impact: other

Release Notes
ENABLE_SBT_THIN_CLIENT flag is best used with a pre-installed SBT (aka best with the SBT script). This allows you to just override the SBT binary/script with SBT_BIN and have the --client flag be handled by Chipyard's Makefile. This also adds more make help documentation, fixes an issue with the SBT server not being depended on, and removes unnecessary ?=s and overrides in variables.mk (this was done to properly enforce that the only way to modify the CY build system SBT is to change SBT_BIN).

Side note: When using the SBT script to launch things, the thin client acts nicely; properly exits on errors, displays colors, etc.

[jerryz123]

Should SBT_OPTS be part of SBT_BIN or SBT? I would expect SBT_OPTS to be the same for various settings of SBT_BIN

[jerryz123]

Hmm this also drops JAVA_TOOL_OPTIONS when SBT_BIN is custom set. Is that intended?

[abejgonzalez]

When using the SBT script the variables in the SBT_OPTS makefile variable are passed to the script in two ways:

1. the .sbtopts file in the top-level
2. the env. variable SBT_OPTS (which is assumed to be set by users)

As for JAVA_TOOL_OPTIONS, I need to double-check... I think a user would need to have it as an environment variable for it to work with the custom SBT thing to begin with.

[abejgonzalez]

I wonder if Chipyard should just automatically download SBT (its just a .jar and a bash script) and use it by default... then we wouldn't have to have all this makefile stuff around it. The downside is that we would have 2 sbt-launch.jar and we would not use the default SBT installation. Another option is to just enforce that SBT is installed in the pre-reqs script.

[abejgonzalez]

Ok. Cleaned this up a bit but I can confirm that doing export var exports the variable to the recipe. Therefore, JAVA_TOOL_OPTIONS is passed to java no matter what.

[jerryz123]

Can we add some help text here for SBT_BIN?

[tymcauley]

So, the recommended usage of thin-client mode would be:

ENABLE_SBT_THIN_CLIENT=1 SBT_BIN=sbt make ...

Should we automatically set SBT_BIN to sbt when ENABLE_SBT_THIN_CLIENT is set?

Also, is there any downside to always using SBT_BIN=sbt as the default case, and just never using sbt-launch.jar? On one hand it means people need an extra binary on their PATH (sbt rather than just java), but I think that all of the Chipyard setup docs ensure that people have sbt installed.

[abejgonzalez]

I think outside of me, you (@tymcauley) and Jerry are the two main users (at least the two main ones who I know about) so I would love to hear your feedback as always.

So, the recommended usage of thin-client mode would be:

ENABLE_SBT_THIN_CLIENT=1 SBT_BIN=sbt make ...

With that being said, I have both ENABLE_SBT_THIN_CLIENT and SBT_BIN=sbt set in my .bashrc. You could also set it the way that you specified.

Should we automatically set SBT_BIN to sbt when ENABLE_SBT_THIN_CLIENT is set?
Also, is there any downside to always using SBT_BIN=sbt as the default case, and just never using sbt-launch.jar? On one hand it means people need an extra binary on their PATH (sbt rather than just java), but I think that all of the Chipyard setup docs ensure that people have sbt installed.

I have this debate every so often. While the installed sbt (which really just is just a bash script) could be used instead of the sbt-launch.jar, it would add one more thing for Chipyard to version against. My concern is mainly around... what version of the sbt script is being used, does it support --client, and how well does this script interact with a different version of sbt specified here. If I have clearer answers to those concerns then I would be more than happy to switch (since it would "freeze" how you might set sbt/java args).

[jerryz123]

If we make SBT_BIN=sbt the default case, we'd need to update all the setup instructions to make sure the user gets the right SBT on their PATH. While this is fine, I'd prefer that to go into a separate PR later, as we should dogfood this feature a bit more before changing default behavior.

[tymcauley]

Regarding the sbt version, I don't really know what the minimum-supported-version should be, but as far as I can tell, there are two sbt versions to care about:

• The sbt Java binary, which is what actually runs the build. The version of this is set in project/build.properties.
• The version of the launcher script. This is the version of the sbt executable that you have installed on your machine. This is basically a shell script which downloads the desired sbt Java binary, which must match the version in project/build.properties.

This was the most useful link I could find on that subject: sbt/sbt#4503 (comment)

I don't know what the relationship looks like between these two programs and their supported versions. Not sure where to find that.

Regarding SBT_OPTS, it looks to me like we're now depending on users to copy the settings from JAVA_TOOL_OPTIONS into SBT_OPTS when people set SBT_BIN=sbt, is that right?

I also like the idea of addressing setup/usage for this use-case in a future PR. I'm very happy to see that using SBT_BIN=sbt will now properly communicate the exit code of the sbt server task 🙂 Seems like this should make it a more broadly-useful feature now.

[abejgonzalez]

Regarding the sbt version, I don't really know what the minimum-supported-version should be, but as far as I can tell, there are two sbt versions to care about:

• The sbt Java binary, which is what actually runs the build. The version of this is set in project/build.properties.
• The version of the launcher script. This is the version of the sbt executable that you have installed on your machine. This is basically a shell script which downloads the desired sbt Java binary, which must match the version in project/build.properties.

This was the most useful link I could find on that subject: sbt/sbt#4503 (comment)

I don't know what the relationship looks like between these two programs and their supported versions. Not sure where to find that.

Yup. Looks like we are referencing the same thing and having two versions to track (the script vs the build.properties file).

Regarding SBT_OPTS, it looks to me like we're now depending on users to copy the settings from JAVA_TOOL_OPTIONS into SBT_OPTS when people set SBT_BIN=sbt, is that right?

The JAVA_TOOL_OPTIONS is exported in the Makefile so it should be automatically passed as an environment variable to the sbt script (and thus to the underlying java bin). So my experience, no... you shouldn't have to change anything (you still set JAVA_TOOL_OPTIONS and SBT_OPTS normally).

I also like the idea of addressing setup/usage for this use-case in a future PR. I'm very happy to see that using SBT_BIN=sbt will now properly communicate the exit code of the sbt server task 🙂 Seems like this should make it a more broadly-useful feature now.

Yup, I plan on using this (virtually) all the time. Although there still seems to be issues with nested SBT calls that @davidbiancolin noticed (frequently used in FireSim-land)