-
Notifications
You must be signed in to change notification settings - Fork 41
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
Replace remaining third party actions with native Github CLI commands + Implement idea from #233 + Automate Kron4ek wine builds pulling + Various improvements #234
Conversation
koplo199
commented
Mar 27, 2023
•
edited
Loading
edited
- Replace remaining third party actions with native Github CLI commands
- Fixes Improved naming of new component updates? #233
- Automate Kron4ek wine builds pulling
- Various improvements (dependencies caching, etc)
This is awesome and will be a huge help for managing bottle components! Bottles is blessed to have you on the team! :) if [ "$channel" = "unstable" ] && [ "$latest_channel" = "unstable" ]; then
if [[ $latest =~ .*-([[:digit:]])-[[:alnum:]]{7} ]]; then
revision="${BASH_REMATCH[1]}"
commit_sha=${component_name: -7}
((updated_revision=$revision+1))
component_name=${component_name/-$revision-$commit_sha/-$updated_revision-$commit_sha}
echo "NAME=$component_name" >> $GITHUB_ENV
fi
fi This keeps rolling forever, right? So the revisions would look like this:
It won't really matter but is there a way to handle restarting the sequence whenever the upstream project changes their latest project tag? Meaning we restart counting revisions at Edit: Upon closer look, it seems like maybe earlier parts of the code are handling the initial setting of the |
I'm not on the team, I just like to contribute to Bottles but thank you ^^
This is exactly what is happening :) First, there is an environment variable which set the component name following the stable naming scheme:
(with dxvk- being the name-prefix, and 2.1 the pulled version tag, eventually the name is also transformed to lowercase)
If a stable update is found, then check for experimental/unstable builds are ignored. If not, the environment variable is updated with the experimental naming scheme:
Well, hopefully you did because it allowed me to see and fix a mistake, the line: component_name=${component_name/-$revision-$commit_sha/-$updated_revision-$commit_sha} Should be in fact: component_name=${component_name/-1-$commit_sha/-$updated_revision-$commit_sha} ;) |
Oh! Thanks for the explanation, and nice catch about the -1- pattern! Edit: I incorrectly thought there was an error in the new code, but realized now that the It seems to be doing exactly that pattern in the code you quoted for "unstable builds". So the naming seems to be
In that case your new code is definitely correct. Awesome. :) |
Btw, what happens in this scenario?
I think then it would name the first version of Whether it breaks or not depends on whether Bottles is guaranteed to always make a |
Yes, this is what happens: if a stable update is detected, then the CI does not check for unstable builds (which makes sense). Concretely this is achieved using the components/.github/workflows/pull-components.yml Lines 114 to 118 in 99e3f51
Which is then checked later in the workflow at multiple occasions, for instance when deciding whether to check or not for experimental builds: components/.github/workflows/pull-components.yml Lines 139 to 141 in 99e3f51
That way, the scenario you described in your last comment is guaranteed to never happen. |
@koplo199 Perfect perfect. One more potential pitfall to ask about: When the CI names the first "git revision" ( |
I'm not on the team either, but I feel this PR is a bit overloaded. There are two independent things going on here, the improvements to CI, and then the addition of the Kron4ek builds. My comments above only refer to the last commit: Automate Kron4ek wine builds pulling |
It feels overloaded because it probably is. But it's too late now ¯\(ツ)/¯ |
4da79c0
to
955d35b
Compare
@Kinsteen Rebase done, it's ready for merge now (it includes the |