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

j4-dmenu-desktop: add support for checks #31937

Merged
merged 1 commit into from Jul 14, 2021

Conversation

meator
Copy link
Contributor

@meator meator commented Jul 13, 2021

The current version of tests errors out when /usr/share/applications does not exist. See enkore/j4-dmenu-desktop#123

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

Does it build and run successfully?

(Please choose at least one native build and, if supported, at least one cross build. More are better.)

  • I built this PR locally for my native architecture, (x86_64-glibc)
  • I built this PR locally for these architectures (if supported. mark crossbuilds):
    • x86_64-musl
    • aarch64-musl
    • armv7l
    • armv6l-musl

@ericonr
Copy link
Member

ericonr commented Jul 13, 2021

Alternatively, we could add a package with files under /usr/share/applications to checkdepends, so it's installed when the test suite runs. What do you think?

@meator
Copy link
Contributor Author

meator commented Jul 13, 2021

What do you mean by that?

@ericonr
Copy link
Member

ericonr commented Jul 13, 2021

*with files

If it needs /usr/share/applications to exist, having something like xterm installed should be enough for the test suite to work...

@meator
Copy link
Contributor Author

meator commented Jul 13, 2021

It doesn't actually need any .desktop files in /usr/share/applications, it just needs the folder. Adding some packages with desktop files to checkdepends is not really necessary. Another solution would be to mkdir -p /usr/share/applications in pre_check(). What do you think?

@ericonr
Copy link
Member

ericonr commented Jul 13, 2021

I dislike mkdir here because we shouldn't touch the masterdir outside builddir and destdir. Using a package is preferred (add a comment on top of the checkdepends line, though.

@meator
Copy link
Contributor Author

meator commented Jul 13, 2021

Ok. I have removed the patch.

@ericonr
Copy link
Member

ericonr commented Jul 14, 2021

Great, thanks!

@ericonr ericonr merged commit ea59d69 into void-linux:master Jul 14, 2021
@meator meator deleted the j4-dmenu-desktop branch July 14, 2021 06:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants