sinol-make file structure and run command #1

MasloMaslane · 2023-04-18T08:35:21Z

What's left to do:

compile programs without makefile
~~check if dependencies are installed for run command (time and timeout)~~
~~add measuring time and memory with python as an option (do we want this?)~~

twalen

Just approving, if you would like to have normal review, please let me know.

tonowak · 2023-04-19T10:26:07Z

Just approving, if you would like to have normal review, please let me know.

@twalen while I plan to review the project in the context of added functionality, decisions and etc, I don't have much experience with Python. I would appreciate if you could make reviews 😄

tonowak · 2023-04-19T10:28:01Z

add measuring time and memory with python as an option (do we want this?)

That depends on the existing issues with oiejq/sio2jail. One of the next steps would be to write tests (e.g. in GitHub Actions) to check whether the tool works correctly on the popular operating systems. If we won't find any issues, there is no need to have a separate measuring option (especially that I want to force the usage of oiejq). If we'll have issues, then we can consider implementing the time.

tonowak · 2023-04-19T10:30:30Z

What's left to do:

Let's try to implement then in separate PRs.

MasloMaslane · 2023-04-19T10:31:05Z

add measuring time and memory with python as an option (do we want this?)

That depends on the existing issues with oiejq/sio2jail. One of the next steps would be to write tests (e.g. in GitHub Actions) to check whether the tool works correctly on the popular operating systems. If we won't find any issues, there is no need to have a separate measuring option (especially that I want to force the usage of oiejq). If we'll have issues, then we can consider implementing the time.

I think that oiejq only works on linux. I remember trying to compile it on macOS, but I didn't succeed. Maybe after changing some code it may work?

MasloMaslane · 2023-04-19T10:34:01Z

What's left to do:

Let's try to implement then in separate PRs.

You mean that compiling without makefiles should be in seperate PR? There is some unfinished code for compiling here

tonowak

Overall looks good!

README.md

pyproject.toml

src/sinol_make/commands/run/__init__.py

src/sinol_make/config/__init__.py

src/sinol_make/util.py

tonowak · 2023-04-19T10:51:46Z

I think that oiejq only works on linux. I remember trying to compile it on macOS, but I didn't succeed. Maybe after changing some code it may work?

From what I remember, Marek Sokołowski might be a good person to ask about MacOS stuff.

tonowak · 2023-04-19T10:52:29Z

You mean that compiling without makefiles should be in seperate PR? There is some unfinished code for compiling here

Yes, I prefer to have separate functionalities in separate PRs (but you don't have to split this PR). Otherwise reviewing becomes a mess and PRs are left open for eternity.

src/sinol_make/commands/run/__init__.py

pyproject.toml

MasloMaslane · 2023-04-19T13:41:54Z

Also, which license do we want to use?

tonowak · 2023-04-19T13:53:28Z

The same as sio2, I guess.

MasloMaslane · 2023-04-19T15:22:29Z

9d1b07a I removed global config and finished compilation without makefiles

tonowak · 2023-04-19T15:23:11Z

Nice! I'll try to review it today.

add --oiejq_path flag for run command replace asserts with proper error messages

src/sinol_make/commands/run/__init__.py

src/sinol_make/helpers/compiler.py

tonowak

Just some small comments. I think we're going to merge this PR as soon as they are resolved 🚀

src/sinol_make/__init__.py

src/sinol_make/commands/run/__init__.py

src/sinol_make/helpers/compile.py

src/sinol_make/commands/run/__init__.py

src/sinol_make/util.py

src/sinol_make/commands/run/__init__.py

This reverts commit 63ac565 and ac0acca.

MasloMaslane · 2023-04-28T17:53:53Z

Config suggestions was moved to #3, so I think we can merge this. @tonowak what do you think?

tonowak

Looks good, I'm giving approve so that you can merge (if you think that the TODO comments are not enough and you'll forget about changing something, you can create an issue). Though, before merging, please resolve the last open comment.

tonowak · 2023-04-28T15:52:58Z

src/sinol_make/commands/run/__init__.py

