Tweak rockspec to pass CI again #444
Conversation
|
Note this project isn't (yet) using our reusable workflows for rockspec testing hence the failures on 3.1.3 aren't showing up here, I'm just copying this from lessons learned testing other projects. If you want I want I can bring in the rockspec testing and auto-publish stuff to this project too. |
|
If you don't like the coding style I just noticed copas has a different workaround: https://github.com/lunarmodules/copas/blob/master/copas-cvs-6.rockspec#L12-L13 Slightly more verbose but also easier to grok. |
This is the one I use these days, so this definitely has my preference |
|
also I tried to switch to "dev" rockspecs a while ago, because it seemed the more obvious name. But it turned out it gave me some issues with LuaRocks, it seems not all labels are treated equal; dev/scm/cvs. Maybe @hishamhm has some background on this? |
|
I'm confused on the And sure I'll tweak the rockspec code styling. |
|
Also (not talking to you specifically just the ecosystem in general) the practice of rockrel bumping makes no sense to me on dev/scm/cvs rockspecs, I wish people would stop doing that. |
alerque
force-pushed
the
ci
branch
2 times, most recently
from
46d03df
to
2a09798
Compare
|
As long as I was at it using a more readable workaround, I tweaked the whole thing a bit for what I consider better readability. Let me know if you don't like the style changes. |
|
As long as we are mucking with rockspecs we might as well test them. I added our modular rockspec build checks. We'll have to check results in my fork until after the PR merge I think. I did not include the automatic deploy part that publishes rockspecs after checking them, just the test build section. |
Setting source.branch works for most cases, but when the branch is actually a tag and not a branch then a Luarocks 3.1.3 bug rears and tries to concatenate a nil. This dodges that bullet by setting source.tag explicitly to not trigger the branch_or_tag concatenation. All the other changes are cosmetic and for readability/maintainability.
|
Rockspec build checks ran here. |
I actually thought about creating a GHA that actually checks and barfs if you do 😄 |
| local github_repo_name = package_name | ||
| local git_checkout = package_version == "dev" and "master" or package_version | ||
|
|
||
| package = "penlight" |
There was a problem hiding this comment.
I prefer the vertical alignment of the "local"s, aesthetically. Despite this is just copying one variable into another.
(hence also the 2 lines white-space below to accentuate the vertical alignment above)
|
|
||
| rockspec_format = "3.0" | ||
| package = package_name | ||
| version = package_version .. "-" .. rockspec_revision | ||
| version = ("%s-%s"):format(rock_version, rock_release) |
There was a problem hiding this comment.
I think the concatenations are easier to read for non-Lua folks than the format-placeholder approach.
| branch = rock_version == "dev" and "master" or nil, | ||
| tag = rock_version ~= "dev" and rock_version or nil, |
There was a problem hiding this comment.
this I like, I have switched other rockspecs to this as well. Wondering if it is worthwhile to remove the hardcoded "master" here and add a local at the top?
local default_branch = "master"
They're almost equal; In the presence of both in a server, The background story is that in the olden CVS days they started as TL;DR: just use |
A while back @leafo fixed some stuff in the luarocks GH Action we are using to setup containers to test our rockspec builds. With those fixes in place I've been able to re-enable the matrix that checks our builds against multiple
luarocksversions. For a long time we had only been testing 3.8.0, now I'm doing an assortment of the newest, the one found on Ubuntu LTS, and the oldest supported.This has turned up the fact that the checkout branch declaration we use was broken in LuaRocks 3.1.3 due to an upstream bug. It doesn't play nice with automatically figuring out whether something is a tag or a branch and ends up trying to concatenate a string to nil. This was fixed in Luarocks, but that version has quite a bit of market penetration so there is some sense in testing with it.
This uses an uglier but effective workaround to get the same work done without leaving 3.1.3 in the cold.