Closed Bug 798969 Opened 8 years ago Closed 6 years ago

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

Categories

(Core :: WebRTC, defect)

18 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: roc, Assigned: roc)

Details

(Whiteboard: [WebRTC], [blocking-webrtc-])

Attachments

(1 file)

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.
Using .. to navigate out of $(topsrcdir) seems like the wrong thing to do.
Assignee: nobody → rjesup
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.
Assignee: rjesup → nobody
Attached patch fixSplinter Review
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.
Assignee: nobody → roc
Attachment #668966 - Flags: review?(rjesup)
./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 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?"
(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.
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.
Whiteboard: [WebRTC], [blocking-webrtc-]
Attachment #668966 - Flags: review?(rjesup) → review?(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.
Attachment #668966 - Flags: review?(ted.mielczarek) → review+
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.)
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.
overtaken by events
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.