+		for program in self.get_programs(None):
+			ext = os.path.splitext(program)[1]
+			if ext == '.c' and args.c_compiler_path is None:
+				if sys.platform == 'darwin':
+					print(util.bold(util.color_red('Couldn\'t find a C compiler. Tried gcc-{9,10,11,12}. Try specifying a compiler with --c_compiler_path.')))
+					exit(1)
+				else:
+					print(util.bold(util.color_red('Couldn\'t find a C compiler. Tried gcc. Try specifying a compiler with --c_compiler_path.')))
+			elif ext == '.cpp' and args.cpp_compiler_path is None:
+				if sys.platform == 'darwin':
+					print(util.bold(util.color_red('Couldn\'t find a C++ compiler. Tried g++-{9,10,11,12}. Try specifying a compiler with --cpp_compiler_path.')))
+					exit(1)
+				else:
+					print(util.bold(util.color_red('Couldn\'t find a C++ compiler. Tried g++. Try specifying a compiler with --cpp_compiler_path.')))
+					exit(1)
+			elif ext == '.py' and args.python_interpreter_path is None:
+				print(util.bold(util.color_red('Couldn\'t find a Python interpreter. Tried python3. Try specifying an interpreter with --python_interpreter_path.')))
+				exit(1)
+			elif ext == '.java' and args.java_compiler_path is None:
+				print(util.bold(util.color_red('Couldn\'t find a Java compiler. Tried javac. Try specifying a compiler with --java_compiler_path.')))
+				exit(1)


Big wall of similar text. Maybe we can simplify it?

Is it better now? 273e79e

Kostero · 2023-04-29T06:19:10Z

src/sinol_make/helpers/compile.py

+	arguments = []
+	if ext == '.cpp':
+		arguments = [compilers['cpp_compiler_path'] or compiler.get_cpp_compiler_path(), program, '-o', output] + \
+						'-O3 -lm -Werror -Wall -Wextra -Wshadow -Wconversion -Wno-unused-result -Wfloat-equal'.split(' ')


can we make this as a variable/argument and then just pass C_FLAGS?

Maybe we should add flags to config.yml and if none are set then the code should use less strict flags then those that are now used?

Or by default the flags should be the same as those that oioioi uses

It would indeed be beneficial to be able to pass custom flags, e.g. -fsanitize=address,undefined or -D_GLIBCXX_DEBUG, or temporarily disable one of the warning flags.

But then, the calculated times won't necessarily be the same as the ones on sio2 and it might also trigger the sinol_expected_scores mechanism. If we want to have the ability to specify flags, it should give a warning that the times might not be the same and ignore the sinol_expected_scores. Probably later we would want to have a sinol-make verify that runs each command and checks whether they work as expected (e.g. the input files haven't changed their hashes, they satisfy the inwer, the output files are correct, the sinol_expected_scores is set as expected, etc) and in this command we wouldn't want the user to specify the custom flags.

To do it perfectly we might need to put into it more thought and the implementation won't be trivial. I think, at this point, it's better to create an Issue for later, focus on merging this PR and adding non-necessary functionality after writing basic tests.

I created the issue #4, so I think that we can merge this PR

* Add flag to choose template This change allows providing a custom template arg flag -t/--template: ``` sinol-make init foo -t [repo link or disk path] [optional subdir] ``` It defaults to the upstream repository and example_package subdir. Changed the example_package id pattern from `abc` to `__ID__`. All filenames and file contents are now substituted. Changed the positional output directory argument to `-o/--output` flag to help avoid accidents. Added result directory cleanup on failure. * Test quick fix #1 * Always cleanup tmpdir * Add other git url schemes * One more used_tmpdir tweak * Pacify tests #2 * Leave old 'abc' template string for now * Test without connecting to github Try accessing the local git repo in the project root first when cloning example_package. Only if that doesn't work, fall back to online github. Also adds some extra local path tests to init/test_unit. Also removes one unnecessary os.getcwd() call. Also removes the old "abc" template string hack since we test the current template now. * Fall back on copying local directory instead of online github

sinol-make file structure and run command

f2b03c5

MasloMaslane requested a review from tonowak April 18, 2023 08:35

MasloMaslane self-assigned this Apr 18, 2023

twalen approved these changes Apr 18, 2023

View reviewed changes

tonowak reviewed Apr 19, 2023

View reviewed changes

fix indentation, mention changes made in run

3119b83

tonowak reviewed Apr 19, 2023

View reviewed changes

src/sinol_make/commands/run/__init__.py Outdated Show resolved Hide resolved

tonowak reviewed Apr 19, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Update pyproject.toml

ddee221

compilation without makefiles

9d1b07a

MasloMaslane added 4 commits April 19, 2023 17:26

Add GNU GPLv3 license

30a6f20

remove measuring time with time

09f92da

check for oiejq only in ~/.local/bin

ad0f9f0

add --oiejq_path flag for run command replace asserts with proper error messages

rename oiejq.sh to oiejq

1cd1f0f

tonowak reviewed Apr 19, 2023

View reviewed changes

src/sinol_make/commands/run/__init__.py Outdated Show resolved Hide resolved

