Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Traverse the filesystem only once when looking for UDI and Subiquity snaps. #258

Merged
merged 6 commits into from
Aug 1, 2022

Conversation

CarlosNihelton
Copy link
Collaborator

Here is the key of this PR:

        // if none of the required snaps exist in the rootfs no UI will be available.
-        if (!internal::hasUdiSnap() && !internal::hasSubiquitySnap()) {
+        if (!internal::hasAnyOfSnaps(L"ubuntu-desktop-installer"sv, L"subiquity"sv)) {

For that to be possible, hasAnyOfSnaps() has to be implemented as a template function that can accept a variadic number of string_view arguments and applies the existing algorithms to find entries inside the snap directory matching one of the supplied arguments. Since that receives a variadic argument in order to nicely compose find_file_if and std::any_of we put together a push_back_many algorithm that populates a vector of string_views from a variadic argument list.

BTW, find_file_if returns a bool instead of iterator, so more like std::any_of than std::find_if, thus the function was renamed to any_file_of.

And since there were some documentation comments lacking on the algorithms.h header and some unevitable linter complains about parameters of the same type in starts/ends_with algorithms, I put some comments to stop linter at those lines.

I think it is easier to review this PR commit by commit.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Please check out carefully the isses found by clang-tidy in the new code ⚠️
⚠️ clang-format found formatting issues in the code submitted.:warning:
Make sure to run clang-format and update this pull request.
(1/1)

DistroLauncher/algorithms.h Outdated Show resolved Hide resolved
DistroLauncher/WinOobeStrategy.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@EduardGomezEscandell EduardGomezEscandell left a comment

Choose a reason for hiding this comment

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

Minor performance comment

DistroLauncher/algorithms.h Outdated Show resolved Hide resolved
Co-authored-by: Edu Gómez Escandell <edu.gomez.escandell@canonical.com>
@CarlosNihelton CarlosNihelton merged commit b6c900e into main Aug 1, 2022
@CarlosNihelton CarlosNihelton deleted the has-anyof-snaps branch August 1, 2022 11:26
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.

2 participants