Allow disabling cache, some refinements to version selection

[Lord-Kamina]

This PR does a couple things.

1. It adds an input to allow disabling the cache step. The reason for this is because I intend to wrap this action with something I'm working on, and I'll implement the caching in there.

2. It removes the "version" parameter from the CI workflows, which is related to

3. The action gets the version information exclusively from the parameter file. But, if that is left blank, it instead defaults to the latest macports release. If a version is specified, much like using a different prefix, macports will be built from source instead of using an installer package. The reason for this is simple: The package runs selfupdate as a postflight script. If somebody goes to the trouble of specifying a particular version, I assume it's because for some reason they don't want the latest version and thus presumably, they wouldn't want it to update automatically. (Don't really know why would someone want that, though)

[foretspaisibles]

It looks great, thanks! I'll review that today.

[foretspaisibles]

HI @Lord-Kamina thank you for your interest in setup-macports and sharing your code to make it better and more useful!

I recommended to make a couple of changes to make the code easier to maintain and more robust, tell me what you think about it.

It was quite a long time, If your interest moved to some other questions I could also do these changes by myself. Thanks!

PS Please also consider describing the changes to the version behavior to the README. Pinning the version explicitly should be the recommended approach but mentioning the ability to track LATEST could be useful.

Maybe warning the users they are using an obsolete version of macports could be useful too?

[foretspaisibles]

Rather define two functions:

macports_release_url()
{
  curl https://raw.githubusercontent.com/macports/macports-base/master/config/RELEASE_URL\
  || curl https://trac.macports.org/export/HEAD/macports-base/config/RELEASE_URL\
  || curl https://distfiles.macports.org/MacPorts/RELEASE_URL
}

output_latest_version()
{
   awk -F'/v' '{ printf("latest_version=%s\n", $NF) }'
}

macports_release_url | output_latest_version >> ${GITHUB_OUTPUT}

Advantages: less expansions and variables, functions are easy to test/mock.

Common recommendation is to avoid ECHO with variables text, it's super brittle. Generally use printf(1) instead but if another tool like awk is used, why not print within awk.

[foretspaisibles]

Have you considered moving his check to a separated workflow that runs periodically (daily?) and opens an issue if a new version is out?

[Lord-Kamina]

Pinning the version explicitly should be the recommended approach

I strongly disagree with this, considering it's exceedinly rare for macports updates to have breaking changes.

[foretspaisibles]

That seems rather convoluted. What is the reason for this? Why not just ${{ inputs.enable-cache }} since the value is a boolean?

[Lord-Kamina]

Because I think that boolean inputs are actually parsed as string, instead of real booleans.

[foretspaisibles]

export adds variables to a list that is transmitted to subprocesses, it's useful to pass parameters to subprocesses but I do not think this is your intent here.

[foretspaisibles]

The empty string does not really convey the idea that we are going to use the latest available version. What about using the string LATEST instead ?

[foretspaisibles]

You do not need to do that here. Add the version selection logic to the action itself and add the detail you're interested in (macports_version) to the action output. It will simplify your design.

[foretspaisibles]

Sourcing files that have an effect beyond defining yields very confusing programs. I would recommend to move your logic to the configure_macports file and add the configuration details to the output of the action. For the logic maybe

leave if the if [ -f "${parameterfile}" ] untouched and add another block

if [ "${macports_version}" = 'LATEST' ]; then
  macports_version="$(determine_latest_macports_version)"
fi

where you define the determine_latest_macports_version with your curl/awk combo.

[foretspaisibles]

Attention to the intent conveyed by quotes. Double quotes are for interpolation and single quotes for literal expressions. It's a bit weird to write "true" rather than true here.

[foretspaisibles]

I would rather approach this by moving the logic provided by check_version.sh to this file, be sure to define functions printing the desired values instead of storing the values in variables. Continue to use macports.sh everywhere you need your functions.