Skip to content
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

rockspec: use git+https:// for git repository URL #30

Closed
wants to merge 1 commit into from
Closed

rockspec: use git+https:// for git repository URL #30

wants to merge 1 commit into from

Conversation

Totktonada
Copy link

@Totktonada Totktonada commented Apr 3, 2022

GitHub disables unencrypted Git protocol, so git:// URLs stop working now (see https://github.blog/2021-09-01-improving-git-protocol-security-github/).

luarocks 3.8.0 autoconverts git:// URLs into git+https:// ones (see luarocks/luarocks@9ff512e), but there is plenty large community of users that still use one of previous luarocks version.

Since a distro package verifies a released rockspec with checksums, bumped revisions for them. Technically I had copied an X.Y.Z-1 rockspec into the X.Y.Z-2 one and apply a change like the following:

--- rockspecs/luacov-coveralls-0.2.3-1.rockspec
+++ rockspecs/luacov-coveralls-0.2.3-2.rockspec
@@ -1,7 +1,7 @@
 package = "LuaCov-coveralls"
-version = "0.2.3-1"
+version = "0.2.3-2"
 source = { 
-   url = "git://github.com/moteus/luacov-coveralls",
+   url = "git+https://github.com/moteus/luacov-coveralls",
    tag = "v0.2.3"
 }
 description = { 

The scm-0 rockspec is updated in place.

Totktonada added a commit to tarantool/crud that referenced this pull request Apr 4, 2022
GitHub had disabled insecure git clone (see [1]), so we should use
`git+https://` in the rockspec instead of `git://`. The fix is proposed
to the upstream in [2], but is not merged at the moment of writting
this. This patch workarounds the problem on our side.

Recent luarocks (3.8.0) autoreplaces `git://` with `git+https://`, but
we have older luarocks version shipped with tarantool. See [3] for
details.

[1]: https://github.blog/2021-09-01-improving-git-protocol-security-github/
[2]: moteus/luacov-coveralls#30
[3]: tarantool/tarantool#6597
Totktonada added a commit to tarantool/crud that referenced this pull request Apr 4, 2022
GitHub had disabled insecure git clone (see [1]), so we should use
`git+https://` in the rockspec instead of `git://`. The fix is proposed
to the upstream in [2], but is not merged at the moment of writting
this. This patch workarounds the problem on our side.

Recent luarocks (3.8.0) autoreplaces `git://` with `git+https://`, but
we have older luarocks version shipped with tarantool. See [3] for
details.

[1]: https://github.blog/2021-09-01-improving-git-protocol-security-github/
[2]: moteus/luacov-coveralls#30
[3]: tarantool/tarantool#6597
@alerque
Copy link
Contributor

alerque commented Apr 6, 2022

I don't think you can really bump all the historical rockspec releases without also bumping the rockrel, otherwise you will cause an issue for distro packages that verify them with checksums. The current DEV/SCM rockspec can be bumped (and force-pushed to LuaRocks) but in order to make old releases go smoothly with old LuaRocks (pre 3.8.0) I think the rockrels would need bumping.

GitHub disables unencrypted Git protocol, so `git://` URLs stop working
now (see [1]).

luarocks 3.8.0 autoconverts `git://` URLs into `git+https://` ones (see
[2]), but there is plenty large community of users that still use one
of previous luarocks version.

Since a distro package verifies a released rockspec with checksums,
bumped revisions for them. Technically I had copied an X.Y.Z-1 rockspec
into the X.Y.Z-2 one and apply a change like the following:

```diff
--- rockspecs/luacov-coveralls-0.2.3-1.rockspec
+++ rockspecs/luacov-coveralls-0.2.3-2.rockspec
@@ -1,7 +1,7 @@
 package = "LuaCov-coveralls"
-version = "0.2.3-1"
+version = "0.2.3-2"
 source = {
-   url = "git://github.com/moteus/luacov-coveralls",
+   url = "git+https://github.com/moteus/luacov-coveralls",
    tag = "v0.2.3"
 }
 description = {
```

The scm-0 rockspec is updated in place.

[1]: https://github.blog/2021-09-01-improving-git-protocol-security-github/
[2]: luarocks/luarocks@9ff512e
@Totktonada
Copy link
Author

@alerque Thank you for pointing it out! I copied X.Y.Z-1 rockspecs to X.Y.Z-2 and updated version and url (see the diff in the updated pull request description). Updated scm-0 in place.

I don't know, whether I should remove the X.Y.Z-1 rockspecs from the repository. I would appreciate a suggestion.

@alerque
Copy link
Contributor

alerque commented Apr 7, 2022

I don't know, whether I should remove the X.Y.Z-1 rockspecs from the repository. I would appreciate a suggestion.

That is totally up to the maintainer. I prefer to keep them around and I think it is a lot disruptive to the ecosystem in general not to have the links to them die. Consider the case of somebody with a recent LuaRocks (v3.8.0) that has a script installing a rockspec directly from a raw GitHub URL (there are some cases where this is better than sourcing it from LuaRocks, dodgy country firewalls being one of them). Having the rockspec link go away would be a disruption for them because they can still install the -1 rockrel fine since LuaRocks transparently fixes the URL. My method is to keep around al rockspecs that I consider "suppored" and only drop them if they are truly known permanently useless to anybody.

On the other hand some maintainers like to only keep the most recent one(s) on the grounds that they are available in Git history anyway. They have a point there too.

All that to say it's up to @moteus.

By the way @moteus if you are interested I did some work automating LuaRocks publish operations from Github when rockspecs are touched. You can see it in action on the luarcheck, luasocket, and luaexpat repositories—all of which were recently published to LuaRocks just by updating the rockspecs on GitHub. If you are interested in that being done here please feel free to open an issue an assign it to me and I'll work up a PR adapted for this repo.

@Totktonada
Copy link
Author

@alerque Thank you again for the detailed clarification!

Giving your inputs I consider this pull request ready, but if maintainers will ask for updates, I'm glad to follow up.

@moteus
Copy link
Owner

moteus commented Apr 12, 2022

Right now I have no ability to check any code, but i can accept PR, bump version and publish it.
I realy do not know about ricent LuaRocks changes

@Totktonada
Copy link
Author

I worked it around for me (tarantool/crud#270), but I think that the changes may be useful for other users. They're harmless: at least I performed the same for a lot of modules in tarantool (tarantool/tarantool#6587) and I didn't heard about any problem.

@alerque
Copy link
Contributor

alerque commented Apr 12, 2022

A merge/bump/push would be appreciated. I have been following this issue across many Lua projects and con vouch for this been both a necessary and correct fix. The real issues is GitHub deprecating plain Git access. The recent LuaRocks changes were just to make it easier to work around the limited GitHub access for people that have new LuaRocks, but for people with old versions of LuaRocks (such as all the Ubuntu folks) they need new rockspecs that specify the git+https access scheme otherwise they are just plain out in the cold.

@Totktonada
Copy link
Author

Any decision (positive or negative)?

@Totktonada
Copy link
Author

I see no interest from the maintainers and want to drop stale forks from my personal github namespace. Closing then.

If something will be changed, feel free to apply/use this patch as you wish.

@Totktonada Totktonada closed this Apr 19, 2023
@Totktonada Totktonada deleted the fix-git-url-in-rockspec branch April 19, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants