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

Fix JasminResult.run() not working #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ttoino
Copy link
Contributor

@ttoino ttoino commented May 10, 2024

JasminResult.run() had two problems in my compiler deployment, namely: the libs-jmm directory was required to be able to interact with the outside world, and bash was also required to run the program, which wasn't available in the alpine based image

@limwa
Copy link
Owner

limwa commented May 10, 2024

Once again, thank you for your PR!

Personally, I didn't plan on supporting compiled code execution in the website but, if we go with that route, we need to address two main concerns:

  1. This website was made to be future proof and manually adding the files that need to be copied to the Dockerfile might not work in the best of ways... I was thinking we could investigate how to do that on the Gradle build.gradle.kts file and then create documentation explaining how to do that. That way, the folders can change and, as long as it is adjusted on the Gradle side, it will just work. That also gets us out of a position where we might break backwards compatibility just because a folder changed name in more recent editions of the Compilers course.
  2. The website wasn't made for code execution and, as such, it lacks many necessary safety features. To "officially" support code execution, we would need to create a new user on the image to execute the code, so that the code can't modify the website files. Furthermore, a timeout would need to be introduced so that we can avoid infinite loops.

What do you think?

@ttoino
Copy link
Contributor Author

ttoino commented May 10, 2024

  1. This website was made to be future proof and manually adding the files that need to be copied to the Dockerfile might not work in the best of ways... I was thinking we could investigate how to do that on the Gradle build.gradle.kts file and then create documentation explaining how to do that. That way, the folders can change and, as long as it is adjusted on the Gradle side, it will just work. That also gets us out of a position where we might break backwards compatibility just because a folder changed name in more recent editions of the Compilers course.

You're right, that's definitely the better way of doing things, I just sort of hacked it to work with my compiler.

  1. The website wasn't made for code execution and, as such, it lacks many necessary safety features. To "officially" support code execution, we would need to create a new user on the image to execute the code, so that the code can't modify the website files. Furthermore, a timeout would need to be introduced so that we can avoid infinite loops.

Yeah, I hadn't thought of that, although infinite loops don't seem to break my deployment.

@limwa
Copy link
Owner

limwa commented May 10, 2024

  1. This website was made to be future proof and manually adding the files that need to be copied to the Dockerfile might not work in the best of ways... I was thinking we could investigate how to do that on the Gradle build.gradle.kts file and then create documentation explaining how to do that. That way, the folders can change and, as long as it is adjusted on the Gradle side, it will just work. That also gets us out of a position where we might break backwards compatibility just because a folder changed name in more recent editions of the Compilers course.

You're right, that's definitely the better way of doing things, I just sort of hacked it to work with my compiler.

Yeah, that would also solve the problem with the config.properties file in my compiler! Are you going to investigate that or do you prefer it if I do it?

  1. The website wasn't made for code execution and, as such, it lacks many necessary safety features. To "officially" support code execution, we would need to create a new user on the image to execute the code, so that the code can't modify the website files. Furthermore, a timeout would need to be introduced so that we can avoid infinite loops.

Yeah, I hadn't thought of that, although infinite loops don't seem to break my deployment.

That's interesting. Technically, the website waits for the compiler process to exit 🤔

@ttoino
Copy link
Contributor Author

ttoino commented May 11, 2024

  1. This website was made to be future proof and manually adding the files that need to be copied to the Dockerfile might not work in the best of ways... I was thinking we could investigate how to do that on the Gradle build.gradle.kts file and then create documentation explaining how to do that. That way, the folders can change and, as long as it is adjusted on the Gradle side, it will just work. That also gets us out of a position where we might break backwards compatibility just because a folder changed name in more recent editions of the Compilers course.

You're right, that's definitely the better way of doing things, I just sort of hacked it to work with my compiler.

Yeah, that would also solve the problem with the config.properties file in my compiler! Are you going to investigate that or do you prefer it if I do it?

I don't plan on spending more time on this, at least for now, so feel free to close this PR if you prefer!

@limwa
Copy link
Owner

limwa commented May 11, 2024

I'll keep this open and pick it up when I have the time.

@limwa limwa self-assigned this May 11, 2024
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.

2 participants