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.

Sign up for GitHub

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

Jump to bottom

shared/install: modernize and eliminate the use of basename() #32603

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

shared/install: modernize and eliminate the use of basename() #32603

wants to merge 12 commits into from
+298 −277

Conversation

YHNdnzj
Copy link
Member

@YHNdnzj YHNdnzj commented May 1, 2024 •
edited by github-actions bot

Split out from #31588

@github-actions github-actions bot added systemctl util-lib portable Anything to do with systemd-portable and portablectl and portables please-review PR is ready for (re-)review by a maintainer labels May 1, 2024
@github-actions GitHub Actions

This comment was marked as off-topic.

Sign in to view
@yuwata yuwata added the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label May 1, 2024
@github-actions github-actions bot removed the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label May 1, 2024
@YHNdnzj YHNdnzj added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed please-review PR is ready for (re-)review by a maintainer labels May 1, 2024
@YHNdnzj YHNdnzj marked this pull request as draft May 1, 2024 10:23
@YHNdnzj YHNdnzj added install and removed ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR labels May 1, 2024
@YHNdnzj YHNdnzj force-pushed the install-basename branch 3 times, most recently from 56421e2 to 7180e72 Compare May 1, 2024 13:14
@YHNdnzj YHNdnzj force-pushed the install-basename branch 3 times, most recently from 6d4f82e to 5d680d1 Compare May 10, 2024 13:20
@github-actions github-actions bot added the tests label May 10, 2024
@YHNdnzj YHNdnzj force-pushed the install-basename branch 6 times, most recently from 714bdac to bc69542 Compare May 11, 2024 08:36
@YHNdnzj YHNdnzj marked this pull request as ready for review May 11, 2024 08:36
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label May 11, 2024
@github-actions github-actions bot added the tests label May 13, 2024
bluca
bluca approved these changes May 14, 2024
View reviewed changes
src/shared/install.c Show resolved Hide resolved
@bluca bluca added good-to-merge/with-minor-suggestions and removed please-review PR is ready for (re-)review by a maintainer labels May 14, 2024
@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels May 14, 2024
yuwata
yuwata reviewed May 14, 2024
View reviewed changes
src/shared/install.c Outdated Show resolved Hide resolved
yuwata
yuwata approved these changes May 14, 2024
View reviewed changes
Copy link
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

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

LGTM, except for the above.

@yuwata yuwata added good-to-merge/with-minor-suggestions and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels May 14, 2024
YHNdnzj added 12 commits May 15, 2024 22:22
@YHNdnzj
shared/dropin: minor modernization
d54dc23
@YHNdnzj
shared/install: return bool where appropriate
523fb38
@YHNdnzj
shared/install: modernize remove_marked_symlink_fd
b65d845 
Note that we now collect errors from
install_changes_add(), too.
@YHNdnzj
shared/install: modernize find_symlinks and friends
c2ad83e
@YHNdnzj
shared/install: modernize install_info_add
dfda272
@YHNdnzj
install,dbus-manager: make unit_file_* take (char* const*) for strv
4e485e8
@YHNdnzj
shared/install: modernize unit_file_link
0fd9170 
Also eliminate one more use of basename(), yay!
@YHNdnzj
shared/install: modernize unit_file_get_list, use key destructor
5088d29 
The rest of the basename()s are easy to drop.
@YHNdnzj
shared/install: use path_extract_filename in install_info_traverse
abaab54
@YHNdnzj
shared/install: replace streq + basename with path_equal_filename
d003db9
@YHNdnzj
shared/install: use FOREACH_ARRAY at one more place
3b7a973
@YHNdnzj
unit-file,portable: replace streq + basename with path_equal_filename
ef6f0a9
@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels May 15, 2024
@bluca bluca added good-to-merge/after-next-release and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels May 20, 2024
@YHNdnzj YHNdnzj added this to the v257 milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluca bluca bluca approved these changes

@yuwata yuwata yuwata approved these changes

Assignees
No one assigned
Labels
good-to-merge/after-next-release install portable Anything to do with systemd-portable and portablectl and portables systemctl tests util-lib
Milestone
v257
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@YHNdnzj @bluca @yuwata