Fix $filter operants and Msys/Cygwin

facebook / zstd

Zstandard - Fast real-time compression algorithm

http://www.zstd.net

Other

22.77k stars 2.03k forks source link

Fix $filter operants and Msys/Cygwin #4067

Closed QBos07 closed 3 weeks ago

QBos07 commented 1 month ago

switched the patter and input of $filter into the right places
added pattern wildcard to MSYS_NT & CYGWIN_NT as they change with windows versions
correctly identify MSYS2, even in an env like MINGW64

QBos07 commented 1 month ago

CI failure not because of changes

Cyan4973 commented 1 month ago

Are there scenarios that would fail before this PR ? If yes, does it feel possible to add a corresponding test ?

QBos07 commented 1 month ago

Yes, Msys and Cygwin didn't work before. You could add tests but in practice this should be a one off with little impact if it doesn't work

Cyan4973 commented 1 month ago

I'm mostly concerned that, if Msys and Cygwin were failing before, it remained invisible in our CI, so we might have a blind spot, and this may be worth fixing.

Also, without a test, it could be that the delivered property (works with Msys and Cygwin) will be broken by accident in some future PR, and we would not notice it.

Looking at our current CI tests, it seems we already have a bunch of mingw tests running, we even have one cygwin test, but they apparently all completed fine, therefore, were these tests wrong, or incomplete?

QBos07 commented 1 month ago

The toolchain for msys already uses the workaround and the most impacted part wich is installing is not tested