Skip to content

Refactor testCloneRepository function and update .gitignore #85

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

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
18c57de
refactor: enhance testCloneRepository function with git executable ha…
Apr 29, 2025
209fe0a
chore: add .idea and .devcontainer to .gitignore
Apr 29, 2025
6aa90ce
fix: replace cp command with mv in testCloneRepository for better fil…
ryan-gang Apr 29, 2025
90fee20
refactor: improve testCloneRepository by updating git executable hand…
ryan-gang Apr 29, 2025
7eb2f24
chore: set global git user configuration in your_git.sh
ryan-gang Apr 29, 2025
b1c5d9e
chore: set default branch name to 'main' in your_git.sh
ryan-gang Apr 29, 2025
8a0a2a1
fix: improve logging for git executable movement in testCloneRepository
Apr 29, 2025
ce98ee6
fix: remove unused debug subproject reference
ryan-gang May 20, 2025
448311b
fix: update git executable handling in testCloneRepository to use sud…
May 20, 2025
366d42e
fix: ensure temporary git directory is removed after cloning
May 20, 2025
6754885
fix: improve logging in testCloneRepository and clean up deferred fun…
ryan-gang May 20, 2025
5318018
fix: add logging for git command location in testCloneRepository
ryan-gang May 20, 2025
f8ae2ff
fix: comment out unused git command execution in testCloneRepository
ryan-gang May 20, 2025
eccd922
feat: add support for Rust and Python setup in GitHub Actions workflo…
ryan-gang May 20, 2025
88b1597
fix: comment out git configuration commands in your_git.sh
ryan-gang May 20, 2025
07a50d6
fix: uncomment git configuration commands in your_git.sh
ryan-gang May 20, 2025
87a489c
feat: add test_with_gix step to GitHub Actions workflow
ryan-gang May 20, 2025
5f9d8fc
fix: add export PATH command for cargo binaries in GitHub Actions wor…
ryan-gang May 20, 2025
6d68173
fix: update PATH handling for cargo binaries in GitHub Actions workflow
ryan-gang May 20, 2025
1a8c8b7
fix: add --force flag to cargo binstall for gitoxide
ryan-gang May 20, 2025
06a9732
fix: uncomment cargo binstall command for gitoxide in GitHub Actions …
ryan-gang May 20, 2025
46be12b
fix: add init command handling in your_git.sh script
May 20, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ jobs:
uses: actions/setup-go@v5
with:
go-version: 1.24.x
- name: Set up Rust
uses: actions-rust-lang/setup-rust-toolchain@v1
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.12'
- run: cargo install cargo-binstall
# This sets up the binaries required for gix.
- run: cargo binstall gitoxide --force
- run: make test
- run: make test_with_gix
- run: make test_with_git
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ dist
__pycache__
vendor
.history/
.idea/
.devcontainer/
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ test:

test_with_git: build
CODECRAFTERS_REPOSITORY_DIR=$(shell pwd)/internal/test_helpers/pass_all \
CODECRAFTERS_TEST_CASES_JSON='[{"slug":"gg4","tester_log_prefix":"stage-1","title":"Stage #1: Initialize the .git directory"},{"slug":"ic4","tester_log_prefix":"stage-2","title":"Stage #2: Read a blob object"},{"slug":"jt4","tester_log_prefix":"stage-3","title":"Stage #3: Create a blob object"},{"slug":"kp1","tester_log_prefix":"stage-4","title":"Stage #4: Read a tree object"},{"slug":"fe4","tester_log_prefix":"stage-5","title":"Stage #5: Write a tree object"},{"slug":"jm9","tester_log_prefix":"stage-6","title":"Stage #6: Create a commit"},{"slug":"mg6","tester_log_prefix":"stage-7","title":"Stage #7: Clone a repository"}]' \
CODECRAFTERS_TEST_CASES_JSON='[{"slug":"gg4","tester_log_prefix":"stage-1","title":"Stage #1: Initialize the .git directory"},{"slug":"ic4","tester_log_prefix":"stage-2","title":"Stage #2: Read a blob object"},{"slug":"jt4","tester_log_prefix":"stage-3","title":"Stage #3: Create a blob object"},{"slug":"kp1","tester_log_prefix":"stage-4","title":"Stage #4: Read a tree object"},{"slug":"fe4","tester_log_prefix":"stage-5","title":"Stage #5: Write a tree object"},{"slug":"jm9","tester_log_prefix":"stage-6","title":"Stage #6: Create a commit"}]' \
$(shell pwd)/dist/main.out

test_with_gix: build
CODECRAFTERS_REPOSITORY_DIR=$(shell pwd)/internal/test_helpers/clone \
CODECRAFTERS_TEST_CASES_JSON='[{"slug":"mg6","tester_log_prefix":"stage-7","title":"Stage #7: Clone a repository"}]' \
$(shell pwd)/dist/main.out

copy_course_file:
Expand Down
62 changes: 61 additions & 1 deletion internal/stage_clone_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package internal

