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

Revert "Revert "Only build bundled fts if system has a bad version th… #324

Closed

Conversation

ignatenkobrain
Copy link
Contributor

…at doesn't handle LFS""

Test failure is bug in fakechroot.

This reverts commit 7e65bec.

@ignatenkobrain
Copy link
Contributor Author

ignatenkobrain commented Sep 9, 2017

I've tried to build with Fedora's fakechroot version: https://koji.fedoraproject.org/koji/taskinfo?taskID=21748556

Seems to be green!

@pmatilai
Copy link
Member

Yeah it fails because fakechroot doesn't have LFS support - and looking at upstream still doesn't. Fedora packages dont count.

Revisit when fakechroot upstream has a release that supports LFS, it's not like there's any actual urgency in getting this change in.

@ignatenkobrain
Copy link
Contributor Author

@pmatilai, upstream is looking on it btw.

@mhatle
Copy link

mhatle commented Sep 11, 2017

Instead of 'fakechroot', have you considered using 'pseudo'? It does support large filesystems and other APIs.

http://git.yoctoproject.org/cgit/cgit.cgi/pseudo/

Both inside and outside of the OpenEmbedded work, we exclusively use pseudo to replace 'fakeroot' and 'fakechroot' capabilities.

@ldv-alt
Copy link
Contributor

ldv-alt commented Sep 12, 2017

@pmatilai, how often do you sync your copy of fts.c with glibc?
I bet your fts.c doesn't have any fixes made in glibc since the last sync.

@ldv-alt
Copy link
Contributor

ldv-alt commented Sep 12, 2017

@n3npq I wish to get rid of fts.c completely, as well as all other stuff that belongs to glibc.
I wish you haven't added that stuff in the first place! :)

@ldv-alt
Copy link
Contributor

ldv-alt commented Sep 12, 2017

@n3npq Why should rpm ever want to walk a remote URI?

Anyway, rpm.org doesn't need its copy of fts.c when linked with glibc >= 2.23, and the bug is that

  • rpm.org's maintainer is not ready to admit this fact and act accordingly;
  • his attitude to contributors discourages further contributions.

@Conan-Kudo
Copy link
Member

Why should rpm ever want to walk a remote URI?

There are at least a couple of occasions where it does. RPM is capable of installing packages from remote locations. It'll fetch the RPM and go through the correct process to install the package as described by the user. I use this all the time for installing source packages, and it can be done with binary ones too.

@Conan-Kudo
Copy link
Member

@mhatle Would you consider proposing a patch to switch from fakechroot to pseudo? It looks like we have pseudo in Fedora, and if RPM's testsuite depends on it, I can introduce packages for Mageia and openSUSE so that the test suite can be run there.

@mhatle
Copy link

mhatle commented Sep 12, 2017

@Conan-Kudo patch for what? Generally speaking switching from fakeroot/fakechroot to pseudo should be seamless... (simple search/replace)

@Conan-Kudo
Copy link
Member

@mhatle Aside from that, @n3npq mentioned this:

Almost everything Just Works ... the few tests failing can likely be fixed rapidly.

If you could provide a PR with the change and fixed the failing tests, I think it'd be quite appreciated.

@ignatenkobrain
Copy link
Contributor Author

fakechroot patch has been merged in upstream btw.

@pmatilai
Copy link
Member

What a tempest in a teapot. Folks, time for a reality check.

I dislike bundled copies at least as much as the next guy, but:

  • Replacing X with Y broke Z, yes Y gets blamed first
  • Sometimes life's more complicated than that
  • Short-term, a working test-suite is FAR MORE VALUABLE to me than ideological "bundling is not nice"

We're talking about something bundled 20 years ago and on my watch, I don't recall seeing a single bug beyond portability tweaks reported on the bundled FTS copy. And I sure as hell dont have time to listen up this OH MY GOD HOUSE IS ON FIRE yack-yack on what is from rpm functional point a total non-issue.

I'll revisit the issue when its time.

@ldv-alt
Copy link
Contributor

ldv-alt commented Sep 13, 2017

@pmatilai, you are a perfect marvel! Thank you for being so grateful to us for getting rid of bundled X and fixing your buggy Z.
Thanks everybody for this enlightening discussion, I suppose no more comments are needed here.

@Conan-Kudo
Copy link
Member

@ldv-alt That's just rude. 👎

@pmatilai
Copy link
Member

@ldv-alt - the thing is, a "use foo on glibc" kind of change doesn't really do anything to get rid of the bundled code because it'll still be needed for other platforms. Instead these things end up adding more burden by introducing another path requiring validation. So when such a change breaks something (even if the something is broken by itself) without actually bringing in any concrete bug fixes or enhancements, pardon me for not jumping up and down in joy in here.

You really want to get rid of the bundled FTS, rewrite it using something that's actually standardized, such as nftw().

…at doesn't handle LFS""

Test failure is bug in fakechroot.

This reverts commit 7e65bec.
@pmatilai
Copy link
Member

pmatilai commented Mar 5, 2018

Half a year later, there's still no fakechroot upstream release with the relevant fix in it. Not exactly a huge surprise since the latest release is from 2016.

We can reconsider when they finally cut that release. In the meanwhile, see my earlier comment about nftw()...

@pmatilai pmatilai closed this Mar 5, 2018
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