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

Trying to upstream debian patches: Fix compilation on platforms without MAP_POPULATE #985

Closed

Conversation

@Conan-Kudo
Copy link
Member

I feel like this patch is just asking for trouble. Given that it touches the debuginfo generation code, I'm not terribly comfortable passing judgment here.

Copy link
Contributor

@ignatenkobrain ignatenkobrain left a comment

Choose a reason for hiding this comment

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

from what I have read in the documentation, MAP_POPULATE will just speed-up things. So if anybody will use this patch anyway, I think this desserves some better commit message and just be merged.

That said, I am not sure if I am right :)

@KOLANICH
Copy link
Author

I think this desserves some better commit message

I have just preserved the message present in the patch. BTW, it LGTM. How do you propose to change it?

@pmatilai
Copy link
Member

pmatilai commented Jan 2, 2020

How do you propose to change it?

Just note that MAP_POPULATE is safe to drop in this manner as it's only an optimization, so future generations won't need to chase it down.

@Conan-Kudo
Copy link
Member

@KOLANICH Here's a suggestion:

$ git commit --amend --author="Michal Čihař <nijel@debian.org>" --date="Sat, 11 Nov 2017 14:27:10 +0100"

With the following commit message:

tools/sepdebugcrcfix: Conditionally use MAP_POPULATE with mmap()

Not all architectures offer MAP_POPULATE. As MAP_POPULATE is only an
optimization to improve performance, it is safe to drop it when it is
unavailable.

Not all architectures offer MAP_POPULATE. As MAP_POPULATE is only an
optimization to improve performance, it is safe to drop it when it is
unavailable.

Original message from patch file:

> Fix compilation on platforms without MAP_POPULATE

Original patch URI: https://salsa.debian.org/pkg-rpm-team/rpm/raw/1af008f89ee30a604de1c617c8bb9dc4dcd8bf81/debian/patches/map-populate.patch
@pmatilai
Copy link
Member

Merged via #1006 . Thanks for the effort of bringing this upstream.

@pmatilai pmatilai closed this Jan 14, 2020
@KOLANICH
Copy link
Author

Thanks.

@KOLANICH KOLANICH deleted the map_populate branch January 14, 2020 12:51
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

5 participants