src/sinol_make/helpers/compiler.py Show resolved Hide resolved

src/sinol_make/helpers/compiler.py Outdated Show resolved Hide resolved

MasloMaslane added 3 commits April 20, 2023 08:59

default values for compiler flags

adf7091

add missing checks for Windows/Cygwin

4c9c813

Update readme

ebc5c61

tonowak reviewed Apr 23, 2023

View reviewed changes

MasloMaslane added 12 commits April 26, 2023 12:20

remove trailing new lines and spaces

d25f2b9

refactor program compiling

8453f36

change 'not in project' error message

0f98fd0

change error message when oiejq download failed

636c08e

Change names of compiler flags

c17ac32

fix errors when oiejq is not installed

ef9d92c

fix oiejq errors, add missing exit

a481261

remove verbose flag

f13e098

change behaviour of show_memory flag

bc89098

change schema of subtasks in config

ac0acca

suggest subtask configuration when none found

63ac565

Revert "lasta two commits"

946dbf7

This reverts commit 63ac565 and ac0acca.

MasloMaslane marked this pull request as ready for review April 28, 2023 17:52

tonowak approved these changes Apr 28, 2023

View reviewed changes

Kostero reviewed Apr 29, 2023

View reviewed changes

refactor

273e79e

MasloMaslane mentioned this pull request Apr 30, 2023

Add possibility to measure performance with time #2

Closed

1 task

MasloMaslane merged commit 06c0eff into main Apr 30, 2023

MasloMaslane deleted the sinol-make-structure branch April 30, 2023 19:58

j4b6ski added a commit to j4b6ski/sinol-make that referenced this pull request Dec 8, 2024

Test quick fix sio2project#1

89e1fc1

j4b6ski added a commit to j4b6ski/sinol-make that referenced this pull request Dec 8, 2024

Test quick fix sio2project#1

ba4145a

j4b6ski added a commit to j4b6ski/sinol-make that referenced this pull request Dec 8, 2024

Test quick fix sio2project#1

48a9028

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sinol-make file structure and run command #1

sinol-make file structure and run command #1

MasloMaslane commented Apr 18, 2023 •

edited

Loading

twalen left a comment

tonowak commented Apr 19, 2023 •

edited

Loading

tonowak commented Apr 19, 2023

tonowak commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

tonowak left a comment

tonowak commented Apr 19, 2023

tonowak commented Apr 19, 2023 •

edited

Loading

MasloMaslane commented Apr 19, 2023

tonowak commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

tonowak commented Apr 19, 2023

tonowak left a comment

MasloMaslane commented Apr 28, 2023

tonowak left a comment

tonowak Apr 28, 2023

MasloMaslane Apr 29, 2023 •

edited

Loading

Kostero Apr 29, 2023

MasloMaslane Apr 29, 2023

MasloMaslane Apr 29, 2023

tonowak Apr 30, 2023

MasloMaslane Apr 30, 2023

sinol-make file structure and run command #1

sinol-make file structure and run command #1

Conversation

MasloMaslane commented Apr 18, 2023 • edited Loading

twalen left a comment

Choose a reason for hiding this comment

tonowak commented Apr 19, 2023 • edited Loading

tonowak commented Apr 19, 2023

tonowak commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

tonowak left a comment

Choose a reason for hiding this comment

tonowak commented Apr 19, 2023

tonowak commented Apr 19, 2023 • edited Loading

MasloMaslane commented Apr 19, 2023

tonowak commented Apr 19, 2023

MasloMaslane commented Apr 19, 2023

tonowak commented Apr 19, 2023

tonowak left a comment

Choose a reason for hiding this comment

MasloMaslane commented Apr 28, 2023

tonowak left a comment

Choose a reason for hiding this comment

tonowak Apr 28, 2023

Choose a reason for hiding this comment

MasloMaslane Apr 29, 2023 • edited Loading

Choose a reason for hiding this comment

Kostero Apr 29, 2023

Choose a reason for hiding this comment

MasloMaslane Apr 29, 2023

Choose a reason for hiding this comment

MasloMaslane Apr 29, 2023

Choose a reason for hiding this comment

tonowak Apr 30, 2023

Choose a reason for hiding this comment

MasloMaslane Apr 30, 2023

Choose a reason for hiding this comment

MasloMaslane commented Apr 18, 2023 •

edited

Loading

tonowak commented Apr 19, 2023 •

edited

Loading

tonowak commented Apr 19, 2023 •

edited

Loading

MasloMaslane Apr 29, 2023 •

edited

Loading