github-pr-resource icon indicating copy to clipboard operation
github-pr-resource copied to clipboard

Support multiple paths/ignore_paths and `**` in pattern

Open invidian opened this issue 4 years ago • 12 comments

It would be great to have that back, so one does not need configure something like following:

ignore_paths:
- docs/*
- docs/*/*
- docs/*/*/*
- ...

It has been added in #16 and then removed in #24 :/

invidian avatar Apr 06 '20 12:04 invidian

It would be helpfull if you could use the standar unix of **/* to match for all, seems like other people have ran into similar issues https://stackoverflow.com/questions/26809484/how-to-use-double-star-glob-in-go and even created a package we could use to do the heavy lifting.

https://github.com/yargevad/filepathx/blob/master/filepathx.go

At the moment we have to so something like the following to match :

"src/**/*", "src/js/index.js"
"src/**/**/*", "src/js/components/index.js"

We should just be able to do src/**/* to math them both

fbritoferreira avatar Aug 07 '20 10:08 fbritoferreira

The same would be useful for paths option too. We are moving our existing repos into a Monorepo and are currently blocked due to lack of recursive glob lookup **/* as without it any change in the repo will trigger builds for all the apps.

harshalpatel06 avatar Aug 07 '20 11:08 harshalpatel06

@itsdalmo is this something we could add?

fbritoferreira avatar Aug 21 '20 10:08 fbritoferreira

We are dealing with the same issue:

source:
  paths:
    - "foo/bar/*"
    - "foo/bar/**/*"
    - ...
    - "foo/bar/**/**/**/**/**/**/**/**/**/**/*"

@itsdalmo can it be prioritize for the next release?

m-raby avatar May 13 '21 11:05 m-raby

It would seem that file paths have two common use cases, trigger by watching single file for changes or watching a portion of the directory structure for changes. The second case being the issue some are having. It would appear that the filepath.walk function would more closely match the use case.

luker31337 avatar Aug 17 '21 13:08 luker31337

I'm clearly late to the party 🥳. It looks like an undocumented feature exists for these kind of use cases. Looking at how paths are compared in FilterPath() (also FilterIgnorePath()), if filepath.Match() fails using the patterns in the paths attribute, then IsInsidePath() is used as a fallback, to determine if changed files exists within a parent directory according to prefix matching.

Where you might write this:

source:
  paths:
    - foo/*

Instead use:

source:
  paths:
    - foo

If foo is a directory, any files changed in a pull request that are children of that directory (i.e. paths that contain the prefix foo/) will be reflected in the versions emitted by the check container. The same applies to subdirectories (e.g. foo/bar, foo/bar/baz).

marcransome avatar Nov 13 '23 09:11 marcransome

This is actually documented, albeit very easy to miss:

Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes

Emphasis added to the last word 'prefixes'.

marcransome avatar Nov 13 '23 09:11 marcransome

The issue is that it only matches at that single level. It should glob multi levels down. My temp solution was to do this.

  • dir1/*
  • dir1/dir2/*
  • dir1/dir2/dir3/*
  • ....

On Mon, Nov 13, 2023, 4:15 AM Marc Ransome @.***> wrote:

This is actually documented, albeit very easy to miss:

Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes

Emphasis added to the last word 'prefixes'.

— Reply to this email directly, view it on GitHub https://github.com/telia-oss/github-pr-resource/issues/192#issuecomment-1807738345, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVHLRM5UWJDTJ2ZYFUAJ4D3YEHQKVAVCNFSM4MCH6C2KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBQG43TGOBTGQ2Q . You are receiving this because you commented.Message ID: @.***>

luker31337 avatar Nov 13 '23 12:11 luker31337

@luker31337 Wouldn't - dir1 therefore work for your scenario, given it's the common prefix for any files that would match the example glob patterns you gave?

The issue is that it only matches at that single level. It should glob

multi levels down. My temp solution was to do this.

  • dir1/*

  • dir1/dir2/*

  • dir1/dir2/dir3/*

  • ....

marcransome avatar Nov 13 '23 16:11 marcransome

The issue isn't that you can't explicitly watch every level. It is possible to configure a path to every directory level. Instead, it would be nice when setting the paths parameter to also be able to glob all directories, instead of having to include every enumeration of path. This is a usability enhancement because newer versions of Go do support globbing directory paths.

paths:
  - dir1/*
  - dir1/dir2/*
  - dir1/dir2/dir3/*
  - dir1/dir2/dir3/dir4/*
  - dir1/dir2/dir3/dir4/dir5/*
  - ....

replaced with

paths:
  - dir1/**/*

