Open
Bug 680968
Opened 13 years ago
Updated 2 years ago
Support symlinks in nsinstall.py
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
NEW
People
(Reporter: gps, Unassigned)
References
(Depends on 1 open bug)
Details
Attachments
(3 obsolete files)
This is a follow-up from bug 680636 to have the Python version of nsinstall, nsinstall.py, have feature parity with the original C version with regards to symlinks.
Currently, nsinstall.py ignores symlinks options. It should be made to honor them.
Reporter | ||
Comment 1•13 years ago
|
||
I think I fat fingered the bug number when I created this bug.
Reporter | ||
Comment 2•13 years ago
|
||
This patch adds basic symlink support to nsinstall.py. If -l, -L, or -R are specified, it creates symlinks instead of copying files. The expected behavior of -R is ignored. I /think/ this is acceptable. If you think it is needed and can explain how it is supposed to work, I can code it up.
In nsinstall.c, the mtime of the symlink and its source are compared when determining whether to unlink the existing symlink. I did not port this. This /could/ have an impact on makefile freshness determination. But, I /think/ that would only come into play if the tester were using lstat() instead of stat(). I'm not sure what make/pymake uses internally. If you don't want to risk it, I could certainly port this logic forward.
I have this patch applied on top of one for bug 680636 and the tree appears to mostly build fine! Although, I did discover that nsinstall.py isn't performing thread-safe directory creation (os.mkdir will raise if the path exists and multiple nsinstall.py invocations could happen in parallel). I can make the necessary changes in this patch or deal with it elsewhere.
Finally, I don't like the Python style used in this patch. But, it matches what existed previously. To fix the style would require rewriting the entire file. I'm not about to make Ted review a completely new nsinstall.py ;)
Comment 3•13 years ago
|
||
I've added a "win32" Python module dir and moved all the win32 junk we had in JarMaker.py into modules in there.
Assignee: gps → sagarwal
Attachment #590971 -
Attachment is obsolete: true
Attachment #590971 -
Flags: review?(ted.mielczarek)
Comment 4•13 years ago
|
||
Ugh, this is never going to work on Windows. rm -rf follows junctions :(
Comment 5•13 years ago
|
||
Attachment #596146 -
Attachment is obsolete: true
Note, should this come up again -- doing this with junctions isn't the right way. NTFS has real symbolic links since Vista; see http://msdn.microsoft.com/en-us/library/windows/desktop/aa365682%28v=vs.85%29.aspx for details.
Reporter | ||
Comment 7•11 years ago
|
||
In my ideal world nsinstall and its Python variant are obsoleted in favor of the installation code in mozpack (python/mozbuild/mozpack). The code in the latter is more intelligent about copying only when things change, etc.
Copying doesn't help; we need actual symlinks for developers so that dependencies on header files work properly.
Reporter | ||
Comment 9•11 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #8)
> Copying doesn't help; we need actual symlinks for developers so that
> dependencies on header files work properly.
That's why we are switching to using manifests for managing header installs (bug 896797). Up until bug 884587, we completely blew away dist/include at the top of builds, forcing file re-copy. We now maintain manifests of which files actually belong so as to not incur unnecessary copying. The next step is to actually install these files with install manifests instead of nsinstall.
Comment 10•11 years ago
|
||
(In reply to comment #9)
> (In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #8)
> > Copying doesn't help; we need actual symlinks for developers so that
> > dependencies on header files work properly.
>
> That's why we are switching to using manifests for managing header installs
> (bug 896797). Up until bug 884587, we completely blew away dist/include at the
> top of builds, forcing file re-copy. We now maintain manifests of which files
> actually belong so as to not incur unnecessary copying. The next step is to
> actually install these files with install manifests instead of nsinstall.
The point is that we want to avoid the copy stage necessary to make the change made to a header in your srcdir be visible in the version in objdir/dist/include.
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•6 years ago
|
Attachment #596207 -
Attachment is obsolete: true
Updated•6 years ago
|
Assignee: sid.bugzilla → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 11•6 years ago
|
||
I have patches in bug 1382697 that support this.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•