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

Normalize file endings #102

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

minhnhatnoe
Copy link
Contributor

No description provided.

@minhnhatnoe minhnhatnoe marked this pull request as ready for review June 9, 2023 04:23
@minhnhatnoe
Copy link
Contributor Author

knock knock

Copy link
Owner

@natsukagami natsukagami left a comment

Choose a reason for hiding this comment

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

We should write a SQL migration file that does this normalization on all existing files/submissions.
On that note, can we do this within SQLite? I suppose we'd not want native line-endings, but to just normalize everything into \n, to make the database portable...

models/files.go Outdated Show resolved Hide resolved
@minhnhatnoe
Copy link
Contributor Author

We should write a SQL migration file that does this normalization on all existing files/submissions. On that note, can we do this within SQLite? I suppose we'd not want native line-endings, but to just normalize everything into \n, to make the database portable...

What do you mean by "within SQLite"? It looks like SQLite has no option for line endings normalization.

I'm currently normalizing BEFORE inserting them into the database. Maybe we can write a conversion script for the entire db and run it at startup? This can make startup slower than it should be, so a better solution would be saving the current OS in the db, then if it doesn't match we will issue some kind of warning or maybe panic.

@natsukagami
Copy link
Owner

natsukagami commented Jun 27, 2023

I'm currently normalizing BEFORE inserting them into the database.

When you merge this PR it creates a problem where we have an invariant assuming that all text in the database has already been normalized, which is not true for text that already has been inside the database before the update.
One of the ways we can handle this is to write an SQL migration file (something like the one in embed/assets/sql) that does this conversion for everything in the database. We can also special-case this migration, to run additional code on the Go side (probably altering some code in the db package).

That aside, my original vision for the kjudge.db file was that it should be a portable copy of the whole contest data, something akin to Themis' .contest file. SQLite already provides us with a cross-platform file format, we should strive to keep it that way.
That might mean we need to normalize to just a certain ending (e.g. everything to \n in the DB), and add them back when we serve them. This way we maintain the invariant that the DB is fully consistent across OSes.

@natsukagami
Copy link
Owner

There's also the problem of mismatch between the end user's OS (the contestants') and the server. We cannot normalize the contestant's code using \r\n into the Linux server's format, and serve the \n format back to the contestant; it breaks certain editors (looking at you Notepad).
We might want something more user-oriented here too.

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