Containerized compilation

[Matios102]

This pull request removes the dedicated compiler stage from the codebase, shifting compilation responsibility into the executor stage. The pipeline now detects compilation errors by checking for output in the compilation error file after execution, rather than relying on a separate compiler interface. The changes also update related tests and dependency injection throughout the codebase to reflect this architectural change.

Pipeline and Architecture Refactor:

• Removed the compiler stage and interface from the worker pipeline, dependency injection, and related code, consolidating compilation into the executor stage (cmd/main.go, internal/pipeline/pipeline.go, generate_mocks.sh) [1] [2] [3] [4] [5] [6] [7] [8].

• Updated the task processing logic to check for compilation errors by inspecting the compilation error file after execution, instead of invoking a compiler interface. This includes handling for both success and error flows (internal/pipeline/pipeline.go) [1] [2] [3] [4].

Testing Updates:

• Refactored all pipeline tests to remove usage of the compiler mock, update mock setups, and simulate compilation errors by writing directly to the compile error file in the executor mock (internal/pipeline/pipeline_test.go) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12].

Docker and Build Environment:

• Removed the installation of g++ from both the builder and runtime images in the Dockerfile, reflecting the removal of the dedicated compiler stage (Dockerfile) [1] [2].

Executor and Docker Client Enhancements:

• Updated the Docker client interface and implementation to add an alwaysCopyFiles parameter for more flexible file extraction from containers (internal/docker/docker_client.go) [1] [2] [3].

[Copilot]

Pull request overview

This PR refactors the architecture to move compilation into the containerized executor stage, eliminating the dedicated compiler interface and host-based compilation. Compilation now occurs inside Docker containers, with errors detected by checking a compilation error file after execution completes. This change simplifies the pipeline and removes the dependency on g++ in the host environment.

Key Changes

• Removed the compiler stage, interface, and all related code, consolidating compilation into the executor stage where it runs inside containers
• Modified the pipeline to detect compilation errors by checking file size after execution instead of catching exceptions from a compiler interface
• Enhanced the Docker client and tar extraction utilities to support an alwaysCopyFiles parameter, ensuring compilation error files are always extracted from containers

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
cmd/main.go	Removed compiler initialization and dependency injection
Dockerfile	Removed g++ from builder and runtime images as compilation now happens in containers
generate_mocks.sh	Removed compiler mock generation
internal/docker/docker_client.go	Added alwaysCopyFiles parameter to enable extracting compilation error files regardless of directory restrictions
internal/pipeline/pipeline.go	Removed compiler dependency, added compilation error detection via file size check after execution
internal/scheduler/scheduler.go	Removed compiler from scheduler constructor and worker initialization
internal/scheduler/scheduler_test.go	Updated tests to remove compiler mock usage
internal/stages/compiler/compiler.go	Deleted entire compiler interface and implementation
internal/stages/compiler/compiler_test.go	Deleted compiler tests
internal/stages/compiler/cpp_compiler.go	Deleted C++ compiler implementation
internal/stages/compiler/cpp_compiler_test.go	Deleted C++ compiler tests
internal/stages/executor/cpp/Dockerfile	Added g++ to container runtime to support in-container compilation
internal/stages/executor/executor.go	Added buildCompileCommand function and compilation-related fields to CommandConfig, updated environment variables to pass compilation commands to container
internal/stages/executor/executor_test.go	Updated mock expectations to account for new alwaysCopyFiles parameter
internal/stages/executor/run_tests.sh	Added compilation logic to handle in-container compilation before test execution
internal/pipeline/pipeline_test.go	Refactored all tests to remove compiler mocks and simulate compilation errors via file writes
tests/mocks/mocks_generated.go	Removed compiler mock, updated Docker client mock signature
utils/utils.go	Refactored ExtractTarArchiveFiltered to support alwaysCopyFiles parameter, added helper functions and context struct for cleaner code organization

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The alwaysCopyFiles validation logic may not work correctly if the file being copied is located in a subdirectory. The check uses filepath.Base(header.Name) which only matches the filename, but if a file with the same name exists in different directories within the tar archive, all of them would be extracted. This could potentially extract more files than intended. Consider validating against the full path or at least checking that the file is in the expected location.

[Copilot]

Similar to the issue in shouldExtractEntry, the validation logic uses filepath.Base(header.Name) which may incorrectly skip validation for files with the same name in different directories. If an attacker places a file with the same name as an alwaysCopy file (e.g., "compile.err") in a subdirectory, it would bypass file count validation. Consider checking the full path or ensuring the file is in the expected root location.

[Copilot]