import (
"fmt"
"io"
"os"
"os/exec"
"path"

"github.com/codecrafters-io/tester-utils/random"
Expand Down Expand Up @@ -31,7 +33,7 @@ func (r TestRepo) randomFile() TestFile {
return r.exampleFiles[random.RandomInt(0, len(r.exampleFiles))]
}

var testRepos []TestRepo = []TestRepo{
var testRepos = []TestRepo{
{
url: "https://github.com/codecrafters-io/git-sample-1",
exampleCommits: []string{
Expand Down Expand Up @@ -90,6 +92,46 @@ func testCloneRepository(harness *test_case_harness.TestCaseHarness) error {
testRepo := randomRepo()

logger.Infof("$ ./%s clone %s <testDir>", path.Base(executable.Path), testRepo.url)

oldGitPath, err := exec.LookPath("git")
if err != nil {
return fmt.Errorf("git executable not found: %v", err)
}
oldGitDir := path.Dir(oldGitPath)
logger.Debugf("Found git executable at: %s", oldGitPath)

tmpGitDir, err := os.MkdirTemp("/tmp", "git-*")
if err != nil {
return err
}
logger.Debugf("Created temporary directory for git clone: %s", tmpGitDir)
tmpGitPath := path.Join(tmpGitDir, "git")
defer os.RemoveAll(tmpGitDir)

// Copy the custom_executable to the output path
command := fmt.Sprintf("sudo mv %s %s", oldGitPath, tmpGitDir)
logger.Debugf(command)
copyCmd := exec.Command("sh", "-c", command)
copyCmd.Stdout = os.Stdout
copyCmd.Stderr = os.Stderr
if err := copyCmd.Run(); err != nil {
return fmt.Errorf("CodeCrafters Internal Error: mv1 failed: %w", err)
}
logger.Debugf("mv-ed git to temp directory: %s", tmpGitDir)

logger.Infof("$ git clone %s %s", testRepo.url, "test_dir")

//////// which git
// command = fmt.Sprint("which git")
// logger.Debugf(command)
// cmd := exec.Command("sh", "-c", command)
// cmd.Stdout = os.Stdout
// cmd.Stderr = os.Stderr
// if err := cmd.Run(); err != nil {
// return fmt.Errorf("CodeCrafters Internal Error: mv1 failed: %w", err)
// }
// logger.Debugf("mv-ed git to temp directory: %s", tmpGitDir)

result, err := executable.Run("clone", testRepo.url, "test_dir")
if err != nil {
return err
Expand Down Expand Up @@ -136,5 +178,23 @@ func testCloneRepository(harness *test_case_harness.TestCaseHarness) error {
}
logger.Successf("File contents verified")

defer func() error {
// Copy the custom_executable to the output path
command := fmt.Sprintf("sudo mv %s %s", tmpGitPath, oldGitDir)
logger.Debugf(command)
copyCmd := exec.Command("sh", "-c", command)
copyCmd.Stdout = io.Discard
copyCmd.Stderr = io.Discard
if err := copyCmd.Run(); err != nil {
return fmt.Errorf("CodeCrafters Internal Error: mv2 failed: %w", err)
}
logger.Debugf("mv-ed git to og directory: %s", oldGitDir)

if err := os.RemoveAll(tmpGitDir); err != nil {
return fmt.Errorf("CodeCrafters Internal Error: delete directory failed: %s", tmpGitDir)
}
return nil
}()
Comment on lines +181 to +197
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix error handling in deferred function and use Go's file operations

The deferred function has several issues:

  1. It returns an error that isn't captured by the caller
  2. Uses shell commands with sudo, which isn't portable
  3. Contains duplicate code to remove the temporary directory
-defer func() error {
+defer func() {
-	// Copy the custom_executable to the output path
-	command := fmt.Sprintf("sudo mv %s %s", tmpGitPath, oldGitDir)
-	logger.Debugf(command)
-	copyCmd := exec.Command("sh", "-c", command)
-	copyCmd.Stdout = io.Discard
-	copyCmd.Stderr = io.Discard
-	if err := copyCmd.Run(); err != nil {
-		return fmt.Errorf("CodeCrafters Internal Error: mv2 failed: %w", err)
-	}
-	logger.Debugf("mv-ed git to og directory: %s", oldGitDir)
+	// Restore the git executable to its original location
+	originalPath := path.Join(oldGitDir, "git")
+	logger.Debugf("Restoring git executable to: %s", originalPath)
+	
+	// Copy the git executable back to its original location
+	srcFile, err := os.Open(tmpGitPath)
+	if err != nil {
+		logger.Errorf("Failed to open temporary git executable: %v", err)
+		return
+	}
+	defer srcFile.Close()
+	
+	destFile, err := os.Create(originalPath)
+	if err != nil {
+		logger.Errorf("Failed to create original git executable: %v", err)
+		return
+	}
+	defer destFile.Close()
+	
+	if _, err := io.Copy(destFile, srcFile); err != nil {
+		logger.Errorf("Failed to copy git executable back: %v", err)
+		return
+	}
+	
+	if err := os.Chmod(originalPath, 0755); err != nil {
+		logger.Errorf("Failed to make original git executable: %v", err)
+		return
+	}
+	
+	logger.Debugf("Restored git executable to original location: %s", originalPath)

-	if err := os.RemoveAll(tmpGitDir); err != nil {
-		return fmt.Errorf("CodeCrafters Internal Error: delete directory failed: %s", tmpGitDir)
-	}
-	return nil
}()
+	// Verify git is available after restoration
+	if _, err := exec.LookPath("git"); err != nil {
+		logger.Errorf("Git executable was not restored correctly: %v", err)
+	}
+}()

Note: The os.RemoveAll(tmpGitDir) is already called in the earlier defer statement on line 109, so it's removed here to avoid duplication.

🧰 Tools
🪛 golangci-lint (1.64.8)

197-197: Error return value is not checked

(errcheck)

🤖 Prompt for AI Agents
In internal/stage_clone_repository.go around lines 181 to 197, the deferred
function incorrectly returns an error that is not handled, uses a non-portable
shell command with sudo to move files, and duplicates the removal of the
temporary directory already handled by an earlier defer on line 109. To fix
this, remove the error return from the deferred function and handle errors
internally by logging them. Replace the shell command with Go's os.Rename
function to move the directory without sudo for portability. Also, remove the
os.RemoveAll call from this defer to avoid duplicate directory removal.


return nil
}
11 changes: 11 additions & 0 deletions internal/test_helpers/clone/codecrafters.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This is the current stage that you're on.
#
# Whenever you want to advance to the next stage,
# bump this to the next number.
current_stage: 1

# Set this to true if you want debug logs.
#
# These can be VERY verbose, so we suggest turning them off
# unless you really need them.
debug: true
13 changes: 13 additions & 0 deletions internal/test_helpers/clone/your_git.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/sh

if [ "$1" = "init" ]
then
ein init "@$"
fi
Comment on lines +3 to +6
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix argument handling and quoting for the init branch
Using ein init "@$" is incorrect:

  • "@$" is a typo; it should be "$@" to forward all args.
  • You need to shift off the first argument ("init") before invoking ein so that the remaining args are passed correctly.
  • Use exec to replace the shell process and avoid falling through.

Apply this diff:

-if [ "$1" = "init" ]
-then
-  ein init "@$"
-fi
+if [ "$1" = "init" ]; then
+  shift
+  exec ein init "$@"
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$1" = "init" ]
then
ein init "@$"
fi
if [ "$1" = "init" ]; then
shift
exec ein init "$@"
fi
🤖 Prompt for AI Agents
In internal/test_helpers/clone/your_git.sh around lines 3 to 6, fix the argument
handling by replacing the incorrect `"@$"` with `"$@"` to correctly forward all
arguments. Before calling `ein init`, use `shift` to remove the first argument
"init" so that only the remaining arguments are passed. Also, use `exec` to run
`ein init` to replace the current shell process and prevent further script
execution.


if [ "$1" = "cat-file" ]
then
gix cat "@$"
fi
Comment on lines +8 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix argument handling and quoting in wrapper script
The current logic gix cat "@$" is incorrect:

  • @$ is a typo; it should be "$@" to forward all args.
  • You need to shift off the cat-file token and use exec inside the branch to avoid falling through to the default exec.

Proposed diff:

 #!/usr/bin/env sh
-if [ "$1" = "cat-file" ]
-then
-  gix cat "@$"
-fi
+if [ "$1" = "cat-file" ]; then
+  shift
+  exec gix cat "$@"
+fi

 exec gix "$@"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$1" = "cat-file" ]
then
gix cat "@$"
fi
#!/usr/bin/env sh
if [ "$1" = "cat-file" ]; then
shift
exec gix cat "$@"
fi
exec gix "$@"
🤖 Prompt for AI Agents
In internal/test_helpers/clone/your_git.sh around lines 3 to 6, fix the argument
handling by replacing the incorrect `gix cat "@$"` with `exec gix cat "$@"`
after shifting off the first argument to remove the "cat-file" token. This
ensures all remaining arguments are correctly forwarded and the exec prevents
falling through to the default exec later in the script.


exec gix "$@"
4 changes: 4 additions & 0 deletions internal/test_helpers/pass_all/your_git.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/bin/sh

git config --global user.email "[email protected]"
git config --global user.name "Your Name"
Comment on lines +3 to +4
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid mutating the user's global Git config
Running git config --global … here will permanently overwrite ~/.gitconfig for anyone executing these tests. Instead, confine the settings to this invocation or to the repo under test. For example:

- git config --global user.email "[email protected]"
- git config --global user.name "Your Name"
- exec git "$@"
+ # Apply email/name only for this invocation, without touching ~/.gitconfig
+ exec git \
+   -c user.email="[email protected]" \
+   -c user.name="Your Name" \
+   "$@"

This keeps your test setup isolated and avoids side-effects on the developer’s environment.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
git config --global user.email "[email protected]"
git config --global user.name "Your Name"
#!/usr/bin/env bash
# Apply email/name only for this invocation, without touching ~/.gitconfig
exec git \
-c user.email="[email protected]" \
-c user.name="Your Name" \
"$@"

git config --global init.defaultBranch "main"

if [ "$1" = "write-tree" ]
then
git add .
Expand Down