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

Compare script language defaults #2135

Closed
wants to merge 8 commits into from

Conversation

vmcj
Copy link
Member

@vmcj vmcj commented Sep 1, 2023

If/When this is approved we need another PR to also use these values in the run/build scripts per language to fully implement the CLICS spec. Given that we don't implement the draft yet we have some time still.

One import thing is that I've changed the python to pypy3 to be consistent, this will require that this is installed in the host so I'm not sure if the consistency is wanted.

See:
https://ccs-specs.icpc.io/2023-06/contest_api?highlight=args#languages

The fields are not exposed yet in the API, not shown on the page yet and
not used for actual building yet.
We can now also set the default for the version command to remove some
duplication.

This would according to spec already be picked if we leave the field
empty but explicit is always better.
@meisterT
Copy link
Member

meisterT commented Sep 1, 2023

I cannot really review this on my tiny phone screen, just wanted to leave two comments:

I started some work in this direction here #1241 - we probably want to use the same mechanism to pass in flags for both?

Historically speaking, we decided explicitly not to use the configured languages as you might have different options for submissions and validators. I think this is worth reconsidering; my current gut feeling is that we could do something like this:
Have per language an executable defined with which validators are compiled as well custom flags and a boolean, e.g. to disallow Python as submission language but allow it as validator language. If allowed as submission language but not separate executable / flags are set, we fall back to the configuration for submissions.

@vmcj
Copy link
Member Author

vmcj commented Sep 1, 2023

I cannot really review this on my tiny phone screen, just wanted to leave two comments:

I'll wait for you to be on a larger screen,

I started some work in this direction here #1241 - we probably want to use the same mechanism to pass in flags for both?

That was the intent, I extended the language object with the CLICS flags {runner,compiler} where you had those specific for the _version. If that is not available we use the old defaults of the judgedaemon.

Historically speaking, we decided explicitly not to use the configured languages as you might have different options for submissions and validators. I think this is worth reconsidering; my current gut feeling is that we could do something like this: Have per language an executable defined with which validators are compiled as well custom flags and a boolean, e.g. to disallow Python as submission language but allow it as validator language. If allowed as submission language but not separate executable / flags are set, we fall back to the configuration for submissions.

This PR was to work towards #1238, where it was proposed (if I understand correctly) to explicit use the same options.

@meisterT
Copy link
Member

meisterT commented Sep 3, 2023

If I understand things correctly, after this PR languages will have fields in the UI that ask for compiler/runner and their args. But these commands are not used for the submission language, and only for the validators which can be very confusing (while the code is clean).

My proposal is actually to not introduce the new fields in the UI as a first step, but move to using the same executables that we use for submission languages for validators.

This will allow us to:

  • Eventually split them again, e.g. allow to define a "submission executable" and "validator executable" per language (where we fall back to the submission executable if no validator executable is defined). As well as having a "allow for validation" toggle, so you could allow say python validators but disallow python as submission language. This should help with addressing Compare scripts should be build using language defaults #1238.
  • Add compiler/runner and their args in a separate step since this requires some thorough thinking of how this works with invalidating caching in the judging backend.

@vmcj
Copy link
Member Author

vmcj commented Sep 3, 2023

If I understand things correctly, after this PR languages will have fields in the UI that ask for compiler/runner and their args. But these commands are not used for the submission language, and only for the validators which can be very confusing (while the code is clean).

My proposal is actually to not introduce the new fields in the UI as a first step, but move to using the same executables that we use for submission languages for validators.

This will allow us to:

* Eventually split them again, e.g. allow to define a "submission executable" and "validator executable" per language (where we fall back to the submission executable if no validator executable is defined). As well as having a "allow for validation" toggle, so you could allow say python validators but disallow python as submission language. This should help with addressing [Compare scripts should be build using language defaults #1238](https://github.com/DOMjudge/domjudge/issues/1238).

* Add compiler/runner and their args in a separate step since this requires some thorough thinking of how this works with invalidating caching in the judging backend.

Makes sense, so I'll close this for now.

@vmcj vmcj closed this Sep 3, 2023
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