Skip to content

deps: Remove the use of the regex crate#201

Merged
mxpv merged 4 commits intocontainerd:mainfrom
bryantbiggs:deps/swap-regex-lite
Sep 21, 2023
Merged

deps: Remove the use of the regex crate#201
mxpv merged 4 commits intocontainerd:mainfrom
bryantbiggs:deps/swap-regex-lite

Conversation

@bryantbiggs
Copy link
Copy Markdown
Contributor

@bryantbiggs bryantbiggs commented Sep 19, 2023

  • Instead of using the regex crate to ensure the path ends at the correct / which is a bit heavy handed for this use case, it has been replaced with an iterator based implementation
  • Simplify longest common prefix logic - this was pinched from here, the original logic seemed a bit verbose

@github-actions github-actions Bot added the C-shim Containerd shim label Sep 19, 2023
@mxpv
Copy link
Copy Markdown
Member

mxpv commented Sep 20, 2023

I wonder if we need regex dependency at all.
we only use it in trim_flawed_dir.

Given these test cases:

tcases.push(("/.foo-_bar/foo", "/.foo-_bar/".to_string()));
tcases.push(("/.foo-_bar/foo/", "/.foo-_bar/foo/".to_string()));
tcases.push(("/.foo-_bar/foo/bar", "/.foo-_bar/foo/".to_string()));
tcases.push(("/.foo-_bar/foo/bar/", "/.foo-_bar/foo/bar/".to_string()));

I feel like it should be relatively straightforward to rewrite it without regular expressions.

WDYT?

@bryantbiggs
Copy link
Copy Markdown
Contributor Author

I wonder if we need regex dependency at all. we only use it in trim_flawed_dir.

Given these test cases:

tcases.push(("/.foo-_bar/foo", "/.foo-_bar/".to_string()));
tcases.push(("/.foo-_bar/foo/", "/.foo-_bar/foo/".to_string()));
tcases.push(("/.foo-_bar/foo/bar", "/.foo-_bar/foo/".to_string()));
tcases.push(("/.foo-_bar/foo/bar/", "/.foo-_bar/foo/bar/".to_string()));

I feel like it should be relatively straightforward to rewrite it without regular expressions.

WDYT?

Great point! Let me know what you think of the implementation

@bryantbiggs bryantbiggs changed the title deps: Replace regex with the lighter weight regex-lite deps: Remove the use of the regex crate Sep 20, 2023
Comment thread crates/shim/src/mount.rs Outdated
@mxpv mxpv requested a review from a team September 20, 2023 17:07
Copy link
Copy Markdown
Member

@Burning1020 Burning1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Copy link
Copy Markdown
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing this!

@mxpv mxpv added this pull request to the merge queue Sep 21, 2023
Merged via the queue into containerd:main with commit d3088c4 Sep 21, 2023
@bryantbiggs bryantbiggs deleted the deps/swap-regex-lite branch September 21, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-shim Containerd shim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants