Skip to content

fix: allow minor and patch versions of minimatch#146

Merged
hildjj merged 13 commits into
editorconfig:1.0.xfrom
G-Rath:relax-minimatch
Feb 27, 2026
Merged

fix: allow minor and patch versions of minimatch#146
hildjj merged 13 commits into
editorconfig:1.0.xfrom
G-Rath:relax-minimatch

Conversation

@G-Rath

@G-Rath G-Rath commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

This allows downstream consumers to use later versions of minimatch, including v9.0.6 which addresses GHSA-3ppc-4f35-3m26

@G-Rath G-Rath mentioned this pull request Feb 24, 2026
@G-Rath

G-Rath commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Running locally this passes the test suite:

> editorconfig@1.0.4 test
> npm run test:all


> editorconfig@1.0.4 test:all
> mocha && ctest . --preset Test


  [▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬]

  17 passing (50ms)

Test project /workspaces/editorconfig-core-js
196/196 Test #196: multiple_files_on_command_line
100% tests passed, 0 tests failed out of 196

Total Test time (real) =  16.85 sec

@G-Rath

G-Rath commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

right so after further digging I see there is a test failure in the latest test suite, for some reason seemingly with the latest v9 version and v10 beyond the current pinned version - very odd

Relates to isaacs/minimatch#265

@hildjj

hildjj commented Feb 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks for digging in to this. Please keep letting me know what actions I can take to move the ball forward, but as you can see, I only get a few windows a day to help.

@hildjj

hildjj commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator

Let's rebase this, and show the tests failing for the correct reason.

@G-Rath

G-Rath commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

@hildjj there's nothing to rebase since this is on the v1.0.x branch - also there's discussion going on over in minimatch and juliangruber/brace-expansion#88 has been approved, so I'm hopefully this will get resolved upstream soon.

I'm not sure what can be done about the macOS failure specifically other than skipping Node v14 on that OS, but I can look into that later

@hildjj

hildjj commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator

Nod. Let's disable macos/node14, see what happens, then go from there.

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

@hildjj this should be good to go for v1 and v3: the latest minimatch v9 uses brace-expansion V2 which does not have the bug, and the latest brace-expansion has the bug fixed so should be good from all angles 🎉

Are you good to take it from here, or would you like my help?

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

I can take over from here I think. The tricky part is going to be getting it released, since I think I'll need to backport the workflow changes, and we'll probably need to add "publishConfig": { "tag": "1.x" } to the package.json. I'll do this before landing #152 and publishing that patch in case I mess up the latest tag by mistake.

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

I'm also fighting with cmake versions now.

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

I'm going to need more help. The tests don't pass with cmake 4.2.3 which is what I have locally. They get close with cmake 3.13.4, but 65 - braces_closing_in_beginning still fails.

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

@hildjj I have very little experience with cmake so not sure if I'll be able to help much, but will give it a go.

I've been doing all my work in a GitHub Codespace, which looks to be using v3.28.3:

@G-Rath ➜ /workspaces/editorconfig-core-js (1.0.x) $ cmake --version
cmake version 3.28.3

CMake suite maintained and supported by Kitware (kitware.com/cmake).

Running npm run test on the 1.0.x branch and this relax-minimatch branch, all tests pass; if I update the submodule to the latest commit (https://github.com/editorconfig/editorconfig-core-test/tree/895b3a65d0d823dbd0acf2bc402376381995d1b1), then 174 - key_set_to_empty_string fails for both branches.

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

what would be really helpful is if you could show me how to find particular tests - searching for key_set_to_empty_string and similar is not returning anything...

found it, apparently the search just didn't want to find that particular value 🙄

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

They get close with cmake 3.13.4, but 65 - braces_closing_in_beginning still fails.

I have just found that tests seems to be excluded:

"filter": {
"exclude": {
"name": "braces_closing_in_beginning"
}
},

So I would guess the issue is the config file is not being used correctly, rather than a new test failure?

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

So I would guess the issue is the config file is not being used correctly, rather than a new test failure?

Yes, that's right.

All of tests pass for me with 3.28.3 as well. If we can get that version to run in CI, we can move forward.

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

I've tested with node 14 on macos locally, so I'm fine with it not running on CI for this old version.

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

@hildjj CI is green, looks like things stop working after cmake v3.31 - let me know if you're happy with this, or want more work done

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

LGTM. I'm going to take over, and tweak this to see if I can get it to release using OIDC.

@hildjj hildjj mentioned this pull request Feb 27, 2026
@hildjj hildjj merged commit c2c2742 into editorconfig:1.0.x Feb 27, 2026
12 checks passed
@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

Great job @G-Rath! I realized right after I merged this that I really wish your name was in the contributors going forward, if you're ok with it. Can you send a patch against main for that, please?

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

Oh F. I pushed the 1.05 patch to main. that's going to take a minute to unravel.

@G-Rath

G-Rath commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

I've literally just shutdown my laptop 😅 but if you do the patch with me as a co-author I'm happy to approve it :)

@hildjj

hildjj commented Feb 27, 2026

Copy link
Copy Markdown
Collaborator

Looks like I didn't screw anything up too bad. Deep breath.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants