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

Add a flag to gluon.exe to ignore its internal library modules #751

Closed
Etherian opened this issue Jun 13, 2019 · 7 comments · Fixed by #775
Closed

Add a flag to gluon.exe to ignore its internal library modules #751

Etherian opened this issue Jun 13, 2019 · 7 comments · Fixed by #775

Comments

@Etherian
Copy link
Contributor

It can be difficult to test changes to the standard library using the standalone interpreter / REPL because it finds the modules in its internal standard library before checking the file system for the updated files.

Adding a command line flag to skip its internal modules, or maybe just check the filesystem first, would be a nice convenience.


This might be a good introduction for me to Gluon's Rust codebase, if someone could point me to where and what needs to be changed.

@Marwes
Copy link
Member

Marwes commented Jun 18, 2019

Allowing custom modules to shadow standard modules seems like a potential security issue so some care should be taken.

So far I think there are two approaches (that would also work for other use cases).

Add a no-std option which disables loading of the standard library entirely. Since you could still copy over the library files and modify them you could then test those on their own.

Add a special marker to GLUON_PATH which indicates loading the internal standard library and then support multiple paths/markers for that setting. This would let you either remove or reorder the standard library loading to make sure your files come first. This could be used to only copy over a subset of the standard library which then shadows the real files.

I can write up some instructions later, do you have any opinion on which way would be best here? (Or an entirely different solution)

@Etherian
Copy link
Contributor Author

The GLUON_PATH option sounds like it might be more generally useful, but could still be a security vulnerability if the user isn't careful with their GLUON_PATH.

I think the best choice for now is a --no-std command line option. It's simple, and I think it's obvious and deliberate enough to not be a security issue.

I'd love instructions whenever you get a chance.

@Marwes
Copy link
Member

Marwes commented Jun 23, 2019

So the std lib lookup is here

let std_file = STD_LIBS.iter().find(|tup| tup.0 == module);
and you want to thread it in from here
pub struct Opt {
.

You would need to thread that setting into gluon_repl, gluon_doc, gluon_fmt and make sure that it is set on the Settings object https://github.com/gluon-lang/gluon/tree/7b44c202ef17f2974e18d0b5998ac847f684b652/src#L246 .

In import.rs you can then pass Compiler to

gluon/src/import.rs

Lines 172 to 177 in 7b44c20

fn get_unloaded_module(
&self,
vm: &Thread,
module: &str,
filename: &str,
) -> Result<UnloadedModule, MacroError> {
and read the setting there.

Let me know if anything is unclear!

@Etherian
Copy link
Contributor Author

Thanks! I'll let you know if I have any questions.

@Etherian
Copy link
Contributor Author

Etherian commented Jun 28, 2019

I noticed that file paths passed to gluon.exe on the command line are checked against its internal library too. Is this expected behavior?

@Marwes
Copy link
Member

Marwes commented Jun 28, 2019

That's just a consequence of it going through the same loading as any other file (which seems to make sense to me?)

@Etherian
Copy link
Contributor Author

Etherian commented Jun 28, 2019

While I did find it surprising, I doubt it would affect many users. But I do think it's likely that some file loading behavior will need to differ from script imports in some way eventually. How difficult would it be currently to make one file load differ from another within the same run?

bors bot added a commit that referenced this issue Aug 12, 2019
775: feat(repl): add a --no-std option to the standalone interpreter / REPL r=Marwes a=Etherian

## Description
Adds a command-line switch `--no-std` to the standalone interpreter / REPL which causes it to ignore its internal standard library modules when searching for modules to import.

This should make less awkward to use the REPL to test changes to the standard library.

resolves #751 

Co-authored-by: Etherian <etherain@gmail.com>
@bors bors bot closed this as completed in #775 Aug 12, 2019
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.

2 participants