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

New package: ydotool-0.2.0 #31237

Closed
wants to merge 1 commit into from

Conversation

noarchwastaken
Copy link
Contributor

It seems like the package uses CPM in its CMake config for statically linking libevdevPlus and libuInputPlus, both are versioned newer than the existing ones in Void, with breaking changes. There's another dependency called IODash which doesn't exist in Void yet.

Is it OK to simply use CPM? Or should we update the first two and add the last one into Void, then build with those? Pinging @steinex for opinions.

We already have #25341, but it's an older version and apparently have some unfixed problems... So I wish this is a friendly takeover of that PR.

closes #31163

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

@ericonr ericonr added the new-package This PR adds a new package label Jun 2, 2021
@steinex
Copy link
Contributor

steinex commented Jun 2, 2021

Updating libevdevPlus and libuInputPlus is fine, there are no packages in the repo currently which depend on these. I guess I added them back in the day to package ydotool and then left Wayland, so feel free.

@noarchwastaken
Copy link
Contributor Author

noarchwastaken commented Jun 2, 2021

Updating libevdevPlus and libuInputPlus is fine, there are no packages in the repo currently which depend on these. I guess I added them back in the day to package ydotool and then left Wayland, so feel free.

The problem is that both of them became static libraries now... I don't see a point to package them anymore.

@steinex
Copy link
Contributor

steinex commented Jun 3, 2021

I'll leave it to the team to decide if it's fine to use CPM. If they agree, I'm fine with removing libevdevPlus / libuInputPlus altogether. 👍

@noarchwastaken noarchwastaken marked this pull request as ready for review June 6, 2021 22:33
@noarchwastaken noarchwastaken changed the title [WIP] New package: ydotool-0.2.0 New package: ydotool-0.2.0 Jun 6, 2021
@noarchwastaken
Copy link
Contributor Author

noarchwastaken commented Jun 6, 2021

I fixed the musl build problem, and this package is ready for review!

Note that it statically links against dependencies with CPM. It looks like we accept this for other languages like Rust and Go (static linking against external packages), so I guess it's fine here...

Comment on lines +17 to +20
# patches a dependency pulled by CPM, so it must be done after configure
post_configure() {
patch -sl -Np0 -i ${FILESDIR}/iodash-musl.patch 2>/dev/null
}
Copy link
Member

Choose a reason for hiding this comment

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

CPM shouldn't be allowed to pull in dependencies. You need to manually download the tarball or package the dependency.

@github-actions
Copy link

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-package This PR adds a new package Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[package request] ydotool
3 participants