798969 - Build broken due to $(OBJDIR)/media/webrtc/trunk/common.mk dependency on mozmake.py being structured incorrectly

Status: RESOLVED WONTFIX

(Core :: WebRTC, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My copy of the file has
$(topsrcdir)/../../../mnt/hgfs/Z/roc/mozilla-central/media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py
and $(topsrcdir) is /home/roc/mozilla-central ... which is a symlink to /mnt/hgfs/Z/roc/mozilla-central. So this path resolves to /mnt/hgfs/mnt/hgfs/Z/roc/mozilla-central/media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py, which does not exist.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Using .. to navigate out of $(topsrcdir) seems like the wrong thing to do.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The offending path is generated by
  def topsrcdir_path(path):
    return "$(topsrcdir)/" + swapslashes(gyp.common.RelativePath(path, topsrcdir))
called by
  scriptname = topsrcdir_path(__file__)

I guess __file__ is giving us /mnt/hgfs/Z/roc/mozilla-central/media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py, and then making that relative to /home/roc/mozilla-central does what we see here.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Not the most elegant approach I guess, since this will have to be updated if the files get moved around in the tree, but it's simple enough.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

./security/nss/Makefile:NSPR_PREFIX = $$(topsrcdir)/../dist/$(OBJDIR_NAME)

Though I get your point.

The source of the problem is using RelativePath()

Passing more info into the gyp generator might help.

I think your patch will work for now at least.  I'm worried that other instances using the relative paths will be wrong, but we can see.

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 668966 [details] [diff] [review]
fix

Review of attachment 668966 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py
@@ +439,5 @@
>                  depth,
>                  swapslashes(top),
>                  swapslashes(src),
>                  swapslashes(relative_srcdir))
> +  scriptname = "$(topsrcdir)/media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py"

I don't like this because it hardcodes the path to mozmake.py inside the file itself. Can you not use os.path.abspath, os.path.normpath, os.path.realpath, etc with __file__ to get the right path "format?"

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Gregory Szorc [:gps] from comment #5)
> I don't like this because it hardcodes the path to mozmake.py inside the
> file itself. Can you not use os.path.abspath, os.path.normpath,
> os.path.realpath, etc with __file__ to get the right path "format?"

I don't see how any of those could help here. normpath specifically advises "It should be understood that this may change the meaning of the path if it contains symbolic links!" which is precisely what happens here.

We might be able to make gyp.common.RelativePath(path, base) handle an absolute 'path' by calling realpath(base) and then making 'path' be relative to that. But I don't know whether it's a good idea to make RelativePath depend on the current state of the file system in general.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

I think we should take roc's patch as a bandaid for developers with complex filesystem structures, and then try to solve this in a better way.  Might help with the other symlink/absolute-objdir issues as well.

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 668966 [details] [diff] [review]
fix

Review of attachment 668966 [details] [diff] [review]:
-----------------------------------------------------------------

I don't like this either, but I don't want to block people doing useful work on a perfect solution right now. Let's land this and either keep this bug open or file a new bug to do this right.

I looked at this code again yesterday and couldn't come up with a simple solution to do the right thing without breaking other weird cases. We'll sort this out, but not right now.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e84a64d3d4

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This was backed out because it broke builds. I'm not sure how.
https://tbpl.mozilla.org/php/getParsedLog.php?id=16012336&tree=Mozilla-Inbound&full=1

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

I'm pretty sure this change requires a clobber when applied to the tree, because this affects how the makefiles are built, and because I believe touching mozmake.py doesn't force a clobber automatically.  (The current bug on modifying gyp files not forcing rebuilds correctly might also be involved.)

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

The current patch here (ugly fix) is included in the patch for bug 800847; we may want to keep this open for solving it in a cleaner fashion.

Comment 13

[(not currently active) Ted Mielczarek]

Possibly related: bug 800546

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

overtaken by events