From 8f1e238f89b73b5abef3a64fa33a5c73c6fb8f9d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 6 Oct 2021 22:17:18 +0100 Subject: [PATCH 1/4] Ensure that git daemon export ok is created for mirrors There is an issue with #16508 where it appears that create repo requires that the repo does not exist. This causes #17241 where an error is reported because of this. This PR fixes this and also runs update-server-info for mirrors and generated repos. Fix #17241 Signed-off-by: Andrew Thornton --- models/repo.go | 52 +++++++++++++++++----------------- modules/repository/adopt.go | 3 ++ modules/repository/create.go | 4 +++ modules/repository/fork.go | 4 +++ modules/repository/generate.go | 11 +++++++ modules/repository/repo.go | 11 +++++++ 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/models/repo.go b/models/repo.go index efd78c6042d69..9d466f3a7e36f 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1146,16 +1146,6 @@ func CreateRepository(ctx context.Context, doer, u *User, repo *Repository, over return fmt.Errorf("recalculateAccesses: %v", err) } - if u.Visibility == api.VisibleTypePublic && !repo.IsPrivate { - // Create/Remove git-daemon-export-ok for git-daemon... - daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) - if f, err := os.Create(daemonExportFile); err != nil { - log.Error("Failed to create %s: %v", daemonExportFile, err) - } else { - f.Close() - } - } - if setting.Service.AutoWatchNewRepos { if err = watchRepo(db.GetEngine(ctx), doer.ID, repo.ID, true); err != nil { return fmt.Errorf("watchRepo: %v", err) @@ -1169,6 +1159,31 @@ func CreateRepository(ctx context.Context, doer, u *User, repo *Repository, over return nil } +// CheckDaemonExportOK creates/removes git-daemon-export-ok for git-daemon... +func (repo *Repository) CheckDaemonExportOK() error { + // Create/Remove git-daemon-export-ok for git-daemon... + daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) + isExist, err := util.IsExist(daemonExportFile) + isPublic := !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePublic + if err != nil { + log.Error("Unable to check if %s exists. Error: %v", daemonExportFile, err) + return err + } + if !isPublic && isExist { + if err = util.Remove(daemonExportFile); err != nil { + log.Error("Failed to remove %s: %v", daemonExportFile, err) + } + } else if isPublic && !isExist { + if f, err := os.Create(daemonExportFile); err != nil { + log.Error("Failed to create %s: %v", daemonExportFile, err) + } else { + f.Close() + } + } + + return nil +} + func countRepositories(userID int64, private bool) int64 { sess := db.GetEngine(db.DefaultContext).Where("id > 0") @@ -1318,24 +1333,9 @@ func updateRepository(e db.Engine, repo *Repository, visibilityChanged bool) (er } // Create/Remove git-daemon-export-ok for git-daemon... - daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) - isExist, err := util.IsExist(daemonExportFile) - isPublic := !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePublic - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", daemonExportFile, err) + if err := repo.CheckDaemonExportOK(); err != nil { return err } - if !isPublic && isExist { - if err = util.Remove(daemonExportFile); err != nil { - log.Error("Failed to remove %s: %v", daemonExportFile, err) - } - } else if isPublic && !isExist { - if f, err := os.Create(daemonExportFile); err != nil { - log.Error("Failed to create %s: %v", daemonExportFile, err) - } else { - f.Close() - } - } forkRepos, err := getRepositoriesByForkID(e, repo.ID) if err != nil { diff --git a/modules/repository/adopt.go b/modules/repository/adopt.go index c5c059f4718b4..07b8d17bc38ef 100644 --- a/modules/repository/adopt.go +++ b/modules/repository/adopt.go @@ -68,6 +68,9 @@ func AdoptRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mode if err := adoptRepository(ctx, repoPath, doer, repo, opts); err != nil { return fmt.Errorf("createDelegateHooks: %v", err) } + if err := repo.CheckDaemonExportOK(); err != nil { + return fmt.Errorf("checkDaemonExportOK: %v", err) + } // Initialize Issue Labels if selected if len(opts.IssueLabels) > 0 { diff --git a/modules/repository/create.go b/modules/repository/create.go index 0e91a73b8359c..4c1851255a276 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -105,6 +105,10 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod } } + if err := repo.CheckDaemonExportOK(); err != nil { + return fmt.Errorf("checkDaemonExportOK: %v", err) + } + if stdout, err := git.NewCommand("update-server-info"). SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)). RunInDir(repoPath); err != nil { diff --git a/modules/repository/fork.go b/modules/repository/fork.go index 59c07271a646f..6bd0ea3f8fb35 100644 --- a/modules/repository/fork.go +++ b/modules/repository/fork.go @@ -105,6 +105,10 @@ func ForkRepository(doer, owner *models.User, opts models.ForkRepoOptions) (_ *m return fmt.Errorf("git clone: %v", err) } + if err := repo.CheckDaemonExportOK(); err != nil { + return fmt.Errorf("checkDaemonExportOK: %v", err) + } + if stdout, err := git.NewCommand("update-server-info"). SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())). RunInDir(repoPath); err != nil { diff --git a/modules/repository/generate.go b/modules/repository/generate.go index 4fcebc06dc3ea..47bc70734afc9 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -275,5 +275,16 @@ func GenerateRepository(ctx context.Context, doer, owner *models.User, templateR return generateRepo, err } + if err = generateRepo.CheckDaemonExportOK(); err != nil { + return generateRepo, fmt.Errorf("checkDaemonExportOK: %v", err) + } + + if stdout, err := git.NewCommand("update-server-info"). + SetDescription(fmt.Sprintf("GenerateRepository(git update-server-info): %s", repoPath)). + RunInDir(repoPath); err != nil { + log.Error("GenerateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", generateRepo, stdout, err) + return generateRepo, fmt.Errorf("error in GenerateRepository(git update-server-info): %v", err) + } + return generateRepo, nil } diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 6b40a894fb8b2..bde1d7adc4ba6 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -96,6 +96,17 @@ func MigrateRepositoryGitData(ctx context.Context, u *models.User, repo *models. } } + if err = repo.CheckDaemonExportOK(); err != nil { + return repo, fmt.Errorf("checkDaemonExportOK: %v", err) + } + + if stdout, err := git.NewCommand("update-server-info"). + SetDescription(fmt.Sprintf("MigrateRepositoryGitData(git update-server-info): %s", repoPath)). + RunInDir(repoPath); err != nil { + log.Error("MigrateRepositoryGitData(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) + return repo, fmt.Errorf("error in MigrateRepositoryGitData(git update-server-info): %v", err) + } + gitRepo, err := git.OpenRepository(repoPath) if err != nil { return repo, fmt.Errorf("OpenRepository: %v", err) From 1443f8c8aa41fee465a4218fff818621a9c7255f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 8 Oct 2021 17:57:53 +0100 Subject: [PATCH 2/4] as per lafriks Signed-off-by: Andrew Thornton --- models/repo.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/repo.go b/models/repo.go index 9d466f3a7e36f..2eba0855d486b 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1163,12 +1163,14 @@ func CreateRepository(ctx context.Context, doer, u *User, repo *Repository, over func (repo *Repository) CheckDaemonExportOK() error { // Create/Remove git-daemon-export-ok for git-daemon... daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) + isExist, err := util.IsExist(daemonExportFile) - isPublic := !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePublic if err != nil { log.Error("Unable to check if %s exists. Error: %v", daemonExportFile, err) return err } + + isPublic := !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePublic if !isPublic && isExist { if err = util.Remove(daemonExportFile); err != nil { log.Error("Failed to remove %s: %v", daemonExportFile, err) From 5aeb29fa2169a6ff429f9353737a9d5b3910cb5f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 8 Oct 2021 18:50:00 +0100 Subject: [PATCH 3/4] prevent panic Signed-off-by: Andrew Thornton --- modules/repository/repo.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index bde1d7adc4ba6..6eafe97634b32 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -96,6 +96,14 @@ func MigrateRepositoryGitData(ctx context.Context, u *models.User, repo *models. } } + if repo.OwnerID == u.ID { + repo.Owner = u + } + + if err = repo.GetOwner(); err != nil { + return repo, fmt.Errorf("unable to get owner[%d] for repo [%d]. Error: %v", repo.OwnerID, repo.ID, err) + } + if err = repo.CheckDaemonExportOK(); err != nil { return repo, fmt.Errorf("checkDaemonExportOK: %v", err) } From af602d5a205366099d1176e3ddf0a8d63e6b1733 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 10 Oct 2021 16:06:10 +0100 Subject: [PATCH 4/4] Make CheckDaemonExport load the repo if necessary Signed-off-by: Andrew Thornton --- models/db/context.go | 8 ++++++++ models/repo.go | 9 +++++++-- modules/repository/adopt.go | 2 +- modules/repository/create.go | 4 ++-- modules/repository/fork.go | 6 +++--- modules/repository/generate.go | 4 ++-- modules/repository/repo.go | 8 ++------ 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/models/db/context.go b/models/db/context.go index 0037bb198ddee..62b77bc72fdf9 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -31,6 +31,14 @@ type Context struct { e Engine } +// WithEngine returns a db.Context from a context.Context and db.Engine +func WithEngine(ctx context.Context, e Engine) *Context { + return &Context{ + Context: ctx, + e: e, + } +} + // Engine returns db engine func (ctx *Context) Engine() Engine { return ctx.e diff --git a/models/repo.go b/models/repo.go index 2eba0855d486b..25b1e65206f77 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1160,7 +1160,12 @@ func CreateRepository(ctx context.Context, doer, u *User, repo *Repository, over } // CheckDaemonExportOK creates/removes git-daemon-export-ok for git-daemon... -func (repo *Repository) CheckDaemonExportOK() error { +func (repo *Repository) CheckDaemonExportOK(ctx context.Context) error { + e := db.GetEngine(ctx) + if err := repo.getOwner(e); err != nil { + return err + } + // Create/Remove git-daemon-export-ok for git-daemon... daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) @@ -1335,7 +1340,7 @@ func updateRepository(e db.Engine, repo *Repository, visibilityChanged bool) (er } // Create/Remove git-daemon-export-ok for git-daemon... - if err := repo.CheckDaemonExportOK(); err != nil { + if err := repo.CheckDaemonExportOK(db.WithEngine(db.DefaultContext, e)); err != nil { return err } diff --git a/modules/repository/adopt.go b/modules/repository/adopt.go index 07b8d17bc38ef..21477ab7d771b 100644 --- a/modules/repository/adopt.go +++ b/modules/repository/adopt.go @@ -68,7 +68,7 @@ func AdoptRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mode if err := adoptRepository(ctx, repoPath, doer, repo, opts); err != nil { return fmt.Errorf("createDelegateHooks: %v", err) } - if err := repo.CheckDaemonExportOK(); err != nil { + if err := repo.CheckDaemonExportOK(ctx); err != nil { return fmt.Errorf("checkDaemonExportOK: %v", err) } diff --git a/modules/repository/create.go b/modules/repository/create.go index 4c1851255a276..64d92eeb2d385 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -105,11 +105,11 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (*mod } } - if err := repo.CheckDaemonExportOK(); err != nil { + if err := repo.CheckDaemonExportOK(ctx); err != nil { return fmt.Errorf("checkDaemonExportOK: %v", err) } - if stdout, err := git.NewCommand("update-server-info"). + if stdout, err := git.NewCommandContext(ctx, "update-server-info"). SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)). RunInDir(repoPath); err != nil { log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) diff --git a/modules/repository/fork.go b/modules/repository/fork.go index 6bd0ea3f8fb35..df1dccc596c5f 100644 --- a/modules/repository/fork.go +++ b/modules/repository/fork.go @@ -97,7 +97,7 @@ func ForkRepository(doer, owner *models.User, opts models.ForkRepoOptions) (_ *m needsRollback = true repoPath := models.RepoPath(owner.Name, repo.Name) - if stdout, err := git.NewCommand( + if stdout, err := git.NewCommandContext(ctx, "clone", "--bare", oldRepoPath, repoPath). SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())). RunInDirTimeout(10*time.Minute, ""); err != nil { @@ -105,11 +105,11 @@ func ForkRepository(doer, owner *models.User, opts models.ForkRepoOptions) (_ *m return fmt.Errorf("git clone: %v", err) } - if err := repo.CheckDaemonExportOK(); err != nil { + if err := repo.CheckDaemonExportOK(ctx); err != nil { return fmt.Errorf("checkDaemonExportOK: %v", err) } - if stdout, err := git.NewCommand("update-server-info"). + if stdout, err := git.NewCommandContext(ctx, "update-server-info"). SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())). RunInDir(repoPath); err != nil { log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err) diff --git a/modules/repository/generate.go b/modules/repository/generate.go index 47bc70734afc9..f6b76b14affa2 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -275,11 +275,11 @@ func GenerateRepository(ctx context.Context, doer, owner *models.User, templateR return generateRepo, err } - if err = generateRepo.CheckDaemonExportOK(); err != nil { + if err = generateRepo.CheckDaemonExportOK(ctx); err != nil { return generateRepo, fmt.Errorf("checkDaemonExportOK: %v", err) } - if stdout, err := git.NewCommand("update-server-info"). + if stdout, err := git.NewCommandContext(ctx, "update-server-info"). SetDescription(fmt.Sprintf("GenerateRepository(git update-server-info): %s", repoPath)). RunInDir(repoPath); err != nil { log.Error("GenerateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", generateRepo, stdout, err) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 6eafe97634b32..05306218de8b2 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -100,15 +100,11 @@ func MigrateRepositoryGitData(ctx context.Context, u *models.User, repo *models. repo.Owner = u } - if err = repo.GetOwner(); err != nil { - return repo, fmt.Errorf("unable to get owner[%d] for repo [%d]. Error: %v", repo.OwnerID, repo.ID, err) - } - - if err = repo.CheckDaemonExportOK(); err != nil { + if err = repo.CheckDaemonExportOK(ctx); err != nil { return repo, fmt.Errorf("checkDaemonExportOK: %v", err) } - if stdout, err := git.NewCommand("update-server-info"). + if stdout, err := git.NewCommandContext(ctx, "update-server-info"). SetDescription(fmt.Sprintf("MigrateRepositoryGitData(git update-server-info): %s", repoPath)). RunInDir(repoPath); err != nil { log.Error("MigrateRepositoryGitData(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)