Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No owner for specific file #40

Open
leolambertucci opened this issue Jul 25, 2022 · 6 comments
Open

No owner for specific file #40

leolambertucci opened this issue Jul 25, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@leolambertucci
Copy link

if i create a codeowners file like this:
CODEOWNERS:
/build/logs/log.txt @ghost

owners = CodeOwners(example_file)
owners.of("/build/logs/log.txt") it doesnt return any owner

was this supposed to happen?

@sbdchd sbdchd added the bug Something isn't working label Jul 25, 2022
@sbdchd
Copy link
Owner

sbdchd commented Jul 25, 2022

Is the actual text of the file:

/build/logs/log.txt @ghost

or does it include the CODEOWNERS heading as well?

@leolambertucci
Copy link
Author

this is the actual text

@KrystianOcado
Copy link

Please ignore my previous comments, it was dumb :) I RTFM, and I now know what is \A and \Z. Sorry about this.

Still, the problem is here: https://github.com/sbdchd/codeowners/blob/master/codeowners/__init__.py#L48

Instead of:

regex += r"\A" if anchored else r"(?:\A|/)"

it should be:

regex += r"\A/" if anchored else r"(?:\A|/)"

The / is missing from the regex in the current version.

leolambertucci pushed a commit to leolambertucci/codeowners that referenced this issue Jul 27, 2022
@sbdchd
Copy link
Owner

sbdchd commented Jul 28, 2022

Maybe this is another case of behavior differing from git?

Git ignore says it doesn't match:

_______________________________________________________________ test_specific_patterns_against_git[single-file regression] _______________________________________________________________

name = 'single-file regression', pattern = '/build/logs/log.txt', paths = {'/build/logs/log.txt': True}

    @pytest.mark.parametrize(
        "name,pattern,paths", GO_CODEOWNER_EXAMPLES, ids=ids_for(GO_CODEOWNER_EXAMPLES)
    )
    def test_specific_patterns_against_git(
        name: str, pattern: str, paths: Dict[str, bool]
    ) -> None:
        """
        Ensure the expected patterns match actual git behavior.
    
        Codeowners is a subset of git ignore behavior so checking against it
        should work in most cases.
        """
        if name == "docs with star":
            pytest.skip("Behaviour for docs-with-star does not match gitignore")
    
        assert paths
        with tempfile.TemporaryDirectory() as directory:
            subprocess.run(["git", "init"], cwd=directory, check=True, capture_output=True)
            (Path(directory) / ".gitignore").write_text(pattern + "\n")
            for path, expected in paths.items():
                res = subprocess.run(
                    ["git", "check-ignore", path], cwd=directory, capture_output=True
                )
                actual = res.returncode == 0
>               assert (
                    actual is expected
                ), f"match for pattern:{pattern} and path:{path} failed, expected: {expected}, actual: {actual}"
E               AssertionError: match for pattern:/build/logs/log.txt and path:/build/logs/log.txt failed, expected: True, actual: False
E               assert False is True

@leolambertucci
Copy link
Author

Please ignore my previous comments, it was dumb :) I RTFM, and I now know what is \A and \Z. Sorry about this.

Still, the problem is here: https://github.com/sbdchd/codeowners/blob/master/codeowners/__init__.py#L48

Instead of:

regex += r"\A" if anchored else r"(?:\A|/)"

it should be:

regex += r"\A/" if anchored else r"(?:\A|/)"

The / is missing from the regex in the current version.

when i replace the regex and execute my code, it gets the file owner. it worked.
but the tests are failing. i will try to check the tests and see if I notice anything else

@addepar-tg
Copy link

For anyone else reading this and wondering how to work around this, I removed the leading / from any file query, and it works. But this fix should be merged if the maintainer is so inclined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants