Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Sep 3, 2025

Description

A partition with value "*" should match with a partition with value "".
An empty partition list should be treated as a list with a single partition with value "".
This should also match with a partition with value "*".

This PR refactors the matching of partitions to always use the StringMatching class, and makes the behavior of that class with respect to empty strings consistent between platforms.

@Mergifyio backport 3.3.x 3.2.x 2.14.x

Contributor Checklist

  • Commit messages follow the project guidelines.

  • The code follows the style guidelines of this project.

  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally

  • N/A: Any new/modified methods have been properly documented using Doxygen.

  • N/A: Any new configuration API has an equivalent XML API (with the corresponding XSD extension)

  • Changes are backport compatible: they do NOT break ABI nor change library core behavior.

  • Changes are API compatible.

  • N/A: New feature has been added to the versions.md file (if applicable).

  • N/A: New feature has been documented/Current behavior is correctly described in the documentation.

  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

This is an automatic backport of pull request #5989 done by [Mergify](https://mergify.com).

@mergify mergify bot added the conflicts Backport PR wich git cherry pick failed label Sep 3, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Sep 3, 2025

Cherry-pick of 27ce90d has failed:

On branch mergify/bp/2.14.x/pr-5989
Your branch is up to date with 'origin/2.14.x'.

You are currently cherry-picking commit 27ce90d9.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   test/unittest/utils/StringMatchingTests.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/cpp/rtps/builtin/discovery/endpoint/EDP.cpp
	both modified:   src/cpp/utils/StringMatching.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@MiguelCompany MiguelCompany added this to the v2.14.6 milestone Sep 3, 2025
@emiliocuestaf emiliocuestaf self-requested a review September 15, 2025 14:12
@emiliocuestaf emiliocuestaf removed their request for review October 20, 2025 10:33
@emiliocuestaf emiliocuestaf removed the conflicts Backport PR wich git cherry pick failed label Oct 20, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes partition matching behavior to ensure that empty partition lists (treated as "") correctly match against wildcard partitions ("*"). The implementation refactors all partition matching to use the StringMatching class consistently and ensures uniform empty string handling across platforms (Windows, WinRT, and POSIX).

Key Changes:

  • Unified all partition matching logic to use StringMatching methods instead of custom helpers
  • Added special handling for empty strings matching "*" wildcards on Windows platform
  • Removed platform-specific redundant implementations in favor of shared helper functions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/cpp/utils/StringMatching.cpp Refactored to use shared do_match_pattern() helper functions and added empty string handling for Windows platform
src/cpp/rtps/builtin/discovery/endpoint/EDP.cpp Replaced custom is_partition_empty() function with calls to StringMatching::matchString()
test/unittest/utils/StringMatchingTests.cpp Added test coverage for empty string matching behavior with wildcards

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions github-actions bot added the ci-pending PR which CI is running label Oct 20, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Oct 20, 2025

🧪 CI Insights

Here's what we observed from your CI run for d0eae7a.

❌ Job Failures

Pipeline Job Health on 2.14.x Retries 🔍 CI Insights 📄 Logs
Fast-DDS MacOS CI mac-ci / fastdds_test () Unknown 0 View View

@eProsima eProsima deleted a comment from mergify bot Oct 23, 2025
@emiliocuestaf
Copy link
Contributor

@Mergifyio rebase

MiguelCompany and others added 3 commits October 23, 2025 09:50
* Refs #23623. Always use StringMatching.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23623. Refactor StringMatching.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23623. Regression unit test..

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23623. Fix StringMatching on Windows.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23624. Avoid unnecessary conditionals.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 27ce90d)

# Conflicts:
#	src/cpp/rtps/builtin/discovery/endpoint/EDP.cpp
#	src/cpp/utils/StringMatching.cpp
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Emilio Cuesta Fernandez <emiliocuesta@eprosima.com>
@mergify
Copy link
Contributor Author

mergify bot commented Oct 23, 2025

rebase

✅ Branch has been successfully rebased

@emiliocuestaf emiliocuestaf force-pushed the mergify/bp/2.14.x/pr-5989 branch from 9aa22c0 to d0eae7a Compare October 23, 2025 09:50
@emiliocuestaf emiliocuestaf requested review from richiprosima and removed request for richiprosima October 23, 2025 09:51
@emiliocuestaf emiliocuestaf added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. skip-ci Automatically pass CI and removed ci-pending PR which CI is running labels Oct 24, 2025
@emiliocuestaf emiliocuestaf self-requested a review October 24, 2025 06:12
@MiguelCompany MiguelCompany removed ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. skip-ci Automatically pass CI labels Oct 24, 2025
@github-actions github-actions bot added the ci-pending PR which CI is running label Oct 24, 2025
@emiliocuestaf emiliocuestaf added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Oct 27, 2025
@MiguelCompany MiguelCompany merged commit 6cbdd58 into 2.14.x Oct 27, 2025
46 of 49 checks passed
@MiguelCompany MiguelCompany deleted the mergify/bp/2.14.x/pr-5989 branch October 27, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants