Repository: kubernetes-sigs/krew
Pull request #372 (2019-11-10 10:32:59)

Pin version of release-notes helper
Sigh.. go modules is hard.

See comment https://github.com/kubernetes-sigs/krew/pull/326#issuecomment-552164347

/assign @ahmetb 

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/372#" title="Author self-approved">corneliusweig</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [corneliusweig]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov-io: # [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=h1) Report
> Merging [#372](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=desc) into [master](https://codecov.io/gh/kubernetes-sigs/krew/commit/612112f61793192e8b3fed0394f38bc948aa8dd7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/kubernetes-sigs/krew/pull/372/graphs/tree.svg?width=650&token=J3nFtARI2m&height=150&src=pr)](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master    #372   +/-   ##
======================================
  Coverage    57.2%   57.2%           
======================================
  Files          19      19           
  Lines         916     916           
======================================
  Hits          524     524           
  Misses        339     339           
  Partials       53      53
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=footer). Last update [612112f...1ece0a0](https://codecov.io/gh/kubernetes-sigs/krew/pull/372?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ahmetb: /lgtm
/approve
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/372#issuecomment-552217155" title="Approved">ahmetb</a>*, *<a href="https://github.com/kubernetes-sigs/krew/pull/372#" title="Author self-approved">corneliusweig</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb,corneliusweig]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/hack/make-release-notes.sh b/hack/make-release-notes.sh
index 6989ca7..39b1518 100755
--- a/hack/make-release-notes.sh
+++ b/hack/make-release-notes.sh
@@ -44,7 +44,7 @@ install_release_notes() {
 
   cd "$relnotes_dir"
   go mod init foo
-  GOBIN="${SCRIPTDIR}" go install github.com/corneliusweig/release-notes
+  GOBIN="${SCRIPTDIR}" go get github.com/corneliusweig/release-notes@v0.1.0
   cd -
 }
 
diff --git a/pkg/pathutil/pathutil.go b/pkg/pathutil/pathutil.go
index e30d602..b672426 100644
--- a/pkg/pathutil/pathutil.go
+++ b/pkg/pathutil/pathutil.go
@@ -23,23 +23,15 @@ import (
 
 // IsSubPath checks if the extending path is an extension of the basePath, it will return the extending path
 // elements. Both paths have to be absolute or have the same root directory. The remaining path elements
-func IsSubPath(subPath, path string) ([]string, bool) {
-	basePieces := strings.Split(filepath.Clean(subPath), string(filepath.Separator))
-	extendingPieces := strings.Split(filepath.Clean(path), string(filepath.Separator))
-
-	// the binary has to be in the install path.
-	if len(basePieces) > len(extendingPieces) {
-		return nil, false
+func IsSubPath(basePath, subPath string) (string, bool) {
+	extendingPath, err := filepath.Rel(basePath, subPath)
+	if err != nil {
+		return "", false
 	}
-
-	// Compare path pieces.
-	for i, p := range basePieces {
-		if extendingPieces[i] != p {
-			return nil, false
-		}
+	if strings.HasPrefix(extendingPath, "..") {
+		return "", false
 	}
-
-	return extendingPieces[len(basePieces):], true
+	return extendingPath, true
 }
 
 // ReplaceBase will return a replacement path with replacement as a base of the path instead of the old base. a/b/c, a, d -> d/b/c
@@ -48,5 +40,5 @@ func ReplaceBase(path, old, replacement string) (string, error) {
 	if !ok {
 		return "", errors.Errorf("can't replace %q in %q, it is not a subpath", old, path)
 	}
-	return filepath.Join(replacement, filepath.Join(extendingPath...)), nil
+	return filepath.Join(replacement, extendingPath), nil
 }


Test patch:
diff --git a/pkg/pathutil/pathutil_test.go b/pkg/pathutil/pathutil_test.go
index a968624..10c350c 100644
--- a/pkg/pathutil/pathutil_test.go
+++ b/pkg/pathutil/pathutil_test.go
@@ -28,44 +28,81 @@ func TestIsSubPathExtending(t *testing.T) {
 	tests := []struct {
 		name            string
 		args            args
-		wantExtending   []string
+		wantExtending   string
 		wantIsExtending bool
 	}{
 		{
 			name: "is extending",
 			args: args{
-				basePath:      filepath.Join("a", "b"),
-				extendingPath: filepath.Join("a", "b", "c"),
+				basePath:      filepath.FromSlash("a/b"),
+				extendingPath: filepath.FromSlash("a/b/c"),
 			},
-			wantExtending:   []string{"c"},
+			wantExtending:   "c",
 			wantIsExtending: true,
 		},
 		{
 			name: "is extending same length",
 			args: args{
-				basePath:      filepath.Join("a", "b", "c"),
-				extendingPath: filepath.Join("a", "b", "c"),
+				basePath:      filepath.FromSlash("a/b/c"),
+				extendingPath: filepath.FromSlash("a/b/c"),
 			},
-			wantExtending:   []string{},
+			wantExtending:   ".",
 			wantIsExtending: true,
 		},
 		{
 			name: "is not extending",
 			args: args{
-				basePath:      filepath.Join("a", "b"),
-				extendingPath: filepath.Join("a", "a", "c"),
+				basePath:      filepath.FromSlash("a/b"),
+				extendingPath: filepath.FromSlash("a/a/c"),
 			},
-			wantExtending:   nil,
-			wantIsExtending: false,
 		},
 		{
 			name: "is not smaller extending",
 			args: args{
-				basePath:      filepath.Join("a", "b"),
-				extendingPath: filepath.Join("a"),
+				basePath:      filepath.FromSlash("a/b"),
+				extendingPath: filepath.FromSlash("a"),
 			},
-			wantExtending:   nil,
-			wantIsExtending: false,
+		},
+		{
+			name: "base path is not clean",
+			args: args{
+				basePath:      filepath.FromSlash("d/../a/b/../../a"),
+				extendingPath: filepath.FromSlash("a/b"),
+			},
+			wantExtending:   "b",
+			wantIsExtending: true,
+		},
+		{
+			name: "extending path is not clean",
+			args: args{
+				basePath:      filepath.FromSlash("a"),
+				extendingPath: filepath.FromSlash("d/../a/b/c/.."),
+			},
+			wantExtending:   "b",
+			wantIsExtending: true,
+		},
+		{
+			name: "base path is absolute",
+			args: args{
+				basePath:      filepath.FromSlash("/a"),
+				extendingPath: filepath.FromSlash("a/b"),
+			},
+		},
+		{
+			name: "extending path is absolute",
+			args: args{
+				basePath:      filepath.FromSlash("a"),
+				extendingPath: filepath.FromSlash("/a/b"),
+			},
+		},
+		{
+			name: "both paths are absolute",
+			args: args{
+				basePath:      filepath.FromSlash("/a"),
+				extendingPath: filepath.FromSlash("/a/b"),
+			},
+			wantExtending:   "b",
+			wantIsExtending: true,
 		},
 	}
 	for _, tt := range tests {