817142 - Using nsinstall & nsinstall.py to install files on unix can set bad permissions

Status: REOPENED

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

Running nsinstall -R; nsinstall -R
  - succeeds; symlink dst/fyl to src

Running nsinstall.py -R ; nsinstall.py -R
  - succeeds copy src to dst

Running nsinstall -R ; nsinstall.py -R
  - create symlink
  - chmod stat.S_IWUSR $src through the $dst/fyl symlink (mode==200)
  - remove dst/$fyl
  - fail trying to read/copy source

--w------- /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py


config/nsinstall.py
===================
  def copy_all_entries(entries, target):
        [snip]
        if os.path.exists(targetpath):
          # On Windows, read-only files can't be deleted
          os.chmod(targetpath, stat.S_IWUSR)
          os.remove(targetpath)

Could either make the chmod platform specific or add special handling when the destination file exists and is a symlink.



RUN: /local/mozilla/bugs/foo/obj-x86_64-unknown-linux-gnu/dist/bin/nsinstall -R -m 755 /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py /local/mozilla/bugs/foo/other-licenses/ply/ply/lex.py /local/mozilla/bugs/foo/other-licenses/ply/ply/yacc.py ../../../dist/sdk/bin/ply

+ /bin/ls -l /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py /local/mozilla/bugs/foo/obj-x86_64-unknown-linux-gnu/dist/sdk/bin/ply/__init__.py
lrwxrwxrwx 1 joey joey 58 2012-11-30 16:29 /local/mozilla/bugs/foo/obj-x86_64-unknown-linux-gnu/dist/sdk/bin/ply/__init__.py -> /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py
-rw-rw-r-- 1 joey joey 82 2012-11-30 16:28 /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py
+ set +x


RUN: python /local/mozilla/bugs/foo/config/nsinstall.py -R -m 755 /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py /local/mozilla/bugs/foo/other-licenses/ply/ply/lex.py /local/mozilla/bugs/foo/other-licenses/ply/ply/yacc.py ../../../dist/sdk/bin/ply

+ /bin/ls -l /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py /local/mozilla/bugs/foo/obj-x86_64-unknown-linux-gnu/dist/sdk/bin/ply/__init__.py
/bin/ls: cannot access /local/mozilla/bugs/foo/obj-x86_64-unknown-linux-gnu/dist/sdk/bin/ply/__init__.py: No such file or directory
--w------- 1 joey joey 82 2012-11-30 16:28 /local/mozilla/bugs/foo/other-licenses/ply/ply/__init__.py
+ set +x

Comment 1

[Joey Armstrong [:joey]]

Part 2 - nsinstall should mimic rsync behavior and only copy/update timestamps when content or file type (file/symlink) has changed.  Current behavior for nsinstall and nsinstall.py is to blindly copy files when called.

Comment 2

[Mike Hommey [:glandium]]

nsinstall.py is not used on unix builds.

Comment 3

[Joey Armstrong [:joey]]

Having a portable/script version of nsinstall available would remove the dependency of having to pre-build nsinstall before the tool can be used to install files.

Comment 4

[Joey Armstrong [:joey]]

(In reply to Joey Armstrong [:joey] from comment #3)
> Having a portable/script version of nsinstall available would remove the
> dependency of having to pre-build nsinstall before the tool can be used to
> install files.

Also why would we want to maintain a platform specific python tool in the sandbox ?  If nsinstall.py were generalized that script could be used to avoid the overhead of having to build multiple copies of the nsinstall binary:

% find obj-x86_64-unknown-linux-gnu  -name 'nsinstall'
obj-x86_64-unknown-linux-gnu/js/src/config/nsinstall
obj-x86_64-unknown-linux-gnu/config/nsinstall
obj-x86_64-unknown-linux-gnu/dist/bin/nsinstall
obj-x86_64-unknown-linux-gnu/nsprpub/config/nsinstall

Comment 5

[Mike Hommey [:glandium]]

(In reply to Joey Armstrong [:joey] from comment #3)
> Having a portable/script version of nsinstall available would remove the
> dependency of having to pre-build nsinstall before the tool can be used to
> install files.

Actually, we use nsinstall.py when nsinstall is not built yet. But we never run nsinstall before nsinstall.py for a same file.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Joey Armstrong [:joey] from comment #4)
> (In reply to Joey Armstrong [:joey] from comment #3)
> > Having a portable/script version of nsinstall available would remove the
> > dependency of having to pre-build nsinstall before the tool can be used to
> > install files.
> 
> Also why would we want to maintain a platform specific python tool in the
> sandbox ?

Because starting a python tool that is used thousands of times is significantly slower than starting a native tool, on unix systems. On windows, it's not true, but we have the advantage of not having to actually spawn a new process each time, under pymake.