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

dbus: Add dbus_user option to allow specifying a different run user #22158

Merged
merged 12 commits into from
Apr 9, 2024

Conversation

jwillikers
Copy link
Contributor

Specify library name and version: dbus/*

Building dbus requires that the user specified for dbus_user exists.
To allow projects to build dbus in various situations, it makes sense to allow this option to be modified.


Copy link
Contributor

github-actions bot commented Jan 4, 2024

🤖 Beep Boop! This pull request is making changes to 'recipes/dbus//'.

👋 @jwillikers you might be interested. 😉

@ghost
Copy link

ghost commented Jan 4, 2024

@conan-center-bot

This comment has been minimized.

@jwillikers
Copy link
Contributor Author

Requires #22156

@conan-center-bot

This comment has been minimized.

@jwillikers
Copy link
Contributor Author

@RubenRBS @uilianries Can I get a review for this? Without it, local dbus builds are failing due to the messagebus group not being in the image.

Traceback (most recent call last):
  File "/root/.conan/data/dbus/1.15.8/_/_/build/f90ea84e722ce1012f8fe615de9d901f8ff1b350/src/meson_post_install.py", line 106, in <module>
    post_install_exe()
  File "/root/.conan/data/dbus/1.15.8/_/_/build/f90ea84e722ce1012f8fe615de9d901f8ff1b350/src/meson_post_install.py", line 95, in post_install_exe
    os.chown(exe_path, 0, grp.getgrnam(dbus_user).gr_gid)
KeyError: "getgrnam(): name not found: 'messagebus'"

It is also probably be worth opening an issue upstream to verify the group exists.

@AbrilRBS
Copy link
Member

AbrilRBS commented Mar 6, 2024

My main concern here is if this option changing the package id is desirable or not. Do you have any extra insight?

@AbrilRBS AbrilRBS self-assigned this Mar 6, 2024
@jwillikers
Copy link
Contributor Author

My main concern here is if this option changing the package id is desirable or not. Do you have any extra insight?

I'll check and see if the default user is compiled into the binary, in which case, it should effect the id.

@jwillikers
Copy link
Contributor Author

Looks like it's only present in the system.conf file. The library itself doesn't contain any references: strings /home/jordan/.conan2/p/b/dbusbed52a0433957/p/lib/libdbus-1.a | grep -i messagebus was empty. No headers include it either. I'll remove the option in the package_id method.

@jwillikers
Copy link
Contributor Author

Someone beat me to opening an issue upstream: https://gitlab.freedesktop.org/dbus/dbus/-/issues/492

@jwillikers
Copy link
Contributor Author

Aha, the dbus maintainer recommends message_bus=false to disable the system related stuff, which would skip all of this.

@conan-center-bot

This comment has been minimized.

This will only build the library.
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

AbrilRBS
AbrilRBS previously approved these changes Apr 5, 2024
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

sorry for the delay, i've finally got the time to follow up on this, and now that I've understood it, this looks like the best way to go, thanks a lot for following up on it :)

As for dbus beind dependend upon with snapshot versions, I'm super open to approve bump PR for those to the latest "LTS" ones if you want, else just let me know which ones and I can do that myself!

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@franramirez688
Copy link
Contributor

@jwillikers the PR has some conflicts.

@conan-center-bot

This comment has been minimized.

1 similar comment
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 11 (e66b1e36090ba678cd64e0cd266ee2ca694a0709):

  • dbus/1.15.8:
    All packages built successfully! (All logs)

  • dbus/1.15.2:
    All packages built successfully! (All logs)

  • dbus/1.15.0:
    All packages built successfully! (All logs)

  • dbus/1.15.6:
    All packages built successfully! (All logs)

@jwillikers
Copy link
Contributor Author

@franramirez688 Done.

@jwillikers jwillikers requested a review from AbrilRBS April 8, 2024 14:45
@jwillikers jwillikers requested review from valgur and AbrilRBS April 8, 2024 16:37
@conan-center-bot conan-center-bot merged commit 04aa574 into conan-io:master Apr 9, 2024
28 checks passed
@jwillikers jwillikers deleted the dbus-user branch April 9, 2024 12:15
yhsng pushed a commit to yhsng/conan-center-index that referenced this pull request Apr 12, 2024
…ferent run user

* dbus: Add dbus_user option to allow specifying a different run user

* Remove dbus_user option from the package_id

* Add message_bus option and disable by default

This will only build the library.

* Bump build dependencies

* Actually set message_bus option and fix install paths

* Set session socket dir still

* Try to fix Windows

* Fix merge

* Use list

* Remove backslashes from the prefix path

---------

Co-authored-by: Rubén Rincón Blanco <[email protected]>
ericLemanissier pushed a commit to ericLemanissier/conan-center-index that referenced this pull request Apr 16, 2024
…ferent run user

* dbus: Add dbus_user option to allow specifying a different run user

* Remove dbus_user option from the package_id

* Add message_bus option and disable by default

This will only build the library.

* Bump build dependencies

* Actually set message_bus option and fix install paths

* Set session socket dir still

* Try to fix Windows

* Fix merge

* Use list

* Remove backslashes from the prefix path

---------

Co-authored-by: Rubén Rincón Blanco <[email protected]>
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.

5 participants