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

branch protection status check pattern #33121

Closed
gzzi opened this issue Jan 6, 2025 · 3 comments · Fixed by #33125
Closed

branch protection status check pattern #33121

gzzi opened this issue Jan 6, 2025 · 3 comments · Fixed by #33125
Labels

Comments

@gzzi
Copy link

gzzi commented Jan 6, 2025

Description

Hi,

I have a branch protection status check pattern issue. My pattern use a glob and is **/*@(code_style|affected). On the setting page, I see that my CI is marked as match (see first screenshot). But on pull request view same CI passed but a extra line is added with the pattern and written as required (see second screenshot).

Gitea Version

1.22.6

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

image

image

Git Version

No response

Operating System

No response

How are you running Gitea?

unsure but I think it's docker image

Database

None

@gzzi gzzi added the type/bug label Jan 6, 2025
@Zettat123
Copy link
Contributor

Zettat123 commented Jan 6, 2025

Gitea uses two different libs to test glob patterns.

On the branch protection setting page (the first screenshot), we test the pattern with js code.

// show the `Matched` mark for the status checks that match the pattern
const markMatchedStatusChecks = () => {
const patterns = (document.querySelector<HTMLTextAreaElement>('#status_check_contexts').value || '').split(/[\r\n]+/);
const validPatterns = patterns.map((item) => item.trim()).filter(Boolean);
const marks = document.querySelectorAll('.status-check-matched-mark');
for (const el of marks) {
let matched = false;
const statusCheck = el.getAttribute('data-status-check');
for (const pattern of validPatterns) {
if (minimatch(statusCheck, pattern)) {
matched = true;
break;
}
}
toggleElem(el, matched);
}
};

On the pull request page (the second screenshot), we test the pattern with go code.

var missingRequiredChecks []string
for _, requiredContext := range pb.StatusCheckContexts {
contextFound := false
matchesRequiredContext := createRequiredContextMatcher(requiredContext)
for _, presentStatus := range commitStatuses {
if matchesRequiredContext(presentStatus.Context) {
contextFound = true
break
}
}
if !contextFound {
missingRequiredChecks = append(missingRequiredChecks, requiredContext)
}
}
ctx.Data["MissingRequiredChecks"] = missingRequiredChecks

So I think the cause of this bug may be related to the go glob library we are using (gobwas/glob).


As a workaround, maybe you could use two patterns

**/*code_style
**/*affected

@TheFox0x7
Copy link
Contributor

go glob library doesn't support extended syntax (gobwas/glob#3) and @(...) is part of extended one according to bash man:

If the extglob shell option is enabled using the shopt builtin, the shell recognizes several extended pattern matching operators.
[...]
@(pattern-list)
Matches one of the given patterns.

There are two options for solving this as far as I can see:

  1. Fix go library to support extended pattern (or replace it with one that has)
  2. Disable extended syntax for minimatch

Second one is obviously easier and I can make a PR for this.

lafriks pushed a commit that referenced this issue Jan 6, 2025
Underlying go library has no support for it

Fixes: #33121

---

I never touched frontend tests so pointers how to write them are
welcome.

This can be either fix or workaround, depending if this is something
gitea should support in the future or not. The golang side is unlikely
to get updates though.
@gzzi
Copy link
Author

gzzi commented Jan 7, 2025

By looking at go glob I found out that it was in fact possible without using extended pattern. The syntax is:

**/*{affected,code_style}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants