FS#9362 - makepkg: umask 0022 has to be at the top not in run_build()

Attached to Project: Pacman
Opened by Attila (attila) - Sunday, 27 January 2008, 18:25 GMT
Last edited by Xavier (shining) - Monday, 18 February 2008, 22:45 GMT
Task Type Bug Report
Category makepkg
Status Closed
Assigned To Xavier (shining)
Architecture All
Severity Low
Priority Normal
Reported Version 3.1.0
Due in Version 3.1.2
Due Date Undecided
Percent Complete 100%
Votes 2
Private No

Details

Summary and Info:
It is nearly the same as described in http://bugs.archlinux.org/task/9176 but now it happens to makepkg (see Steps). After i comment out "umask 0022" in run_build() and put them at the top (see Patch) all works fine.

Steps to Reproduce:
- login as normal user
- umask 0007
- makepkg -o
- ls -lR /src | less
=> files have rw-r-----
=> dirs have rwxr-x---

Patch:
export TEXTDOMAINDIR='/usr/share/locale'

+# ensure we have a sane umask set
+umask 0022

myver='3.1.1'
This task depends upon

Closed by  Xavier (shining)
Monday, 18 February 2008, 22:45 GMT
Reason for closing:  Fixed
Additional comments about closing:  will be fixed in 3.1.2 (commit 17180890a5).
Comment by Dan McGee (toofishes) - Wednesday, 06 February 2008, 02:23 GMT
It finally occurred to me why this may work this way.

When we unzip/untar the files, we would be using the normal umask, which isn't a surprise. The real question is whether we care. On one hand, I don't know that we need to- the umask of files in src/ _shouldn't matter_. It is the responsibility of the package to install files to pkg/ with the correct permissions.

So should we care here?
Comment by Attila (attila) - Wednesday, 06 February 2008, 06:15 GMT
You be right that it is the job of the "make install" procedure to have the right file permissions in "pkg" and so in the most cases it doesn't matter what is in "src". I recognized this for the first time as a make my own test kernel package 2.6.24. Here the most files have 0644 but the files in scripts have 0755 so i can't copy them all in the same way or solves it with a a simple "find . -type f -print0 | xargs -r0 chmod 644" line in the PKGBUILD and this makes building of such a package a lot more complex. So i look for a solution and after putting 'umask 0022' at the top all works fine.

If you see problems with my workaround or if you don't like it than this is okay for me. I can live with it that i have a NoUpgrade line in pacman.conf for makepkg. I know it is me who wants to use umake 0007 for his users.-)
Comment by Xavier (shining) - Wednesday, 06 February 2008, 06:56 GMT
It seems safer to set a sane umask right at the beginning, so that it affects extraction too, even if it shouldn't matter in most cases.
What are the downsides of this?

I suppose a 007 umask is for hiding confidential files to other users of the same system, right? There is nothing confidential about a package that is going to be installed on the system anwyway.
I have the feeling I might be totally off here though, so I apologize if that's the case :)
Comment by Dan McGee (toofishes) - Thursday, 07 February 2008, 04:38 GMT
Can someone find the commit where this got changed and see if there was any logic as to why? If not, I'll concede this argument and we can set the umask earlier, although I think this is good in that it exposes broken PKGBUILDs that use 'cp' rather than a proper 'install' command with a mode attached.

This commit is relevant, but not our guy:
http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=commitdiff;h=53f2dcaa3aabaeb251706f2e61cd151cf06a2d07

Hell, it looks like 3.0 did it this way too! makepkg blob @ v3.0.6 tag:
http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=blob;f=scripts/makepkg;h=c192855f97d9c53dc4bbd431fb265113447d796d;hb=a06d91f7f9b2e895c5dcfff314632919e417e864#l884
Comment by Xavier (shining) - Thursday, 07 February 2008, 09:36 GMT
After some quick manual bisection on gitweb, here is it :
http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=commitdiff;h=ac965ed4010e55dfffe945a88ba01091acfb6351

I find it odd that makepkg manually sources the files in /etc/profile.d. I didn't understand the reason for that yet.
Comment by Roman Kyrylych (Romashka) - Thursday, 07 February 2008, 10:09 GMT
for example, sourcing /etc/profile.d/* is needed in a case of building a qt3-dependent package with -s option when qt3 is not yet installed on a system
Comment by Dan McGee (toofishes) - Thursday, 07 February 2008, 12:51 GMT
Oh wow, we still do this? This is getting close to Arch-specific territory...

Sourcing /etc/profile might be legit, but I don't feel like we should be sourcing those files in /etc/profile.d/. Any thoughts?
Comment by Roman Kyrylych (Romashka) - Thursday, 07 February 2008, 12:56 GMT
but then how to solve the issue I described above? (I don't have any idea)
Comment by Roman Kyrylych (Romashka) - Thursday, 07 February 2008, 13:07 GMT
ok, sourcing /etc/profile is portable and it will source /etc/profile.d/*,
but then it may override some settings in user's .bashrc etc., so we should source them as well. :-/
Comment by Xavier (shining) - Thursday, 07 February 2008, 13:19 GMT
Why should we do that?
The assumption that it's possible to build a package using system wide settings doesn't seem crazy to me.

And well, thinking about this umask issue again, I am beginning to think we should not care.
If an user is using umask 007, he should know what he's doing. And if he wanted to extract a source tarball manually,
he would get the same result.
Comment by Attila (attila) - Thursday, 07 February 2008, 17:48 GMT
Oh, a lot of comments after i comes back from work.-)

@Xavier As i said above i support you that a user with umask 0007 have to look for himself that all works and still again i can live that i have to find a solution that my workaround survives in makepkg.

There is only one little discrepance: If you think this until the end than you have to delete the lines in run_build() too (which i don't suggest). I only write this report because there is a "umask 0022" in makepkg and for me all works fine if i put this line at the top of makepkg.

@all Sorry if i make your work harder as necessary.
Comment by Xavier (shining) - Thursday, 14 February 2008, 14:27 GMT

Loading...