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.

Sign up for GitHub

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 23 commits into
base: main
Choose a base branch
from
Open

Refactor testCloneRepository function and update .gitignore #85

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 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
9d43810
wip: add tmp config change
ryan-gang Jun 23, 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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ dist
__pycache__
vendor
.history/
.idea/
.devcontainer/
52 changes: 51 additions & 1 deletion internal/stage_clone_repository.go
View file
Open in desktop
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,54 @@ func testCloneRepository(harness *test_case_harness.TestCaseHarness) error {
testRepo := randomRepo()

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

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

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

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider edge cases for path handling

Your implementation assumes that the git executable path and temporary directory creation will always work, but there are some edge cases to consider:

  1. The git path might contain spaces or special characters
  2. The temporary directory might not be accessible or writable
  3. The system might not have a /tmp directory (Windows)

Consider adding more robust error handling and making the path handling more resilient:

- gitDir, err := os.MkdirTemp("/tmp", "git-*")
+ gitDir, err := os.MkdirTemp("", "git-*")

And verify the git executable exists and is executable:

🏁 Script executed:

#!/bin/bash
# Check if this code runs on different platforms
uname -a
echo "Testing git path handling..."
which git
git --version

# Check if temp directory handling works
echo "Testing temp directory handling..."
ls -la /tmp 2>/dev/null || echo "No /tmp directory (likely Windows)"
echo %TEMP% || echo "Not on Windows"

Length of output: 748

Use os.MkdirTemp’s default temp directory for portability

The hard-coded "/tmp" path isn’t portable (e.g. Windows), so switch to the system default:

• File: internal/stage_clone_repository.go, around line 100

- gitDir, err := os.MkdirTemp("/tmp", "git-*")
+ gitDir, err := os.MkdirTemp("", "git-*")

The existing exec.LookPath("git") call already guarantees the git executable is found and executable, so no further checks are needed here.

📝 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
gitPath, err := exec.LookPath("git")
if err != nil {
return fmt.Errorf("git executable not found: %v", err)
}
logger.Debugf("Found git executable at: %s", gitPath)
gitDir, err := os.MkdirTemp("/tmp", "git-*")
if err != nil {
return err
}
logger.Debugf("Created temporary directory for git clone: %s", gitDir)
defer os.RemoveAll(gitDir)
gitPath, err := exec.LookPath("git")
if err != nil {
return fmt.Errorf("git executable not found: %v", err)
}
logger.Debugf("Found git executable at: %s", gitPath)
gitDir, err := os.MkdirTemp("", "git-*")
if err != nil {
return err
}
logger.Debugf("Created temporary directory for git clone: %s", gitDir)
defer os.RemoveAll(gitDir)


// Copy the custom_executable to the output path
command := fmt.Sprintf("cp %s %s", gitPath, gitDir)
logger.Debugf("Cp-ed git to temp directory: %s", gitDir)
//fmt.Println(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: cp failed: %w", err)
}

defer func() error {
// Copy the custom_executable to the output path
command := fmt.Sprintf("cp %s %s", gitDir, gitPath)
logger.Debugf("Cp-ed git to temp directory: %s", gitPath)
//fmt.Println(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: cp failed: %w", err)
}

return nil
}()
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 the directory-to-file copy issue and error handling

There are a few issues with this deferred function:

  1. It's trying to copy a directory to a file path (cp %s %s", gitDir, gitPath) which won't work as expected
  2. The error returned by the deferred function is not being captured
  3. The log message doesn't accurately describe what's happening
 defer func() error {
-  // Copy the custom_executable to the output path
-  command := fmt.Sprintf("cp %s %s", gitDir, gitPath)
-  logger.Debugf("Cp-ed git to temp directory: %s", gitPath)
+  // Restore the git executable from the temporary directory
+  command := fmt.Sprintf("cp %s/git %s", gitDir, gitPath)
+  logger.Debugf("Restoring git executable to original path: %s", gitPath)
   //fmt.Println(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: cp failed: %w", err)
+    logger.Errorf("Failed to restore git executable: %v", err)
+    return err
   }

   return nil
-}()
+}

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 golangci-lint (1.64.8)

134-134: Error return value is not checked

(errcheck)


defer func() error {
if err := os.RemoveAll(gitDir); err != nil {
return fmt.Errorf("CodeCrafters Internal Error: delete directory failed: %s", gitDir)
}
return nil
}()

result, err := executable.Run("clone", testRepo.url, "test_dir")
if err != nil {
return err
Expand Down