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

Always use > not perldoc -d #973

Merged
merged 1 commit into from
Jun 28, 2013
Merged

Always use > not perldoc -d #973

merged 1 commit into from
Jun 28, 2013

Conversation

kentfredric
Copy link
Contributor

perldoc -d $path is limited because perldoc SETUID's to UID=nobody if
UID=0, which basically guarantees that perldoc -d $path will not be
able to write anywhere in the working directory.

Though using redirection may be limited on some platforms, any competing
solution should change the code executed depending on platform.

On UNIX as UID=0, -d should always be avoided because of perldocs
inherent defects.

perldoc -d $path is limited because perldoc SETUID's to `UID=nobody` if
`UID=0`, which basically guarantees that `perldoc -d $path` will not be
able to write anywhere in the working directory.

Though using redirection may be limited on some platforms, any competing
solution should change the code executed depending on platform.

On UNIX as UID=0, -d should **always** be avoided because of perldocs
inherent defects.
@kentfredric
Copy link
Contributor Author

Note: On my $platform, the early configure test says perldoc .... no, both with, and without this patch.

However, without this patch, installation fails due to UID=nobody not being able to write some of the documentation.

This may be a function of how we're running it, our glue layer in bash can be seen here:

http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/dev-lang/parrot/parrot-5.5.0.ebuild?view=markup

@leto leto merged commit 05d8871 into parrot:master Jun 28, 2013
@leto
Copy link
Member

leto commented Jun 28, 2013

I think this makes Parrot work better, on average, so I merged this.

If it causes any problems on supported platforms, I can back it out. People on non-Unix systems: please send bug reports! /cc @rurban @Benabik

@fperrad
Copy link
Contributor

fperrad commented Jul 14, 2013

This commit breaks perldoc detection on Windows (with Strawberry Perl 5.18.0.1)

  auto::perldoc -       Is perldoc installed.................................no.

Please revert it.

@rurban
Copy link
Member

rurban commented Jul 14, 2013

So to summarize the technical issues.

make does not build documentation, make installable does.
so in the default use case sudo make install does.
sudo perldoc will fail.
The workaround with >, without -d fails on windows because it cannot find there
if run from a terminal or not.

So I recommend to build documentation in the default target,
warn prominently when detecting sudo make that make docs must be run as non-root
and use -d $path to fix the windows issues.

So please revert #973, raise up the installable target which includes docs to all,
and add the uid=0 warning

On Jul 14, 2013, at 9:45 AM, François Perrad notifications@github.com wrote:
This commit breaks perldoc detection on Windows (with Strawberry Perl 5.18.0.1)

auto::perldoc - Is perldoc installed.................................no.
Please revert it.


Reply to this email directly or view it on GitHub.

@leto
Copy link
Member

leto commented Jul 14, 2013

@rurban I tend to agree with you. Is there a way to make the current way work on Windows?

Also, would you be willing to create a branch that builds docs in the default target? Is that going to significantly increase compile times?

@rurban
Copy link
Member

rurban commented Jul 14, 2013

On Jul 14, 2013, at 11:22 AM, leto notifications@github.com wrote:

@rurban I tend to agree with you. Is there a way to make the current way work on Windows?

Also, would you be willing to create a branch that builds docs in the default target? Is that going to significantly increase compile times?

Unfortunately I don't have time. I'm super busy with p5p, B::C, p2 and php (!) deadlines.

@leto
Copy link
Member

leto commented Jul 15, 2013

I reverted the commit, @fperrad . Please let me know if that resolves it for you.

@kentfredric
Copy link
Contributor Author

If you made it so the docs were made during make or something, and were not an implicit dependent of make install that'd be a workable compromise for us, because we (Gentoo) can have that build as a non-privileged user.

However, make install always runs as UID=0 atm and there's no easy way for us to do anything else in a way that will work for all our users and on all our package managers ( for which, there are now 3 competing implementations ), so "just drop privs in the ebuild" is not really an option for us.

We'll maybe end up locally patching it to work, but we really do try make bugs get fixed upstream wherever possible.

https://bugs.gentoo.org/show_bug.cgi?id=454058

( Though, here I think the real bug is perldoc dropping privs, which it says it does for "security reasons", but there's no documentation about the actual security risk its trying to avoid, "LETS JUST RUN EVERYTHING AS NOBODY, MAKES TOTAL SENSE!" )

@leto
Copy link
Member

leto commented Jul 15, 2013

@kentfredric I am dedicated to making it easy for Gentoo to package Parrot. It seems we are genuinely running into undocumented and possibly untested behavior in perldoc. Would you agree?

The reason I had to revert your commit is because it caused problems on one of our supported platforms and that is not acceptable. We don't have continuous integration on Windows, so we have to sometimes merge to master and get feedback from our Windows developers. This is Less Than Awesome.

Your contribution is valuable and will not be lost. I will create a branch for it and hopefully @fperrad will help us test your code and make it compatible with his setup. He is a long-time Parrot core developer and wrote the Lua implementation on Parrot. If not, I know others that like this kind of thing.

@fperrad
Copy link
Contributor

fperrad commented Jul 15, 2013

@leto : now, works fine, thanks

@kentfredric
Copy link
Contributor Author

@leto We've been patching it downstream for a while, and until recently the > redirection hack worked.

However, it no longer does, and due to UID dropping perldoc can no longer even read the source file. No amount of chmod tricks get around this. ( Because the parent directory is often not readable or writeable by UID=nobody, or its parent, etc )

So I've taken a little look into how perldoc works, and it appears its easy to make a script that just uses the modules within Perldoc that you actually need to do the work you're doing, and you can do so without the sillyness of dropping to UID=0.

@rurban
Copy link
Member

rurban commented Feb 18, 2014

Implemented a better fix with #1028 in the 6.1.0 release

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.

4 participants