Skip to content

Conversation

@cakebaker
Copy link
Contributor

This PR is a new attempt to get onig working with GCC 15 after the approach with setting CFLAGS='-std=gnu17' (#7944) failed. It also adds some entries to deny.toml to fix warnings from cargo-deny.

@cakebaker cakebaker force-pushed the cargo_toml_use_onig_from_github branch from a783e87 to f9608a0 Compare May 21, 2025 09:27
@cakebaker cakebaker marked this pull request as draft May 21, 2025 09:36
@tertsdiepraam
Copy link
Member

At what point do we just drop it? It wouldn't be compatible but it seems to be more work than it's worth?

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

@drinkcat
Copy link
Collaborator

Hopefully a new release is coming soon... rust-onig/rust-onig#195

@frendsick
Copy link
Contributor

At what point do we just drop it? It wouldn't be compatible but it seems to be more work than it's worth?

The onig is only used for expr to support grep's regex syntax flavor. It is decently close to BRE syntax, but does not exactly match the needs of expr. We still need to transpile the user input to make it compatible with GNU expr, as there are still multiple ignored test_bre* tests.

It would be possible to drop onig by writing a transpiler from the BRE syntax to Rust's regex. It requires more code than with onig, but at least the resulting regex would be compatible with modern regex syntax and support modern features. It could also be easy enough to maintain, as people should be more accustomed to modern regex syntax and there are good tools for debugging modern regex syntaxes, like regex101. I do not know any such tools for BRE syntax.

@frendsick
Copy link
Contributor

If the BRE --> Rust regex transpiler sounds like a decent idea, I could draft one and see how it would feel.

@drinkcat
Copy link
Collaborator

Hopefully a new release is coming soon... rust-onig/rust-onig#195

We could use #7973 + cargo update onig_sys. Or just wait for 6.5.1.

@cakebaker
Copy link
Contributor Author

Closing this PR in favor of #7976

@cakebaker cakebaker closed this May 24, 2025
@cakebaker cakebaker deleted the cargo_toml_use_onig_from_github branch May 24, 2025 06:19
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.

4 participants