search

Raku / whateverable

🤖 Different IRC bots that operate on a bunch of prebuilt Rakudo versions
https://gist.github.com/Whateverable
GNU Affero General Public License v3.0
18 stars 14 forks source link

Sourceable support #369

Closed Altai-man closed 4 years ago

Altai-man commented 4 years ago

First take on attempt to adapt code from https://github.com/zoffixznet/perl6-sourceable/blob/master/lib/Sourceable/Plugin/Sourcery.pm6 ~I am not really sure the licensing allows this.~

So my questions are: 1)How do I even test this if I don't want to setup a box for hosting bots? 2)What else is necessary?

AlexDaniel commented 4 years ago

~I am not really sure the licensing allows this.~

Can you clarify why this is crossed out? I don't see any license in the main repo, meaning that we have to ask Zoffix for their permission. That said, after applying tweaks suggested below, I think there won't be much left from the original code. At that point it'd be just a call to the external module, and that's about it.

1)How do I even test this if I don't want to setup a box for hosting bots?

See xt/ directory. There are plenty examples on how to test features (both direct replies and gists can be tested). You can run test files directly, like ./xt/notable.t. It will start a local IRC server, then make a bot join it, then there will be another bot listening to messages and testing them. Make sure you cloned the repo with submodules.

Another option is to run sake debug:sourceable and the bot will join #whateverable channel on freenode.

2)What else is necessary?

Not much, really. I'm not really following the idea of is-safeish sub, because you can definitely do even more with evalable6 and other bots, so maybe it's not needed. There are some basic protections in place (like most of the filesystem being read-only when running as a systemd service), but otherwise I just expect people to not be assholes (it has been alright for years). I think @FCO is experimenting with dockerized runs, which are just a tad more secure, so if we get that in whateverable we'll then be able to plug it into some generic place, like get-output. By the way, yes, use get-output instead of merging stderr and stdout yourself.

Altai-man commented 4 years ago

Can you clarify why this is crossed out? I don't see any license in the main repo, meaning that we have to ask Zoffix for their permission

Because I am not really sure as well as not sure what to do. As you noted, there is no license in original repo and META6.json doesn't specify it either.

See xt/ directory

I see, thank you very much! I'll test it properly and add some new tests later.

By the way, yes, use get-output instead of merging stderr and stdout yourself.

Will hack on it more when time allows.

Altai-man commented 4 years ago

Added test coverage and made the code actually work. @AlexDaniel a review, please? I am also not sure how $*EXECUTABLE works in context of the place where wheneverable is running, on my dumb localhost it just works.

AlexDaniel commented 4 years ago

As previously mentioned, is-safeish is not needed. It gives false sense of security when we should actually do some steps in order to prevent damage. Now that I know a bit about docker I think I'll be able to hack something eventually that will work much better than this is-safeish sub.

I guess no shortcut tests because there are no shortcuts? That's fine.

If you want to improve it more, we can make it so that you can run it on any commit (much like committable). In that case we can just swap get-output with run-snippet and it'll pretty much just workâ„¢ as long as you give it the right args (including the commit hash).

Otherwise it's good to go. Thank you!

Altai-man commented 4 years ago

As previously mentioned, is-safeish is not needed.

Ok, will remove that...

I guess no shortcut tests because there are no shortcuts? That's fine.

I'd also added some error args test, will do later.

If you want to improve it more, we can make it so that you can run it on any commit (much like committable).

Given the scope of sourceable (search for buggish things in current rakudo) I am not sure it is usable. Maybe for some edge cases, but I think someone other might implement that if needed much.

Thanks for your review!

Altai-man commented 4 years ago

@AlexDaniel I have removed the subroutine in question, and not sure if I have more desire to tinker on this. Can we merge && look at how it works in reality and then think if we should make it any better?

AlexDaniel commented 4 years ago

Of course! Thank you very much!