luker31337 avatar Nov 13 '23 17:11 luker31337

@luker31337 I may be missing your point, so feel free to clarify, but I'm not seeing a functional difference between your example glob pattern and using a prefix for that use case, the latter of which is supported in the current codebase.

The following enumerated paths:

paths:
  - dir1/*
  - dir1/dir2/*
  - dir1/dir2/dir3/*
  - dir1/dir2/dir3/dir4/*
  - dir1/dir2/dir3/dir4/dir5/*
  - ....

Can be replaced with:

paths:
  - dir1

Wouldn't dir1/**/* therefore be synonymous with, or functionally the same as, dir1?


Here's a small test case showing that IsInsidePath() (taken verbatim from the codebase for this project) would match file paths for all of the glob patterns you specified in your enumerated example, given the prefix value dir1 (as if configuring the Concourse resource with a paths value of - dir1 only):

package main

import (
	"fmt"
	"path/filepath"
	"strings"
)

func IsInsidePath(parent, child string) bool {
	if parent == child {
		return true
	}

	// we add a trailing slash so that we only get prefix matches on a
	// directory separator
	parentWithTrailingSlash := parent
	if !strings.HasSuffix(parentWithTrailingSlash, string(filepath.Separator)) {
		parentWithTrailingSlash += string(filepath.Separator)
	}

	return strings.HasPrefix(child, parentWithTrailingSlash)
}

func main() {
	var paths = []string{
		"dir1/test",
		"dir1/dir2/test",
		"dir1/dir2/dir3/test",
		"dir1/dir2/dir3/dir4/test",
		"dir1/dir2/dir3/dir4/dir5/test"
	}

	var parent = "dir1"

	for _, str := range paths {
		if IsInsidePath(parent, str) {
			fmt.Printf("%s is contained within %s\n", str, parent)
		}
	}
}

Outputs:

dir1/test is contained within dir1
dir1/dir2/test is contained within dir1
dir1/dir2/dir3/test is contained within dir1
dir1/dir2/dir3/dir4/test is contained within dir1
dir1/dir2/dir3/dir4/dir5/test is contained within dir1

TL;DR dir1 will match any file path that starts with the prefix dir1/, regardless of the number of path components (i.e. subdirectories).

marcransome avatar Nov 13 '23 18:11 marcransome

It was a long time ago now that I was managing this. I may be misremembering the specifics.

I agree that works, but I think what I found was more related to an issue with setting pipelines dynamically using the pipeline configuration. In my case, we had some repos that we wanted to watch a certain directory and others where we wanted to watch a whole repository. If you want to watch all directories and files in a repo, there wasn't a way to specify from the top level descending into each directory

At the time, we just had to maintain two pipeline configuration. Wasn't an ideal solution, because all other changes had to be replicated between the two. They only difference was this resource. everything-pipeline.yml - we wouldn't set the path parameter dir-pipeline.yml - set the path parameter; we did the long way, but could have used above

- set_pipeline
  file: everything-pipeline.yml 
  vars:
    pipeline-name: pipeline1

or

- set_pipeline
  file: dir-pipeline.yml 
  vars:
    pipeline-name: pipeline2
    paths:
      - dir1/*

Again, it was a while back, but we had someone fix this issue with this library. https://github.com/ryanuber/go-glob Now we use one pipeline and can pass different paths to watch.

- set_pipeline
  file: pipeline.yml 
  vars:
    pipeline-name: pipeline1
    paths:
      - "*"

or

- set_pipeline
  file: pipeline.yml 
  vars:
    pipeline-name: pipeline2
    paths:
      - dir1/*

luker31337 avatar Nov 13 '23 19:11 luker31337