-
Notifications
You must be signed in to change notification settings - Fork 42
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
fix rockspec url #11
fix rockspec url #11
Conversation
Can I suggest you add another commit with the previous stable published rockspec version? Exactly how it is (broken or not) would be preferred. That just gives a way to diff from that to the current one to compare. Useful for PR review but also for later debugging other things. You can actually add as many back-releases of published rockspecs as you like, my personal preference is to keep all of them in the rockspecs folder for posterity. It does make it easier down the road to trace changes. But even if you don't do many of them, the last published one would be useful. |
maybe you missed it because it was a renaming but I did move from the main folder to "rockspecs/" the currently broken rockspec. Following up on your remark I've added the 6.1 version |
Yes I probably skimmed the diff too fast. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
And now we cross our fingers and see if this CI run ends in a successful publish to LuaRocks ;-) |
I've been waiting for this for the past few months. Crossing fingers indeed xD |
and it failed xD ! we dont see the command executed which is sad. I suppose it's a luarocks upload. Not sure why it needs a folder, is that for a rock ? what If I only care about rockspecs ? https://github.com/lunarmodules/lua-iconv/actions/runs/6442931263/job/17494299938#step:8:13 |
Nope. I forgot, the way it is setup right now to publish a stable version there also needs to be a matching tag in the Git repo. On the |
In order to verify that the rockspec actually works, it has to have the Git tag because without it the URL mentioned in the rockspec as where to download sources doesn't exist. |
I had already created the tag but before the merge so maybe it's missing some file. I'll let you do the honour this time as you know more what needs to be done. NB: I had created a manual v7.0.0 tag before this MR, feel free to remove/edit it |
I pushed the tag on your commit, github UI is just lying xD |
I see. The commit is mine and it has no author info on the tag itself so it just makes stuff up. Great. In any case 8e19fde has fixed your issue, see CI ran properly and LuaRocks.org now has a new release. |
I'll just note for the record that editing a rockspec like I just did only works before it successfully publishes. Once it is published to LuaRocks it should be considered immutable (future deploy CI jobs would fail anyway) and if something needs fixing it should be done with a new $ROCKREL bump. |
I was trying to run luarocks nix on lua-iconv but it still fails because the rockspecs urls point at a wrong uri:
And the uploaded rockspecs are absent from the rockspec folder too https://luarocks.org/modules/lunarmodules/lua-iconv/7-3 . I am not sure how to fix that properly, if you dont mind having a look |
@teto This appears to be a bug on LuaRocks.org itself and potentially a related one in the The src URL listed in the published lua-iconv-7.0.0-2.rockspec is https://github.com/lunarmodules/lua-iconv/archive/v7.0.0/lua-iconv-7.0.0.tar.gz which is correct and works. I was somewhat surprised to find I believe the real fix is to make sure LuaRocks.org and |
I checked out the source of |
I just pushed a |
Fixes #8