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

libcgroup: update to 2.0. #32231

Closed
wants to merge 10 commits into from
Closed

libcgroup: update to 2.0. #32231

wants to merge 10 commits into from

Conversation

wibed
Copy link

@wibed wibed commented Jul 28, 2021

update libcgroup to version 2.0

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

srcpkgs/libcgroup/patches/musl-decls.patch Outdated Show resolved Hide resolved
srcpkgs/libcgroup/patches/musl-sterror_r.patch Outdated Show resolved Hide resolved
srcpkgs/libcgroup/template Outdated Show resolved Hide resolved
srcpkgs/libcgroup/template Outdated Show resolved Hide resolved
@paper42
Copy link
Member

paper42 commented Jul 28, 2021

pre_configure is not necessary, because you use a tarball with pregenerated configure script. The commit message body is not necessary.


Copy link
Member

Choose a reason for hiding this comment

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

please revert the changes on patches

Copy link
Author

Choose a reason for hiding this comment

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

the file has been reverted as in the original

Copy link
Member

Choose a reason for hiding this comment

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

Something is wrong, GitHub still shows the change. Are you sure you reverted the changes?

Comment on lines 13 to 16
checksum=aecc501a9ea6a97da0673585db5081df912ae607dec36d5f6f7ab14f69d48ab8
case "$XBPS_TARGET_MACHINE" in
Copy link
Member

Choose a reason for hiding this comment

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

add an empty line between checksum= and case.

Copy link
Author

Choose a reason for hiding this comment

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

added

Copy link
Member

Choose a reason for hiding this comment

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

this was not solved

sorry for the double comment

@wibed
Copy link
Author

wibed commented Jul 28, 2021

pre_configure is not necessary, because you use a tarball with pregenerated configure script. The commit message body is not necessary.

do you mean the block?

pre_configure() {
  autoreconf -if
}

@paper42
Copy link
Member

paper42 commented Jul 28, 2021

pre_configure is not necessary, because you use a tarball with pregenerated configure script. The commit message body is not necessary.

do you mean the block?

pre_configure() {
  autoreconf -if
}

yes and you might be able to remove the hostmakedepends variable then

@wibed
Copy link
Author

wibed commented Jul 28, 2021

pre_configure is not necessary, because you use a tarball with pregenerated configure script. The commit message body is not necessary.

do you mean the block?

pre_configure() {
  autoreconf -if
}

yes and you might be able to remove the hostmakedepends variable then

without the pre_configure block it builds fine
but it depends on the hostmakedepends variable

update libcgroup to version 2.0
@paper42
Copy link
Member

paper42 commented Jul 28, 2021

pre_configure is not necessary, because you use a tarball with pregenerated configure script. The commit message body is not necessary.

yes and you might be able to remove the hostmakedepends variable then

without the pre_configure block it builds fine
but it depends on the hostmakedepends variable

sorry, I forgot about flex, we can drop automake and libtool, but have to leave flex there

hostmakedepends="flex"

Can you also fix the lint issue from CI?

srcpkgs/libcgroup/template:10: use SPDX id for 'LGPL-2.1' license or see Manual.md

distfiles="${SOURCEFORGE_SITE}/libcg/${pkgname}-${version}.tar.bz2"
checksum=e4e38bdc7ef70645ce33740ddcca051248d56b53283c0dc6d404e17706f6fb51
homepage="https://github.com/libcgroup/libcgroup"
distfiles="https://github.com/libcgroup/libcgroup/releases/download/v${version}/${pkgname}-${version}.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

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

don't use $pkgname please

@paper42
Copy link
Member

paper42 commented Aug 12, 2021

@wibed are you still interested in continuing to work on this PR?

@wibed
Copy link
Author

wibed commented Aug 13, 2021

@wibed are you still interested in continuing to work on this PR?

@paper42 i do, please be patient.

@wibed
Copy link
Author

wibed commented Aug 18, 2021

@mt3 you just comitted literally when i was to to push my changes upstream

@wibed
Copy link
Author

wibed commented Aug 18, 2021

@mt3 it seems the checksum should be updated

Copy link
Member

@paper42 paper42 left a comment

Choose a reason for hiding this comment

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

please squash the commits to one

@paper42
Copy link
Member

paper42 commented Aug 18, 2021

@mt3 it seems the checksum should be updated

yes, that should fix the CI

Copy link
Member

@paper42 paper42 left a comment

Choose a reason for hiding this comment

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

squash the commits and it should be good to go

checksum=e4e38bdc7ef70645ce33740ddcca051248d56b53283c0dc6d404e17706f6fb51
license="LGPL-2.1-only"
homepage="https://github.com/libcgroup/libcgroup"
distfiles="https://github.com/libcgroup/libcgroup/releases/download/v${version}/libcgroup-${version}.tar.bz2"
Copy link
Member

Choose a reason for hiding this comment

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

let's use tar.gz

wibed added 3 commits August 20, 2021 02:13
This reverts commit 66ce3b42f27177cffac4dbd985eff07b51c91373.
@wibed
Copy link
Author

wibed commented Aug 20, 2021

okey, despise the "no push" commit message. i have reverted the commit back to 2a257f4, because i have committed and pushed from root directory in 977070c. after that i have rebased to master and pushed again with ef1055f.

@wibed
Copy link
Author

wibed commented Aug 20, 2021

it is getting late

@ericonr
Copy link
Member

ericonr commented Aug 20, 2021

How are you trying to rebase and squash the commits? We have a lot of comments in PRs here explaining how to do it correctly, if you need the exact commands.

@wibed
Copy link
Author

wibed commented Aug 21, 2021

@ericonr

i usually do a : git rebase -i HEAD~N .

but i have accidentally interleaved other commits, which makes it impossible to squash

checksum bz2 corrected

Revert "NO PUSH"

This reverts commit 66ce3b42f27177cffac4dbd985eff07b51c91373.

update to 2.0
@wibed
Copy link
Author

wibed commented Aug 22, 2021

ill reopen a new pr

@wibed wibed closed this Aug 22, 2021
@paper42 paper42 mentioned this pull request Aug 22, 2021
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 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

5 participants