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

[flatpak] Predicate two cmds by gvfs service #3618

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pmoravec
Copy link
Contributor

Two commands "flatpak --installations" and "flatpak --print-updated-env" can enable user's gvfs-daemon.service even with
GVFS_REMOTE_VOLUME_MONITOR_IGNORE=1 set, so let put them under a predicate.

Resolves: #3618
Relevant: RHEL-14328


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

Two commands "flatpak --installations" and "flatpak --print-updated-env"
can enable user's gvfs-daemon.service even with
GVFS_REMOTE_VOLUME_MONITOR_IGNORE=1 set, so let put them under a
predicate.

Resolves: sosreport#3618
Relevant: RHEL-14328

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
@pmoravec pmoravec force-pushed the sos-pmoravec-flatpak-on-gvfsd-only branch from 2bd1ed8 to 03b61f7 Compare April 24, 2024 08:14
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3618
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@pmoravec
Copy link
Contributor Author

The "Report Stage One - centos-stream-8" job (repeatedly?) fails, in the middle of avocado testing, that is weird.. I am re-running it once again.

Copy link
Member

@TurboTurtle TurboTurtle left a comment

Choose a reason for hiding this comment

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

Comment below, but also shouldn't this instead be considered a bug to report to flatpak?

Comment on lines +23 to +27
gvfs_pred = SoSPredicate(
self, cmd_outputs={
'cmd': 'systemctl --user status gvfs-daemon.service',
'output': '(running)'
}
Copy link
Member

Choose a reason for hiding this comment

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

SosPredicate() supports a service parameter to check for running services. Is that not sufficient here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about it, but I think the --user option is crucial difference here. Let me re-test it..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we get services list via systemctl list-unit-files --type=service and that output does not contain users' services, InitSystem is not aware of the service despite it is running. So service parameter is useless here /o.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok, good observation. I guess that makes the question, should we expand our profiling for SystemdInit to include user services, or just go forward with this as written and keep our SystemD abstraction in the dark about user services?

@pmoravec
Copy link
Contributor Author

Comment below, but also shouldn't this instead be considered a bug to report to flatpak?

Yes, the RHEL-14328 is bug against flatpak.

@TurboTurtle
Copy link
Member

Comment below, but also shouldn't this instead be considered a bug to report to flatpak?

Yes, the RHEL-14328 is bug against flatpak.

My question was more of the flavor "does it make sense for sos to make this change, when this is really a bug in flatpak that will (hopefully) be addressed and then render the predicate unnecessary?"

Alternatively, do we need to version-gate this predicate?

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.

None yet

3 participants