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 version commands for more languages/executables #2541

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vmcj
Copy link
Member

@vmcj vmcj commented May 11, 2024

This tries to be non-destructive for upgrades but would remove the command if people have picked the same command and they choose to run the down migration, in that specific edgecase I assume people know what/(and why) they are doing this.

I've not done all the languages yet as I would like some feedback first. it now has all known languages for my installation.

webapp/migrations/Version20240511091916.php Outdated Show resolved Hide resolved
sql/files/defaultdata/adb/run Outdated Show resolved Hide resolved
generate_sql.py Outdated Show resolved Hide resolved
@vmcj vmcj requested a review from meisterT May 11, 2024 13:06
@vmcj
Copy link
Member Author

vmcj commented May 11, 2024

@meisterT should this just replace what we do in the default data? If you wipe your database we always run the migrations and we always run the defaultdata fixture so one of them is redundant. I think the migration is better as it also helps in case people upgrade.

@meisterT
Copy link
Member

I would keep both. The migration is clearly necessary now when you upgrade. The defaultdata fixture is way nicer when you add a completely new language (we wouldn't add a migration for that).

@vmcj
Copy link
Member Author

vmcj commented May 11, 2024

I would keep both. The migration is clearly necessary now when you upgrade. The defaultdata fixture is way nicer when you add a completely new language (we wouldn't add a migration for that).

I think when we add a new language and you upgrade you would indeed not get the new language.

@meisterT
Copy link
Member

And that is fine. Adding a language is a conscious decision anyway and requires changes to the chroot typically.

@vmcj vmcj force-pushed the language_chroot_setup branch 2 times, most recently from 2388bbf to 81295dd Compare May 11, 2024 23:50
doc/manual/install-language.rst Outdated Show resolved Hide resolved
example_problems/problems.yaml Outdated Show resolved Hide resolved
gitlab/integration.sh Outdated Show resolved Hide resolved
doc/manual/install-language.rst Outdated Show resolved Hide resolved
doc/manual/install-language.rst Outdated Show resolved Hide resolved
doc/manual/install-language.rst Outdated Show resolved Hide resolved
doc/manual/install-language.rst Outdated Show resolved Hide resolved
sql/files/defaultdata/adb/run Outdated Show resolved Hide resolved
@vmcj vmcj requested a review from meisterT May 14, 2024 07:07
@@ -10,6 +10,9 @@ MAINSOURCE="$1"
# Set non-existing HOME variable to make GHC program happy, see:
# https://ghc.haskell.org/trac/ghc/ticket/11678
export HOME=/does/not/exist
# Allow temporary files during compilation, this directory is not
# available during the submission run.
export TMPDIR=`pwd`
Copy link
Member

Choose a reason for hiding this comment

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

is $PWD defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

pwd is the command to get the current work directory. Why do you prefer $PWD over pwd?

Copy link
Member

Choose a reason for hiding this comment

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

pwd spawns a process, $PWD just reads an environment variable

Copy link
Member

@meisterT meisterT left a comment

Choose a reason for hiding this comment

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

I think it looks good after cleaning up the commits.

@vmcj
Copy link
Member Author

vmcj commented May 14, 2024

I think it looks good after cleaning up the commits.

I'll wait for @eldering his review and will squash afterwards.

vmcj added 3 commits May 19, 2024 21:16
DOMjudge ships with default languages but no documentation on how to
install those. This commit documents how to get a working setup in the
chroot to get the example problems to run.

In the past we had the idea to enable all languages in
the integration test. If we would do that we could detect the required
package changes also and keep this up to date.

For the interpreted languages we use mostly use the same binary as
runner & compiler.

Some of the versions were already added before and are only documented
on how to install those now. The migration is added for people to get
the version commands to make it easier to use additional languages and
still make it easy to publish the used versions, useful for online
mirrors etc.

The version for `dash`/`sh` is difficult as there is no simple
`--version` and getting the value with `dpkg -s` does not work inside
our chroot during runtime as not all locations are mounted. For more
information see: https://stackoverflow.com/questions/36660724/how-to-tell-the-version-number-of-dash
and
https://unix.stackexchange.com/questions/416760/how-to-find-out-the-version-number-of-dash-without-resorting-to-package-manage.
The other alternative would be `file` as that gives more information
than `md5sum` but its also not available in the chroot.

The installation for swift could be easier if we allow tarballs to be
unpacked in the future. At the moment people would either need to change
`dj_make_chroot` or do it afterwards with `dj_run_chroot`.

Javascript is interpreted so we copy the behaviour of python. While
testing I found that we can only have run-error but there is a `--check`
which we could use to verify that there is a sort of compilation step.
Contents based on the values in the DefaultData/LanguageFixture.
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.

None yet

3 participants