From 3257b57c746700b0cadd0ee5927adb5e27a1c607 Mon Sep 17 00:00:00 2001 From: Alisher Nurmanov Date: Thu, 4 Sep 2025 05:17:08 +0500 Subject: [PATCH 1/3] bug/`qs pr` failed if had large files on `qs dev` --- gitcmds/const.go | 28 +++++++++++++++++++--------- gitcmds/gitcmds.go | 26 +++++++++++--------------- gitcmds/pr.go | 6 +++--- internal/commands/consts.go | 4 ++++ internal/commands/dev.go | 11 ++++++----- internal/commands/u.go | 2 +- 6 files changed, 44 insertions(+), 33 deletions(-) diff --git a/gitcmds/const.go b/gitcmds/const.go index 7036344..bad0474 100644 --- a/gitcmds/const.go +++ b/gitcmds/const.go @@ -7,27 +7,37 @@ const ( errMsgFailedToGetMainBranch = "failed to get main branch: %w" countOfZerosIn1000 = 3 decimalBase = 10 - bitSizeOfInt64 = 64 + refsNotes = "refs/notes/*:refs/notes/*" ) const largeFileHookContent = ` #!/bin/bash totalsize=0 totalcnt=0 -readarray -t arr2 < <(git status --porcelain | awk '{if ($1 == "??" || $1 == "A") print $2}') -for row in "${arr2[@]}";do - extension="${row##*.}" - if [ "$extension" != "wasm" ]; then - fs=$(wc -c $row | awk '{print $1}') - totalsize=$(($totalsize+$fs)) - totalcnt=$(($totalcnt+1)) + +# Use -z to get NUL-separated records: "XYpath\0" +while IFS= read -r -d '' entry; do + status="${entry:0:2}" # e.g., "??", "A ", "AM", etc. + path="${entry:3}" # skip "XY " to get the path + + # consider untracked (??) and added (A*) files + if [[ "$status" == "??" || "$status" == A* ]]; then + extension="${path##*.}" + if [[ "$extension" != "wasm" ]]; then + # wc -c < file gives just the count; quote the path + fs=$(wc -c < "$path") + totalsize=$((totalsize + fs)) + totalcnt=$((totalcnt + 1)) + fi fi -done +done < <(git status --porcelain -z) + if (( $totalsize > 100000 )); then echo " Attempt to commit too large files: Files size = $totalsize" exit 1 fi + if (( $totalcnt > 200 )); then echo " Attempt to commit too much files: Files number = $totalcnt" exit 1 diff --git a/gitcmds/gitcmds.go b/gitcmds/gitcmds.go index 1f83fab..f493ec6 100644 --- a/gitcmds/gitcmds.go +++ b/gitcmds/gitcmds.go @@ -40,8 +40,6 @@ const ( originSlash = "origin/" httppref = "https" pushYes = "y" - nochecksmsg = "no checks reported" - msgWaitingPR = "Waiting PR checks.." MsgPreCommitError = "Attempt to commit too" MsgCommitForNotes = "Commit for keeping notes in branch" oneSpace = " " @@ -51,14 +49,11 @@ const ( userNotFound = "git user name not found" ErrAlreadyForkedMsg = "you are in fork already\nExecute 'qs dev [branch name]' to create dev branch" ErrMsgPRNotesImpossible = "pull request without comments is impossible" - ErrTimer40Sec = "time out 40 seconds" - ErrSomethingWrong = "something went wrong" DefaultCommitMessage = "wip" IssuePRTtilePrefix = "Resolves issue" IssueSign = "Resolves #" - prTimeWait = 40 minIssueNoteLength = 10 minPRTitleLength = 8 minRepoNameLength = 4 @@ -690,7 +685,7 @@ func Upload(cmd *cobra.Command, wd string) error { // Push notes to origin err = helper.Retry(func() error { stdout, stderr, err = new(exec.PipedExec). - Command(git, push, origin, "refs/notes/*:refs/notes/*"). + Command(git, push, origin, refsNotes). WorkingDir(wd). RunToStrings() if err != nil { @@ -855,7 +850,7 @@ func Download(wd string) error { // Step 3: git fetch origin --force refs/notes/*:refs/notes/* err = helper.Retry(func() error { stdout, stderr, err = new(exec.PipedExec). - Command(git, fetch, origin, "--force", "refs/notes/*:refs/notes/*"). + Command(git, fetch, origin, "--force", refsNotes). WorkingDir(wd). RunToStrings() if err != nil { @@ -1483,8 +1478,8 @@ func GetIssueRepoFromURL(url string) (repoName string) { return } -// DevIssue create link between upstream Guthub issue and dev branch -func DevIssue(wd, parentRepo, githubIssueURL string, issueNumber int, args ...string) (branch string, notes []string, err error) { +// CreateGithubLinkToIssue create a link between an upstream GitHub issue and the dev branch +func CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL string, issueNumber int, args ...string) (branch string, notes []string, err error) { repo, org, err := GetRepoAndOrgName(wd) if err != nil { return "", nil, fmt.Errorf("GetRepoAndOrgName failed: %w", err) @@ -1805,12 +1800,12 @@ func GetIssueNameByNumber(issueNum string, parentrepo string) (string, error) { return strings.TrimSpace(stdout), nil } -// Dev creates dev branch and pushes it to origin +// CreateDevBranch creates dev branch and pushes it to origin // Parameters: // branch - branch name // notes - notes for branch -// checkRemoteBranchExistence - if true, checks if branch already exists in remote -func Dev(wd, branchName string, notes []string, checkRemoteBranchExistence bool) error { +// checkRemoteBranchExistence - if true, checks if a branch already exists in remote +func CreateDevBranch(wd, branchName string, notes []string, checkRemoteBranchExistence bool) error { mainBranch, err := GetMainBranch(wd) if err != nil { return fmt.Errorf(errMsgFailedToGetMainBranch, err) @@ -1837,7 +1832,7 @@ func Dev(wd, branchName string, notes []string, checkRemoteBranchExistence bool) } if checkRemoteBranchExistence { - // check if branch already exists in remote + // check if a branch already exists in remote stdout, stderr, err := new(exec.PipedExec). Command(git, "ls-remote", "--heads", "origin", branchName). WorkingDir(wd). @@ -1854,6 +1849,7 @@ func Dev(wd, branchName string, notes []string, checkRemoteBranchExistence bool) } } + // Create new branch from main err = new(exec.PipedExec). Command(git, "checkout", "-B", branchName). WorkingDir(wd). @@ -1864,7 +1860,7 @@ func Dev(wd, branchName string, notes []string, checkRemoteBranchExistence bool) // Fetch notes from origin before pushing stdout, stderr, err = new(exec.PipedExec). - Command(git, fetch, "--force", origin, "refs/notes/*:refs/notes/*"). + Command(git, fetch, origin, "--force", refsNotes). WorkingDir(wd). RunToStrings() if err != nil { @@ -1889,7 +1885,7 @@ func Dev(wd, branchName string, notes []string, checkRemoteBranchExistence bool) // Push notes to origin with retry err = helper.Retry(func() error { stdout, stderr, err = new(exec.PipedExec). - Command(git, push, origin, "refs/notes/*:refs/notes/*"). + Command(git, push, origin, refsNotes). WorkingDir(wd). RunToStrings() diff --git a/gitcmds/pr.go b/gitcmds/pr.go index 7d07b6e..cfb42a6 100644 --- a/gitcmds/pr.go +++ b/gitcmds/pr.go @@ -45,7 +45,7 @@ func Pr(wd string, needDraft bool) error { if branchType == notesPkg.BranchTypeDev { // Fetch notes from origin before checking if they exist _, _, err := new(exec.PipedExec). - Command(git, fetch, origin, "--force", "refs/notes/*:refs/notes/*"). + Command(git, fetch, origin, "--force", refsNotes). WorkingDir(wd). RunToStrings() if err != nil { @@ -134,7 +134,7 @@ func pushPRBranch(wd, prBranchName string) error { // Push notes to origin err := helper.Retry(func() error { _, stderr, err := new(exec.PipedExec). - Command("git", "push", "origin", "refs/notes/*:refs/notes/*"). + Command(git, push, origin, refsNotes). WorkingDir(wd). RunToStrings() if err != nil { @@ -354,7 +354,7 @@ func createPRBranch(wd, devBranchName string) (string, error) { // Step 1.1: Fetch notes from origin err = helper.Retry(func() error { stdout, stderr, err = new(exec.PipedExec). - Command("git", "fetch", "origin", "--force", "refs/notes/*:refs/notes/*"). + Command(git, fetch, origin, "--force", refsNotes). WorkingDir(wd). RunToStrings() if err != nil { diff --git a/internal/commands/consts.go b/internal/commands/consts.go index e771cc8..9bf4fdf 100644 --- a/internal/commands/consts.go +++ b/internal/commands/consts.go @@ -18,6 +18,10 @@ const ( oneSpace = " " EnvSkipQsVersionCheck = "QS_SKIP_QS_VERSION_CHECK" minimumCommitMessageLen = 8 + fetch = "fetch" + origin = "origin" + git = "git" + refsNotes = "refs/notes/*:refs/notes/*" ) const ( diff --git a/internal/commands/dev.go b/internal/commands/dev.go index 660a1b0..5b02693 100644 --- a/internal/commands/dev.go +++ b/internal/commands/dev.go @@ -13,8 +13,9 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/spf13/cobra" "github.com/untillpro/goutils/logger" + "github.com/untillpro/qs/gitcmds" - contextPkg "github.com/untillpro/qs/internal/context" + contextpkg "github.com/untillpro/qs/internal/context" "github.com/untillpro/qs/internal/helper" "github.com/untillpro/qs/internal/jira" "github.com/untillpro/qs/internal/notes" @@ -101,7 +102,7 @@ func Dev(cmd *cobra.Command, wd string, args []string) error { fmt.Print("Dev branch for issue #" + strconv.Itoa(issueNum) + " will be created. Agree?(y/n)") _, _ = fmt.Scanln(&response) if response == pushYes { - branch, notes, err = gitcmds.DevIssue(wd, parentRepo, githubIssueURL, issueNum, args...) + branch, notes, err = gitcmds.CreateGithubLinkToIssue(wd, parentRepo, githubIssueURL, issueNum, args...) if err != nil { return err } @@ -124,7 +125,7 @@ func Dev(cmd *cobra.Command, wd string, args []string) error { } // put branch name to command context - cmd.SetContext(context.WithValue(cmd.Context(), contextPkg.CtxKeyDevBranchName, branch)) + cmd.SetContext(context.WithValue(cmd.Context(), contextpkg.CtxKeyDevBranchName, branch)) exists, err := branchExists(wd, branch) if err != nil { @@ -153,7 +154,7 @@ func Dev(cmd *cobra.Command, wd string, args []string) error { } } - if err := gitcmds.Dev(wd, branch, notes, checkRemoteBranchExistence); err != nil { + if err := gitcmds.CreateDevBranch(wd, branch, notes, checkRemoteBranchExistence); err != nil { return err } default: @@ -208,7 +209,7 @@ func branchExists(wd, branchName string) (bool, error) { func getArgStringFromClipboard(ctx context.Context) string { var err error // context value is first - arg, ok := ctx.Value(contextPkg.CtxKeyClipboard).(string) + arg, ok := ctx.Value(contextpkg.CtxKeyClipboard).(string) if !ok || len(arg) == 0 { arg, err = clipboard.ReadAll() if err != nil { diff --git a/internal/commands/u.go b/internal/commands/u.go index 2fe115c..83ef7c6 100644 --- a/internal/commands/u.go +++ b/internal/commands/u.go @@ -30,7 +30,7 @@ func U(cmd *cobra.Command, cfgUpload vcs.CfgUpload, wd string) error { // Fetch notes from origin _, _, err = new(exec.PipedExec). - Command("git", "fetch", "origin", "--force", "refs/notes/*:refs/notes/*"). + Command(git, fetch, origin, "--force", refsNotes). WorkingDir(wd). RunToStrings() if err != nil { From 2043578882b0e343a2d218f80aa691f60371963d Mon Sep 17 00:00:00 2001 From: Alisher Nurmanov Date: Thu, 4 Sep 2025 17:11:00 +0500 Subject: [PATCH 2/3] bug/ensure large file hook is up to date during pre-commit --- gitcmds/const.go | 6 ++-- gitcmds/gitcmds.go | 67 +++++++++++++++++++++++++++++++++++++-- internal/commands/dev.go | 5 +++ internal/commands/u.go | 5 +++ internal/systrun/types.go | 2 +- 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/gitcmds/const.go b/gitcmds/const.go index bad0474..335b278 100644 --- a/gitcmds/const.go +++ b/gitcmds/const.go @@ -7,8 +7,10 @@ const ( errMsgFailedToGetMainBranch = "failed to get main branch: %w" countOfZerosIn1000 = 3 decimalBase = 10 - bitSizeOfInt64 = 64 - refsNotes = "refs/notes/*:refs/notes/*" + + bitSizeOfInt64 = 64 + refsNotes = "refs/notes/*:refs/notes/*" + LargeFileHookFilename = "large-file-hook.sh" ) const largeFileHookContent = ` diff --git a/gitcmds/gitcmds.go b/gitcmds/gitcmds.go index f493ec6..d50ad50 100644 --- a/gitcmds/gitcmds.go +++ b/gitcmds/gitcmds.go @@ -2551,7 +2551,7 @@ func LocalPreCommitHookExist(wd string) (bool, error) { } func largeFileHookExist(filepath string) bool { - substring := "large-file-hook.sh" + substring := LargeFileHookFilename _, _, err := new(exec.PipedExec).Command("grep", "-l", substring, filepath).RunToStrings() return err == nil @@ -2689,7 +2689,7 @@ func fillPreCommitFile(wd, myFilePath string) error { if err != nil { return err } - fName := "/.git/hooks/large-file-hook.sh" + fName := "/.git/hooks/" + LargeFileHookFilename lfPath := dir + fName lf, err := os.Create(lfPath) @@ -2715,6 +2715,69 @@ func fillPreCommitFile(wd, myFilePath string) error { return new(exec.PipedExec).Command("chmod", "+x", myFilePath).Run(os.Stdout, os.Stdout) } +// isLargeFileHookContentUpToDate checks if the current large-file-hook.sh content matches the expected content +func isLargeFileHookContentUpToDate(wd string) (bool, error) { + dir, err := GetRootFolder(wd) + if err != nil { + return false, err + } + + hookPath := filepath.Join(dir, ".git", "hooks", LargeFileHookFilename) + + // Check if the file exists + if _, err := os.Stat(hookPath); os.IsNotExist(err) { + return false, nil // File doesn't exist, so it's not up to date + } + + // Read the current content + currentContent, err := os.ReadFile(hookPath) + if err != nil { + return false, fmt.Errorf("failed to read large file hook: %w", err) + } + + // Compare with expected content + return string(currentContent) == largeFileHookContent, nil +} + +// updateLargeFileHookContent updates the large-file-hook.sh file with the current content +func updateLargeFileHookContent(wd string) error { + dir, err := GetRootFolder(wd) + if err != nil { + return err + } + + hookPath := filepath.Join(dir, ".git", "hooks", LargeFileHookFilename) + + // Create or overwrite the hook file + lf, err := os.Create(hookPath) + if err != nil { + return fmt.Errorf("failed to create large file hook: %w", err) + } + defer func() { + _ = lf.Close() + }() + + if _, err := lf.WriteString(largeFileHookContent); err != nil { + return fmt.Errorf("failed to write large file hook content: %w", err) + } + + return nil +} + +// EnsureLargeFileHookUpToDate checks and updates the large file hook if needed +func EnsureLargeFileHookUpToDate(wd string) error { + upToDate, err := isLargeFileHookContentUpToDate(wd) + if err != nil { + return err + } + + if !upToDate { + return updateLargeFileHookContent(wd) + } + + return nil +} + func UpstreamNotExist(wd string) bool { return len(getRemotes(wd)) < 2 } diff --git a/internal/commands/dev.go b/internal/commands/dev.go index 5b02693..bbc7737 100644 --- a/internal/commands/dev.go +++ b/internal/commands/dev.go @@ -167,6 +167,11 @@ func Dev(cmd *cobra.Command, wd string, args []string) error { if err := setPreCommitHook(wd); err != nil { logger.Verbose("Error setting pre-commit hook:", err) } + + // Ensure large file hook content is up to date + if err := gitcmds.EnsureLargeFileHookUpToDate(wd); err != nil { + logger.Verbose("Error updating large file hook content:", err) + } // Unstash changes if stashedUncommittedChanges { if err := gitcmds.Unstash(wd); err != nil { diff --git a/internal/commands/u.go b/internal/commands/u.go index 83ef7c6..f908975 100644 --- a/internal/commands/u.go +++ b/internal/commands/u.go @@ -47,6 +47,11 @@ func U(cmd *cobra.Command, cfgUpload vcs.CfgUpload, wd string) error { return err } + // Ensure large file hook content is up to date + if err := gitcmds.EnsureLargeFileHookUpToDate(wd); err != nil { + logger.Verbose("Error updating large file hook content:", err) + } + return gitcmds.Upload(cmd, wd) } diff --git a/internal/systrun/types.go b/internal/systrun/types.go index c3678b9..71ff6c6 100644 --- a/internal/systrun/types.go +++ b/internal/systrun/types.go @@ -202,7 +202,7 @@ func ExpectationLargeFileHooksInstalled(ctx context.Context) error { return fmt.Errorf("pre-commit hook is not installed at %s", hookPath) } - substring := "large-file-hook.sh" + substring := gitcmds.LargeFileHookFilename stdout, _, err := new(goUtilsExec.PipedExec). Command("grep", "-l", substring, hookPath). RunToStrings() From a9edc95b285098c8b1d24f2f10462501ce36af32 Mon Sep 17 00:00:00 2001 From: Alisher Nurmanov Date: Thu, 4 Sep 2025 17:24:03 +0500 Subject: [PATCH 3/3] bug/test large file hook functionality and ensure it prevents large file commits --- internal/systrun/types.go | 61 +++++++++++++++++++++++++++++++++++++++ sys_test.go | 28 ++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/internal/systrun/types.go b/internal/systrun/types.go index 71ff6c6..bebb726 100644 --- a/internal/systrun/types.go +++ b/internal/systrun/types.go @@ -217,6 +217,67 @@ func ExpectationLargeFileHooksInstalled(ctx context.Context) error { return nil } +// ExpectationLargeFileHookFunctional tests that the large file hook actually prevents commits of large files +func ExpectationLargeFileHookFunctional(ctx context.Context) error { + cloneRepoPath := ctx.Value(contextCfg.CtxKeyCloneRepoPath).(string) + if cloneRepoPath == "" { + return errCloneRepoPathNotFoundInContext + } + + // First, ensure the hook is installed + if err := ExpectationLargeFileHooksInstalled(ctx); err != nil { + return fmt.Errorf("large file hook not installed: %w", err) + } + + // Create a large test file (over 100KB) + largeFilePath := filepath.Join(cloneRepoPath, "large_test_file.txt") + largeContent := strings.Repeat("This is a test line to create a large file.\n", 3000) // ~120KB + + if err := os.WriteFile(largeFilePath, []byte(largeContent), 0644); err != nil { + return fmt.Errorf("failed to create large test file: %w", err) + } + + // Clean up the test file at the end + defer func() { + _ = os.Remove(largeFilePath) + }() + + // Add the large file to git + _, stderr, err := new(goUtilsExec.PipedExec). + Command("git", "add", "large_test_file.txt"). + WorkingDir(cloneRepoPath). + RunToStrings() + if err != nil { + return fmt.Errorf("failed to add large file to git: %w, stderr: %s", err, stderr) + } + + // Try to commit - this should fail due to the large file hook + _, stderr, err = new(goUtilsExec.PipedExec). + Command("git", "commit", "-m", "Test commit with large file"). + WorkingDir(cloneRepoPath). + RunToStrings() + + // The commit should fail (err != nil) and stderr should contain the large file message + if err == nil { + return fmt.Errorf("expected commit to fail due to large file hook, but it succeeded") + } + + if !strings.Contains(stderr, "Attempt to commit too large files") { + return fmt.Errorf("expected large file hook error message, but got: %s", stderr) + } + + // Reset the git state to clean up + _, _, resetErr := new(goUtilsExec.PipedExec). + Command("git", "reset", "--hard", "HEAD"). + WorkingDir(cloneRepoPath). + RunToStrings() + if resetErr != nil { + return fmt.Errorf("failed to reset git state: %w", resetErr) + } + + return nil +} + // ExpectationCurrentBranchHasPrefix checks if the current branch has the expected prefix func ExpectationCurrentBranchHasPrefix(ctx context.Context) error { // Open the repository diff --git a/sys_test.go b/sys_test.go index 5e59eb2..9756c6d 100644 --- a/sys_test.go +++ b/sys_test.go @@ -738,6 +738,34 @@ func TestQS(t *testing.T) { require.NoError(err) } +// TestLargeFileHook tests that the large file precommit hook is functional +func TestLargeFileHook(t *testing.T) { + require := require.New(t) + + testConfig := &systrun.TestConfig{ + TestID: strings.ToLower(t.Name()), + GHConfig: getGithubConfig(t), + CommandConfig: &systrun.CommandConfig{ + Command: "dev", + Args: []string{"--no-fork"}, + Stdin: "y", + }, + UpstreamState: systrun.RemoteStateOK, + ForkState: systrun.RemoteStateNull, + ClipboardContent: systrun.ClipboardContentGithubIssue, + NeedCollaboration: true, + Expectations: []systrun.ExpectationFunc{ + systrun.ExpectationBranchLinkedToIssue, + systrun.ExpectationLargeFileHooksInstalled, + systrun.ExpectationLargeFileHookFunctional, + }, + } + + sysTest := systrun.New(t, testConfig) + err := sysTest.Run() + require.NoError(err) +} + // getGithubConfig retrieves GitHub credentials from environment variables // and skips the test if any credentials are missing func getGithubConfig(t *testing.T) systrun.GithubConfig {