Support retrieving latest version tag also when pinning action

[netomi]

This fixes #67 .

Right now, the action can not be pinned, this PR fixes that so that the referenced tag can also be determined from a commit hash.

[rossjrw]

Removing the --single-branch optimisation: yep, this is pretty non-negotiable if we want to able to version this with a SHA. It's gotta go. Thanks for the contribution!

Changing the tag finding mechanism: at first glance I don't like this new approach using git tag --points-at. I can find a couple of ways to break it already, both of which are pertinent when considering the use case of pinning a specific version, tagged or untagged, in a forked version of the action:

• The SHA refers to an untagged commit made since the most recent release: the command won't output anything.
• The SHA refers to a commit with multiple tags: the command will output all tags. This isn't a problem right now with the v*.*.* filter but could be problem if I, or a future forker, make a mistake and accidentally tag a commit twice. git describe --tags will only ever output one thing.

I don't think the git describe --tags solution would cause an issue when a SHA is checked out - might look a bit weird (v1.4.4-4-g2150c59 for current main) but isn't broken. Is there a problem I've missed?

[netomi]

I am open to make this more resilient, the PR so far was my attempt to make it work with a commit hash, but I will invest more time into it.

[rossjrw]

Ah! I think I understand the reason for the change. When you clone the entire bare repository you don't have anything checked out, so git describe doesn't know to describe the SHA you specified.

git describe can take a commit parameter, so I think this can be fixed by changing:

action_version=$(git describe --tags --match "v*.*.*" || git describe --tags || git rev-parse HEAD)

to

action_version=$(git describe --tags --match "v*.*.*" "$git_ref" || git describe --tags "$git_ref" || git rev-parse HEAD)

Does that seem reasonable to you?

[netomi]

yes that is exactly the reason for the change, but I will test if git describe with the ref leads to the same result as my snippet.

[netomi]

ok tested that with various refs and the result is basically the same, so that looks reasonable to me, will update the PR.

[netomi]

updated the PR.

btw. you might wanna look at https://github.com/eclipse-langium/langium-website/blob/main/.github/workflows/preview.yml to support that the comments can also be added to the PR for PRs coming from forks.

It uses the pull_request_target trigger to be able to issue write tokens and deploy the site to a separate repo using a fine-grained token that can only access that repo. Thats a reasonable compromise imho together with approval for workflows running for any PR from outside collaborators.

[rossjrw]

Thanks @netomi, will merge this later today.

That permissions setup does seem like a reasonable compromise. Will be worth me documenting it as a possible solution for #3

[rossjrw]

Released in v1.4.6

[netomi]

That permissions setup does seem like a reasonable compromise. Will be worth me documenting it as a possible solution for #3

I looked at the issue and the proposed solution with workflow_call which I believe is technically sound, however there is still the problem where to push the built preview site to for deployment. Pushing it to the same repo requires write permissions for content and I feel uneasy to grant that to any workflow that would run for PRs coming from forks. So the solution with a dedicated preview repo is a solution to that. Maybe doing the comment with a workflow_call would be useful, but I would rather always do the deployment to a separate repo.

fyi. the deploy-pages action is working on a preview feature: https://github.com/actions/deploy-pages/tree/b580d214b4e13b2a70d0e04376a86ed862ebb558#inputs-

which is currently in closed alpha

[rossjrw]

Agreed re deploying to a separate repo. Fundamentally this entire action is a bad idea and I don't think it can ever be truly secure - but it is convenient. I do think that when it gets as far as needing an entire separate repo just for deployments, at that point you might as well use a dedicated deployment provider, which is why I'll recommend solutions like Netlify and Vercel at the drop of a hat (despite never having actually used either myself).

fyi. the deploy-pages action is working on a preview feature: https://github.com/actions/deploy-pages/tree/b580d214b4e13b2a70d0e04376a86ed862ebb558#inputs-

I've seen. There's some discussion about integration with that action here #21, and about the alpha 'preview' feature specifically in this comment #21 (comment)

The gist of my opinion is that there's no indication of whether that feature is being worked on, if it will be worked on, or even a development roadmap. Based on actions/deploy-pages#61 and actions/deploy-pages#180 there have been no updates since September 2022, so I have no expectations that the feature will ever be generally available. I'm not going to pin any hopes on it until I hear otherwise.

[netomi]

Indeed, this action is a convenient by simple way to support previews. I have used netlify as well and it works like a charm, but not everybody is willing to use it for different reasons. I am confident that this action will remain useful regardless on the progress wrt the preview feature in the deploy-pages action.