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

.NET 6 support #1064

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

.NET 6 support #1064

wants to merge 10 commits into from

Conversation

timbussmann
Copy link

relates to #1060
requires .NET 6 available on the runtimes image: DMOJ/runtimes-docker#40

A first attempt at adding .NET 6 support. This is what I cobbled together based on the some other executor implementations.

Given my lack of detailed understanding of the judge server and python, there are most likely issues with the implementation. Still, it is somewhat complicated to test the behavior locally (especially without the updated runtime image). I'd appreciate some help/guidance on how to test the implementation locally/on docker.

@dmoj-build
Copy link
Collaborator

Can one of the admins verify this patch?

@Riolku
Copy link
Contributor

Riolku commented Oct 17, 2022

Seeing that you fail lint, can you look at the errors and fix them?

Copy link
Contributor

@Riolku Riolku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code doesn't look egregious. Did you setup a copy of the modified docker image and test this locally?

dmoj/executors/NETCS.py Outdated Show resolved Hide resolved
dmoj/executors/NETCS.py Outdated Show resolved Hide resolved
dmoj/executors/NETCS.py Outdated Show resolved Hide resolved
"""

HELLO_WORLD_PROGRAM = """\
Console.WriteLine("Hello, World!");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This program should echo input to output, not just write out "Hello, World!". See e.g. https://github.com/DMOJ/judge-server/blob/master/dmoj/executors/C.py#L11

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@Xyene Xyene Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not "wrong" because it so happens the string we pass as input is "Hello, World!". But not testing input means it might be broken (for syscall whitelist reasons) and CI wouldn't catch it. Rust should be switched to echoing, but it seems more likely that .NET would have wacky syscalls during input than Rust.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update it sometime later then.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 will change the behavior

@timbussmann
Copy link
Author

Did you setup a copy of the modified docker image and test this locally?

@Riolku tbh I'm struggling a bit with this. locking at the docker files in .docker they seem to load files from https://github.com/DMOJ/judge-server/archive/"${TAG}".tar.gz (e.g. in https://github.com/DMOJ/judge-server/blob/master/.docker/tier3/Dockerfile) instead of using local files. Do I have to modify the docker file or is there an easier way to test this locally?

@Xyene
Copy link
Member

Xyene commented Oct 19, 2022

I'd say:

  • Build the runtimes-docker image locally with your changes.
  • Build the corresponding judge image as-is, it should base itself off your runtimes-docker build rather than pull in a copy from docker.io. As you note this'll pull in code from the master branch; this is fine for now.
  • Because the autoconfig didn't run with your changes, enter the container and modify /judge-runtime-paths.yml to point to dotnet.
  • Pass -v /path/to/your/judge/checkout/including/this/PR:/judge when starting the Docker container. This will mount your sources over where the container expects judge sources to live, allowing you to test this PR. (Instead of editing /judge-runtime-paths.yml in the container you can also pull this trick for it as well.)

@timbussmann
Copy link
Author

thanks the -v /path/to/your/judge/checkout/including/this/PR:/judge did the trick (although it required some extra workarounds when running on Windows)

(Instead of editing /judge-runtime-paths.yml in the container you can also pull this trick for it as well.)

how would this work, given the judge-runtime-paths.yml is in the root folder?

running the container yields some errors that seem to be related to some access protection mechanism:

Self-testing NETCS:  Traceback (most recent call last):
  File "/judge/dmoj/cptbox/tracer.py", line 286, in _callback
    return callback(self.debugger)
  File "/judge/dmoj/cptbox/isolate.py", line 412, in inner
    check(debugger)
  File "/judge/dmoj/cptbox/isolate.py", line 298, in check
    self._access_check(debugger, full_path, fs_jail)
  File "/judge/dmoj/cptbox/isolate.py", line 375, in _access_check
    if not fs_jail.check(real):
  File "/judge/dmoj/cptbox/filesystem_policies.py", line 124, in check
    assert os.path.abspath(path) == path, 'Must pass a normalized, absolute path to check'
AssertionError: Must pass a normalized, absolute path to check
Exception ignored in: 'dmoj.cptbox._cptbox.Process._syscall_handler'
Traceback (most recent call last):
  File "/judge/dmoj/cptbox/tracer.py", line 286, in _callback
    return callback(self.debugger)
  File "/judge/dmoj/cptbox/isolate.py", line 412, in inner
    check(debugger)
  File "/judge/dmoj/cptbox/isolate.py", line 298, in check
    self._access_check(debugger, full_path, fs_jail)
  File "/judge/dmoj/cptbox/isolate.py", line 375, in _access_check
    if not fs_jail.check(real):
  File "/judge/dmoj/cptbox/filesystem_policies.py", line 124, in check
    assert os.path.abspath(path) == path, 'Must pass a normalized, absolute path to check'
AssertionError: Must pass a normalized, absolute path to check

any suggestions?

@Riolku
Copy link
Contributor

Riolku commented Oct 19, 2022

The code causing this error is... kind of wrong. I'm honestly a bit confused because a different PR will handle /proc/self differently.

I guess the TL;DR is congratulations, .NET does funky stuff with files and it causes errors in our sandbox, so you probably can't test this right away.

Also, note that we don't support Windows anymore (how did you even test on Windows...). Did you use WSL or something?

@timbussmann
Copy link
Author

The code causing this error is... kind of wrong. I'm honestly a bit confused because a different PR will handle /proc/self differently.

based on the Must pass a normalized, absolute path to check message I was wondering whether this indicates some incorrect file paths?

I guess the TL;DR is congratulations, .NET does funky stuff with files and it causes errors in our sandbox, so you probably can't test this right away.

so is there a way to resolve this?

Also, note that we don't support Windows anymore (how did you even test on Windows...). Did you use WSL or something?

well, let's say it has been a journey:

  1. I've been using VS Code dev containers to have a python environment available in the first place and those containers are Linux containers that mount the source folders from Windows.
  2. The docker images need to be built back on the Windows side again though since that's where Docker is installed.
  3. Now running the docker containers from windows didn't work due to mounting the the local source folders because apparently, I can't easily mount arbitrary folder paths on a Windows drive. That's where WSL came into play because WSL can access the same docker instance but the on Linux I can properly mount folders (and then refer to the the /mnt/c/... path to the Windows folder.
  4. now I had to modify the judge-runtime-paths.yml and I couldn't figure out how I could easily use the suggested -v approach so I copied the file from container (docker cp), modified it and moved it back in. I think I also overwrote the entry-path on docker run to just start a shell and fix the file inside the container and then calling the regular ./docker/entry script manually.
  5. At this point the line endings finally came back to give me some more troubles because the mounted start script for the docker container had Windows newline characters (the source folder is still on Windows) and that made the docker container refuse the bash script until I forced windows to use Linux-style line endings.

(I guess I'm just writing this all down so I can remember how to do this again at some point 🤣 )

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.

4 participants