Bug 1324784

Can you use the name pseudoroot for package and binary? I think pseudo is a too generic name and pseudoroot would be in conjunction with fakeroot what does similiar things. There's an alternative and some binary/script fakeroot-pseudo, is it a plugin?

Are you interest in a review swap with bug #1346457?

It looks like bug 134647 is taken already, but I'll happily review something else for you.

(In reply to Raphael Groner from comment #1)
> Can you use the name pseudoroot for package and binary? I think pseudo is a
> too generic name and pseudoroot would be in conjunction with fakeroot what
> does similiar things.

Makes sense. I'll ask upstream.

> There's an alternative and some binary/script fakeroot-pseudo, is it a plugin?

This is actually a drop-in replacement for fakeroot.

(In reply to Dominik 'Rathann' Mierzejewski from comment #4)
> (In reply to Raphael Groner from comment #1)
> > Can you use the name pseudoroot for package and binary? I think pseudo is a
> > too generic name and pseudoroot would be in conjunction with fakeroot what
> > does similiar things.
> 
> Makes sense. I'll ask upstream.

On second thought, I don't think it's an issue. Besides, Ubuntu is already packaging it as "pseudo"[1] and upstream's rationale for the name is that it's pronounced similar to "sudo".

[1] http://packages.ubuntu.com/xenial/pseudo

> Version:         1.7.5
1.8.1 is the latest.

> Source0:         pseudo-%{version}.tar.xz
https://downloads.yoctoproject.org/releases/pseudo/pseudo-1.8.1.tar.bz2

> Source2:         pseudo-mktarball.sh
not needed

I will just send patch which fixes all problems..

Created attachment 1195640 [details]
Proposed patch for spec

Created attachment 1195641 [details]
[PATCH] man: Add NAME section

Used patch from spec

Thanks for the patches. I'm against using sed instead of patches, it won't fail when patch no longer applies.

(In reply to Dominik 'Rathann' Mierzejewski from comment #9)
> Thanks for the patches. I'm against using sed instead of patches, it won't
> fail when patch no longer applies.
In my patch I didn't remove any of your patches.

Can you update spec and srpm, so I will do final round of review?

Sorry, I've been busy with other stuff. I'll try to spin a new package this weekend.

Spec URL: https://rathann.fedorapeople.org/review/pseudo/pseudo.spec
SRPM URL: https://rathann.fedorapeople.org/review/pseudo/pseudo-1.8.1-1.fc24.src.rpm

* Sun Oct 09 2016 Dominik Mierzejewski <dominik> 1.8.1-1
- update to 1.8.1
- use upstream release tarball
- add missing NAME section to manpage (patch from Debian)
- fix passing CFLAGS containing commas via --cflags
- install missing pseudolog(1) manpage

As I can see, not all bits were applied from patch which I attached...

> BuildRequires:   attr
not removed, it's useless BR

> sed -i -e "/s,@ARCH_FLAGS@/s|,|!|g" configure
I don't see this, but build was failing because we have commas in CFLAGS.

> link=$(readlink -e "%{_bindir}/fakeroot")
IMO we have fakeroot with alternatives support for long time, so this part can be skipped.

Now I'm not around laptop, once I will have it - I will check that it builds, some trivial stuff and approve.

(In reply to Igor Gnatenko from comment #13)
> As I can see, not all bits were applied from patch which I attached...
> 
> > BuildRequires:   attr
> not removed, it's useless BR

configure checks for getfattr availability at build time and prints a warning if it's not present. Failure to run getfattr doesn't disable any features though, so I'll drop this BR and patch out the warning instead.

> > sed -i -e "/s,@ARCH_FLAGS@/s|,|!|g" configure
> I don't see this, but build was failing because we have commas in CFLAGS.

I know. I fixed it another way. See the new patch.

> > link=$(readlink -e "%{_bindir}/fakeroot")
> IMO we have fakeroot with alternatives support for long time, so this part
> can be skipped.

The fakeroot in RHEL5 doesn't support alternatives. Neither does the one in RHEL6.

> Now I'm not around laptop, once I will have it - I will check that it
> builds, some trivial stuff and approve.

I checked that it builds across all primary arches via koji scratch build.

if getfattr --help >/dev/null 2>&1; then
    xattr_runs=true


hmm, then don't drop it.

Just checked all bits. On thing MUST be fixed: %{_libdir}/pseudo/libpseudo.so -> %{_libdir}/%{name}/


APPROVED.

Dominik, can you add me as co-maintainer in PkgDB when you will request package? I would be happy to help with it (as it could potentially become replacement for fakechroot for RPM test suide).

(In reply to Igor Gnatenko from comment #15)
> if getfattr --help >/dev/null 2>&1; then
>     xattr_runs=true
> 
> 
> hmm, then don't drop it.

As I said, it's only a warning.

(In reply to Igor Gnatenko from comment #16)
> Just checked all bits. On thing MUST be fixed:
> %{_libdir}/pseudo/libpseudo.so -> %{_libdir}/%{name}/
     
Why? %{name} is not shorter than "pseudo" and I'd argue that readability is better with the actual name instead of a macro.

> APPROVED.

Is it still approved if I don't make the above change?

> Dominik, can you add me as co-maintainer in PkgDB when you will request
> package? I would be happy to help with it (as it could potentially become
> replacement for fakechroot for RPM test suide).

Yes.

(In reply to Dominik 'Rathann' Mierzejewski from comment #17)
> (In reply to Igor Gnatenko from comment #15)
> > if getfattr --help >/dev/null 2>&1; then
> >     xattr_runs=true
> > 
> > 
> > hmm, then don't drop it.
> 
> As I said, it's only a warning.
> 
> (In reply to Igor Gnatenko from comment #16)
> > Just checked all bits. On thing MUST be fixed:
> > %{_libdir}/pseudo/libpseudo.so -> %{_libdir}/%{name}/
>      
> Why? %{name} is not shorter than "pseudo" and I'd argue that readability is
> better with the actual name instead of a macro.
Point was about owning /usr/lib64/pseudo/. But you was owning file without dir.
> 
> > APPROVED.
> 
> Is it still approved if I don't make the above change?
Sure!
> 
> > Dominik, can you add me as co-maintainer in PkgDB when you will request
> > package? I would be happy to help with it (as it could potentially become
> > replacement for fakechroot for RPM test suide).
> 
> Yes.

(In reply to Igor Gnatenko from comment #18)
> (In reply to Dominik 'Rathann' Mierzejewski from comment #17)
> > (In reply to Igor Gnatenko from comment #16)
> > > Just checked all bits. On thing MUST be fixed:
> > > %{_libdir}/pseudo/libpseudo.so -> %{_libdir}/%{name}/
> >      
> > Why? %{name} is not shorter than "pseudo" and I'd argue that readability is
> > better with the actual name instead of a macro.
> Point was about owning /usr/lib64/pseudo/. But you was owning file without
> dir.

Ah! Of course. I'll fix that.

And I'll keep BR: attr, because getfattr is required for tests.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/pseudo

pseudo-1.8.1-3.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-a7c7a9beb0

pseudo-1.8.1-3.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-f29333e0be

pseudo-1.8.1-3.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

pseudo-1.8.1-3.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.