The error returned from languages.GetVersionFlag is silently ignored. If GetVersionFlag returns an error (e.g., for an invalid version), the versionFlag will be an empty string, resulting in "-std=" being passed to g++ without a version, which will likely cause a compilation error. Consider handling this error properly or propagating it.

Suggested change

	versionFlag, _ := languages.GetVersionFlag(languages.CPP, cfg.LanguageVersion)
	return []string{
	"g++",
	"-o",
	filepath.Base(cfg.DirConfig.UserExecFilePath),
	"-std=" + versionFlag,
	filepath.Base(cfg.SourceFilePath),
	}
	versionFlag, err := languages.GetVersionFlag(languages.CPP, cfg.LanguageVersion)
	args := []string{
	"g++",
	"-o",
	filepath.Base(cfg.DirConfig.UserExecFilePath),
	}
	if err == nil && versionFlag != "" {
	args = append(args, "-std="+versionFlag)
	}
	args = append(args, filepath.Base(cfg.SourceFilePath))
	return args

[HermanPlay]

@Matios102

[Copilot]

The compilation failure at line 56 exits with code 0 (success), which means the container will appear to have completed successfully. However, this is intentional since the pipeline detects compilation errors by checking the error file content. Consider adding a comment explaining why exit 0 is used here instead of a non-zero exit code, as this is counter-intuitive behavior that could confuse future maintainers.

Suggested change

	echo "Compilation failed. Error details saved to ${COMPILE_ERR_FILE}"
	exit 0
	fi
	# Check if executable was created
	if [[ ! -f "${EXEC_FILE}" ]]; then
	echo "Compilation failed: executable not created" > "${COMPILE_ERR_FILE}"
	echo "Compilation failed. Error details saved to ${COMPILE_ERR_FILE}"
	# Intentionally exit 0 here: the surrounding pipeline detects compilation
	# failures by inspecting ${COMPILE_ERR_FILE}, not this script's exit code.
	# Changing this to a non-zero exit would break that contract by causing the
	# container to be treated as failed instead of letting the pipeline logic
	# handle compilation errors.
	exit 0
	fi
	# Check if executable was created
	if [[ ! -f "${EXEC_FILE}" ]]; then
	echo "Compilation failed: executable not created" > "${COMPILE_ERR_FILE}"
	# Same rationale as above: we must still exit 0 so that the pipeline relies
	# on ${COMPILE_ERR_FILE} to signal the compilation failure.

[Copilot]

Typo in log message: "occured" should be "occurred".

Suggested change

	ws.logger.Infof("Compilation error occured for message ID: %s", ws.state.ProcessingMessageID)
	ws.logger.Infof("Compilation error occurred for message ID: %s", ws.state.ProcessingMessageID)

[Copilot]

The compilation error check at line 166 is performed for all language types, including scripting languages like Python that don't require compilation. This could lead to false positives if a compilation error file happens to exist from a previous run or is created inadvertently. Consider checking requiresCompilation before performing the compilation error check to ensure this logic only applies to compiled languages.

Suggested change

	// Check for compilation error
	if fileInfo, statErr := os.Stat(dc.CompileErrFilePath); statErr == nil && fileInfo.Size() > 0 {
	ws.publishCompilationError(dc, task.TestCases)
	return
	// Check for compilation error (only for languages that require compilation)
	if requiresCompilation {
	if fileInfo, statErr := os.Stat(dc.CompileErrFilePath); statErr == nil && fileInfo.Size() > 0 {
	ws.publishCompilationError(dc, task.TestCases)
	return
	}

[Copilot]

Same issue as line 58: exiting with code 0 when the executable wasn't created is counter-intuitive. The pipeline relies on checking the error file content to detect this failure. Consider adding a comment explaining this design decision.

Suggested change

	echo "Compilation failed. Error details saved to ${COMPILE_ERR_FILE}"
	exit 0
	fi
	# Check if executable was created
	if [[ ! -f "${EXEC_FILE}" ]]; then
	echo "Compilation failed: executable not created" > "${COMPILE_ERR_FILE}"
	echo "Compilation failed. Error details saved to ${COMPILE_ERR_FILE}"
	# Intentionally exit with code 0 here: the surrounding pipeline checks COMPILE_ERR_FILE
	# to detect compilation failures instead of relying on this script's exit status.
	exit 0
	fi
	# Check if executable was created
	if [[ ! -f "${EXEC_FILE}" ]]; then
	echo "Compilation failed: executable not created" > "${COMPILE_ERR_FILE}"
	# Same design as above: signal failure via COMPILE_ERR_FILE while exiting successfully
	# so the pipeline can continue and aggregate results based on file contents.

[HermanPlay]

@Matios102

[HermanPlay]

very good