hmarr/codeowners

Doesn't correctly follow rules for ownership in nested directories

ThisIsMissEm opened this issue · 3 comments

ThisIsMissEm commented

I've just noticed that this project parses the codeowners file incorrectly according to Github's specification (examples), given the following lines:

* user-a
src/* user-b

Then the file at src/foo/bar.js should actually be owned by user-a, this is because of this logic:

# In this example, @doctocat owns any files in the build/logs
# directory at the root of the repository and any of its
# subdirectories.
/build/logs/ @doctocat

# The `docs/*` pattern will match files like
# `docs/getting-started.md` but not further nested files like
# `docs/build-app/troubleshooting.md`.
docs/*  docs@example.com

(from https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/about-code-owners )

Essentially the /* only matches direct descendants, not all descendants of a directory. This logic is the same for Gitlab too.

ThisIsMissEm commented

I suspect GitHub is actually using fnmatch(3) rules, with the FNM_PATHNAME | FNM_LEADING_DIR flags, there is a go library that seems to implement the logic of fnmatch(3): https://github.com/danwakefield/fnmatch/

hmarr commented

Thanks for pointing this out – you're totally right. And sorry for missing this issue until now! It's actually using pathspec, so I've updated the pattern matching logic to follow almost exactly what that library does. I tried it out with the example you provided and it seems to fix that behaviour.

I also pulled the tests cases out into JSON file and ran them through the internal codeowners library to verify the new logic matches the canonical one, at least for the cases covered in that file.

I'm planning on running a large project that uses code owners through both this library and the canonical GitHub one and comparing the results, which should help catch any edge cases I've missed. Once I've done that I'll tag a new release.

hmarr commented

I confirmed the new matching implementation is consistent with the canonical GitHub one over a large project, so I just released it in v0.3.0. Thanks again for reporting this!