apply beta versioning (git shorthash) to github action builds

[wagyourtail]

baritone version with git describe --always --tags | cut -c2- on github action builds great for users who report issues on unreleased versions as we can see exactly what commit they were using

[leijurv]

Oopsie misclicked on my phone

[leijurv]

This is cursed, why do we need Javascript to parse a gradle properties? Frankly it might be better to always include the commit hash, then manually remove it when making a release, that way it's never ambiguous which download jars are accurate and official

[wagyourtail]

This is cursed, why do we need Javascript to parse a gradle properties?

they do also run shell scripts, I guess I could've used sed, but then version bumping is hard

[wagyourtail]

Frankly it might be better to always include the commit hash, then manually remove it when making a release, that way it's never ambiguous which download jars are accurate and official

yes that's what this does,

unless you also mean when people manually build it themselves, that's harder

[leijurv]

Idk I really don't like this approach, using Javascript in a github action to rewrite the gradle.properties file makes my skin crawl. :( I think this logic would better belong in https://github.com/cabaletta/baritone/blob/master/buildSrc/src/main/java/baritone/gradle/task/CreateDistTask.java

yes that's what this does

Wait then what's the point of if (process.env.GITHUB_REF.includes("tags")) return e;? It seems to me like the intent here is to have the current naming scheme only when the current build is a tag, which is not the same as manually removing the hexadecimal when it's time to make a release?

[ZacSharp]

Assuming git is installed on the CI I'd suggest using the output of git describe --always --tags (possibly without the leading "v") as the version. It also automatically omits the hash when there is a tag on the current commit.

Also I'd prefer having this version scheme for local builds as well, preferably with something like the --dirty flag of git describe. However that would require a fallback in case git is not installed (e.g. because someone used githubs download as zip option to avoid installing + using git)

Even if you keep the current behavior, please at least pass the value as a -Pversion_suffix=<shorthash> argument instead of rewriting the config file.

[wagyourtail]

Even if you keep the current behavior, please at least pass the value as a -Pversion_suffix=<shorthash> argument instead of rewriting the config file.

good idea, I'm gonna have to steal that for my CI's as well xd

[wagyourtail]

@ZacSharp I've implemented your suggestion in a way I think will work good, ~~doing it to local builds would be too complicated, as it would involve running a shell command inside build.gradle, so instead I just gave it a static flag that it was built without the CI~~

[leijurv]

Looking at the CI https://github.com/cabaletta/baritone/actions/runs/2437026172, I see

What does the -159- mean?

[leijurv]

Also the commit hash in question started with 2b4da2a, why does it put g665f1d1d instead?

[ZacSharp]

The -159- means there are 159 commits between the current commit and v1.2.5 and the commit hash starts with a "g" because git puts that in for git. The reason why it uses 665f1d1d rather than 2b4da2a is a little more interesting. When looking it up at github.com/cabaletta/baritone/commit/<hash> you can see it is a merge commit attributed to wagyourtail merging this pr into master. If you run git fetch upstream refs/pull/3486/merge && git show --format=fuller FETCH_HEAD you can see that that's the same commit and that the actual committer is Github so I guess the reason is that Github always merges the pr and then runs the CI on that commit.

This will happen with any approach unless we explicitly check for being on a refs/pull/*/merge ref and in that case use the second parent instead.

[wagyourtail]

This will happen with any approach unless we explicitly check for being on a refs/pull/*/merge ref and in that case use the second parent instead.

do you know what the command is for that?

[ZacSharp]

Had already written a whole lot of text about bad solutions (git fetch origin refs/pull/*/merge:refs/heads/pull/* + git describe --all --exact-match --match=pull/* HEAD or git ls-remote upstream refs/pull/*/merge + manual search) but then found this, from which I learnt that we should be able to just pass ${{ github.event.pull_request.head.sha }} or ${{ github.event.pull_request.merge.sha }}^2 to git describe and be done (though we might want to include ${{ github.event.pull_request.merge.sha }}^1 as well because it also contributes code to the build).

EDIT: this does work with push and release events as well, but in a hacky way (the context variable evaluates to an empty string in those cases and omitting the argument from git describe makes it use HEAD, which happens to be the desired ref)

