This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit faff17fe74d10b3a9c3796c61aec9c36c5a2cabe Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 17:51:40 2026 +0200 newt/downloader: Fix running git in project for non-git repo dirs Git commands for a repo were run by changing to its directory only. If repos/<name> existed but was not a git checkout (e.g. a copied tree) and the project itself was a git repository, git operated on the project: newt upgrade rewrote the project's origin and checked out the dependency commit in the project, removing project.yml. Before running git in a directory, verify once per directory that it is the top-level directory of a git repository and report an error otherwise. The project directory itself is exempt, as a project may be located inside a larger git repository. Assisted-by: Claude:claude-opus-5-5 --- newt/downloader/downloader.go | 47 ++++++++++++++ newt/repo/repo.go | 3 + newt/repogit_test.go | 109 ++++++++++++++++++++++++++++++++ newt/testdata/repogit_nogit/project.yml | 25 ++++++++ 4 files changed, 184 insertions(+) diff --git a/newt/downloader/downloader.go b/newt/downloader/downloader.go index 9d647d1f..b96d7082 100644 --- a/newt/downloader/downloader.go +++ b/newt/downloader/downloader.go @@ -29,6 +29,7 @@ import ( "sort" "strconv" "strings" + "sync" log "github.com/sirupsen/logrus" @@ -176,6 +177,48 @@ func gitPath() (string, error) { return filepath.ToSlash(gitPath), nil } +// Directories verified to be the top-level directory of a git repository, or +// allowed not to be one. +var gitTopLevelDirs = struct { + sync.Mutex + m map[string]struct{} +}{m: map[string]struct{}{}} + +// AllowGitSubdir allows running git commands in the specified directory even +// if it is not the top-level directory of a git repository (e.g., a project +// located inside a larger git repository). +func AllowGitSubdir(dir string) { + gitTopLevelDirs.Lock() + defer gitTopLevelDirs.Unlock() + + gitTopLevelDirs.m[filepath.Clean(dir)] = struct{}{} +} + +// checkGitTopLevel verifies that dir is the top-level directory of a git +// repository. Otherwise git commands run in dir would operate on an +// enclosing repository (e.g., the project) instead. Each directory is checked +// only once. Must be called with dir as the current working directory. +func checkGitTopLevel(gp string, dir string) error { + dir = filepath.Clean(dir) + + gitTopLevelDirs.Lock() + defer gitTopLevelDirs.Unlock() + + if _, ok := gitTopLevelDirs.m[dir]; ok { + return nil + } + + out, err := exec.Command(gp, "rev-parse", "--show-cdup").Output() + if err != nil || strings.TrimSpace(string(out)) != "" { + return util.FmtNewtError( + "\"%s\" is not a git repository; remove it so that newt can "+ + "download the repository again", filepath.ToSlash(dir)) + } + + gitTopLevelDirs.m[dir] = struct{}{} + return nil +} + func executeGitCommand(dir string, cmd []string, logCmd bool) ([]byte, error) { wd, err := os.Getwd() if err != nil { @@ -193,6 +236,10 @@ func executeGitCommand(dir string, cmd []string, logCmd bool) ([]byte, error) { defer os.Chdir(wd) + if err := checkGitTopLevel(gp, dir); err != nil { + return nil, err + } + gitCmd := []string{gp} gitCmd = append(gitCmd, cmd...) output, err := util.ShellCommandLimitDbgOutput(gitCmd, nil, logCmd, -1) diff --git a/newt/repo/repo.go b/newt/repo/repo.go index 070cc452..16c126fc 100644 --- a/newt/repo/repo.go +++ b/newt/repo/repo.go @@ -688,5 +688,8 @@ func NewLocalRepo(repoName string) (*Repo, error) { return nil, err } + // The project may be located inside a larger git repository. + downloader.AllowGitSubdir(r.Path()) + return r, nil } diff --git a/newt/repogit_test.go b/newt/repogit_test.go new file mode 100644 index 00000000..d91c15e2 --- /dev/null +++ b/newt/repogit_test.go @@ -0,0 +1,109 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +var repoGitNoGitDir = filepath.Join("testdata", "repogit_nogit") + +// repoGitInitProject turns dir into a git repository with a single commit +// and an "origin" remote. Returns the commit hash. +func repoGitInitProject(t *testing.T, env []string, dir string, + origin string) string { + + t.Helper() + + env = append(env, + "GIT_AUTHOR_NAME=newt test", "[email protected]", + "GIT_COMMITTER_NAME=newt test", + "[email protected]") + + runGit(t, env, nil, "-C", dir, "init", "-q") + runGit(t, env, nil, "-C", dir, "add", ".") + runGit(t, env, nil, "-C", dir, "commit", "-q", "-m", "project") + runGit(t, env, nil, "-C", dir, "remote", "add", "origin", origin) + + return runGit(t, env, nil, "-C", dir, "rev-parse", "HEAD") +} + +// If a directory in repos/ is not a git checkout, git commands for that repo +// must not be run against the enclosing project git repository. +func TestRepoGitNotCheckout(t *testing.T) { + env := dummyReposEnv(t) + projDir := copyToTemp(t, repoGitNoGitDir) + + const origin = "https://example.invalid/project.git" + head := repoGitInitProject(t, env, projDir, origin) + + newtOutput(t, projDir, env, "upgrade") + + // Turn the installed repo into a plain copy of its files. + repoDir := filepath.Join(projDir, "repos", "mynewt-dummy-repo-04") + if err := os.Rename(filepath.Join(repoDir, ".git"), + filepath.Join(t.TempDir(), "git")); err != nil { + + t.Fatal(err) + } + + out, err := newtCombinedOutput(projDir, env, "upgrade") + if err == nil { + t.Fatalf("newt upgrade succeeded\n%s", out) + } + if !strings.Contains(out, "is not a git repository") { + t.Fatalf("unexpected error\n%s", out) + } + + gotOrigin := runGit(t, env, nil, "-C", projDir, "config", "--get", + "remote.origin.url") + if gotOrigin != origin { + t.Fatalf("project origin changed to %q", gotOrigin) + } + + gotHead := runGit(t, env, nil, "-C", projDir, "rev-parse", "HEAD") + if gotHead != head { + t.Fatalf("project HEAD changed to %s", gotHead) + } + + if _, err := os.Stat(filepath.Join(projDir, "project.yml")); err != nil { + t.Fatalf("project.yml missing: %v", err) + } +} + +// A project located in a subdirectory of a git repository must still work +// and report the commit of that repository. +func TestRepoGitProjectInSubdir(t *testing.T) { + env := dummyReposEnv(t) + projDir := copyToTemp(t, repoGitNoGitDir) + + head := repoGitInitProject(t, env, filepath.Dir(projDir), + "https://example.invalid/project.git") + + newtOutput(t, projDir, env, "upgrade") + + out := newtOutput(t, projDir, env, "info") + if !strings.Contains(out, "repogit_nogit (project): "+head) { + t.Fatalf("project commit not reported\n%s", out) + } +} diff --git a/newt/testdata/repogit_nogit/project.yml b/newt/testdata/repogit_nogit/project.yml new file mode 100644 index 00000000..d74881ee --- /dev/null +++ b/newt/testdata/repogit_nogit/project.yml @@ -0,0 +1,25 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +project.name: repogit_nogit + +repository.mynewt-dummy-repo-04: + type: git + vers: 0.0.0 + url: 'https://dummy.invalid/mynewt-dummy-repo-04.git'
