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

Restoring lua tests #10777

Closed
tobil4sk opened this issue Aug 19, 2022 · 4 comments · Fixed by #10916
Closed

Restoring lua tests #10777

tobil4sk opened this issue Aug 19, 2022 · 4 comments · Fixed by #10916

Comments

@tobil4sk
Copy link
Member

Seems like we forgot to fix the lua tests after "temporarily disabling them" in 8027d74

We discussed this on slack a long time ago, but we need to update: https://github.com/jdonaldson/haxe-deps
as the git: protocol is no longer supported (which is used in the source url). I think it can just be updated to https.

@jdonaldson I made a PR for this, would you be able to handle making a release? HaxeFoundation/haxe-deps#2

Also, it is worth considering adding luasec to the file, because it is now a potential dependency since: #10593. This haxe-deps project seems to be used to install all potential dependencies, even if they are not necessarily used in all haxe lua projects, so it makes sense to add it. Right now we have to install it manually for the tests:

installLib("haxe-deps", "0.0.1-6");
installLib("luasec", "1.0.2-1");

We should also probably mention haxe-deps in the manual, but that is an issue for that repository.

@Simn
Copy link
Member

Simn commented Nov 9, 2022

Looks like we'll have to fork haxe-deps and handle this ourselves. I'm not sure what "making a release" entails here exactly, are we blocked from something if Justin doesn't handle it?

@tobil4sk
Copy link
Member Author

tobil4sk commented Nov 9, 2022

I think it has to be uploaded here: https://luarocks.org/modules/jdonaldson/haxe-deps

@inklit
Copy link
Contributor

inklit commented Jan 4, 2023

FYI, I attempted to fork haxe-deps and update it with my fork in my PR #10916 but found that simply changing the protocol to https did not seem to work, and after fiddling with it for too long I decided to just remove this dependency and manually reference the dependencies for the unit tests.

@tobil4sk
Copy link
Member Author

I decided to just remove this dependency

@inklit I think it's important to have (and use) this package, as it pins down all the lua dependencies required to run the lua target in a single location. Otherwise, we have to manually keep the unit tests and documentation in sync, which is error prone and bound to cause issues. So I think it's worth fixing this package and adding it to the documentation.

Looks like we have to use git+https, not just https: luarocks/luarocks#1408 (comment). It looks like luarocks 3.8+ autoconverts the git protocol to fix the errors, but since we are still dealing with older versions of luarocks we should probably just fix it properly. I've updated my PR in the haxe-deps repository to fix the protocols.

We should transfer the haxe-deps repository to HaxeFoundation (or make a fork) and figure out how we can add a new author to the luarocks package.

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 a pull request may close this issue.

3 participants