Reason I was looking there is because I'm searching for a way to use git clone --filter=tree:0 to only clone commits and tags without any trees/blobs we don't access (almost like before adding fetch-depth: 0).

[ZacSharp]

This will happen with any approach unless we explicitly check for being on a refs/pull/*/merge ref and in that case use the second parent instead.

do you know what the command is for that?

Kinda.

Got the versioning to work with either

...
    steps:
      ....
    - name: Determine version (pull request)
      if: github.event_name == 'pull_request'
      run: echo version="$(git describe --always --tags ${{github.event.pull_request.head.sha}})+$(git describe --always --tags ${{github.event.pull_request.base.sha}})" >> $GITHUB_ENV

    - name: Determine version (other)
      if: github.event_name != 'pull_request'
      run: echo version="$(git describe --always --tags ${{github.sha}})" >> $GITHUB_ENV

    - run: echo ${{env.version}}

or

job:
  ....
    env:
      ref: ${{github.sha}}

    steps:
      ....
    - name: Use pull request head for versioning
      if: github.event_name == 'pull_request'
      run: echo ref="${{github.event.pull_request.head.sha}}" >> $GITHUB_ENV

    - run: echo $(git describe --always --tags ${{env.ref}})

(cut -c2- missing)

Getting this to work with prs and pushes the seems to cause some complication so I wonder whether we might want to go for the improper solution and just use git describe --always --tags ${{github.event.pull_request.head.sha}}, which works for prs and silently defaults to what happens to be correct (HEAD) for non-pr runs.

I also got the limited clone with history to work with

....
    steps:
    - uses: actions/checkout@v2

    - name: Fetch history
      run: git fetch --unshallow --filter=tree:0 origin $GITHUB_REF

    - name: Fetch tags
      run: git fetch origin +refs/tags/*:refs/tags/*

[wagyourtail]

@ZacSharp I've updated the pr per your suggestions, made some modifications to make it a bit nicer. thoughts?

[ZacSharp]

This doesn't show from which commit of a pr the artifact was generated, which would be of interest e.g. in case a frequently used pr like the one for 1.19.2 needs a bugfix.

I messed up with the fetching (sorta) and it fetches nearly all commits; but without contents so it's still not a heavy full clone. As of now I neither have a solution to this nor do I think it is important.

All tests I did (push and pr with and without available tag) worked, so apart from not showing the pr hash this looks good.

[wagyourtail]

looks right to me

[ZacSharp]

Oh, I though it was target commit plus pr number, but it's pr commit plus pr number. That's way better then.

[ZacSharp]

Did you actually test this without git installed? If I add "doesntexist".execute() to build.gradle the build fails with Cannot run program "doesntexist": error=2, No such file or directory

[wagyourtail]

honestly. if people don't have git, I feel like that's a "not our problem" kinda thing probably fixable with a try/catch tho; git's too essential to my system (linux) so I can't test that...

[ZacSharp]

I guess replacing the command for testing has to do the job then.

[wagyourtail]

yeah. that works I guess. fixed

[wagyourtail]

git inserts a v at the beginning of the tags. that's what the cut -c2- removes the v check is there in case git fails for being installed but not in a git directory (download zip, but with git installed), in which case the output is different

[ZacSharp]

That's fine as long as we can be sure the CI will only ever find version tags. Otherwise it will e.g. turn tag "refactor" into version "efactor". Just checked the manpage and found --match, so we can just filter the wrong tags out. As a bonus this will prevent our hypothetical "refactor" tag from occluding the prefious "v1.*.*" tag like it currently would.

[wagyourtail]

so, which command do I add what to?

[ZacSharp]

Sorry, my bad. I checked the manpage of git describe and found that we can add --match 'v*' to the git describe commands both in the buildscript and the workflow so only valid version tags are considered in the first place. That way the check in the buildscript won't exclude any tags the CI would use. Also if we have a non-version tag, do we want to ignore it or was that just a workaround? The output without a repo seems to be empty (the stuff that shows up in the terminal is from stderr)