Closed Bug 1045329 Opened 6 years ago Closed 6 years ago

compile_targets needs to take into account comm-central

Categories

(Firefox Build System :: General, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: iann_bugzilla, Assigned: glandium)

References

Details

Attachments

(3 files)

When the patch on bug 1043344 landed there were three areas that did not take into account comm-central:
+                            for target in targets:
+                                t = '%s/target' % mozpath.relpath(objdir,
+                                    bf.environment.topobjdir)

+            for d in dirs + obj.tier_static_dirs[tier]:
+                if d.trigger:
+                    self._triggers[d.trigger].add('%s/target' % d)

+    @staticmethod
+    def _build_target_for_obj(obj):
+        return '%s/%s' % (obj.relobjdir, obj.KIND)

They probably need something similar to:
if lib.objdir.startswith(topobjdir + '/'):
    return '$(DEPTH)/%s' % lib.relobjdir
else:
    return lib.relobjdir

The first and third seem fairly simple, it is the second one
that I am unsure how to progress with.
Attached file Example root.mk
This is a root.mk generated.
If you compare the compile_targets to say libs_dirs, you will notice none of the comm-central entries have any ../
Attached file Example root-deps.mk
root-deps.mk generated. Potential issues with some of the compiles/targets
(In reply to Ian Neal from comment #0)
> When the patch on bug 1043344 landed there were three areas that did not
> take into account comm-central:
> +            for d in dirs + obj.tier_static_dirs[tier]:
> +                if d.trigger:
> +                    self._triggers[d.trigger].add('%s/target' % d)

This properly has ../

> +                            for target in targets:
> +                                t = '%s/target' % mozpath.relpath(objdir,
> +                                    bf.environment.topobjdir)

This one doesn't
 
> +    @staticmethod
> +    def _build_target_for_obj(obj):
> +        return '%s/%s' % (obj.relobjdir, obj.KIND)

Neither does this one.

> They probably need something similar to:
> if lib.objdir.startswith(topobjdir + '/'):
>     return '$(DEPTH)/%s' % lib.relobjdir
> else:
>     return lib.relobjdir

The comment in that function is probably outdated and irrelevant. That is, "something similar to" that would not fix the problem. I have an idea.
While most environments are using the root topobjdir, some can be using a
different path. This happens for comm-central files. For those, the compile
target needs to have paths relative to the root topobjdir instead of relative
to the comm-central topobjdir.
Attachment #8463751 - Flags: review?(gps)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Comment on attachment 8463751 [details] [diff] [review]
Use paths relative to the root topobjdir for the compile targets

Ian, please test this.
Attachment #8463751 - Flags: feedback?(iann_bugzilla)
With the patch applied I get this new error:
make[5]: *** No rule to make target `target'.  Stop.
make[4]: *** [../im/app/target] Error 2
make[4]: *** Waiting for unfinished jobs....

Without the patch I was getting:
make: *** im/app: No such file or directory.  Stop.
make[5]: *** [im/app/target] Error 2
make[5]: *** Waiting for unfinished jobs....

Is there a patch that needs to be ported to c-c to fix this new error?
At the very least bug 1043344, but there's now also 1043862, 1043865, and 1043869.
(In reply to Florian Quèze [:florian] [:flo] from comment #7)
> With the patch applied I get this new error:
> make[5]: *** No rule to make target `target'.  Stop.
> make[4]: *** [../im/app/target] Error 2
> make[4]: *** Waiting for unfinished jobs....
> 
> Without the patch I was getting:
> make: *** im/app: No such file or directory.  Stop.
> make[5]: *** [im/app/target] Error 2
> make[5]: *** Waiting for unfinished jobs....
> 
> Is there a patch that needs to be ported to c-c to fix this new error?

Yes, but the issue in this bug was stopping me from progressing further with bug 1044460
If I copy the 3 rules at https://bugzilla.mozilla.org/attachment.cgi?id=8461564&action=diff#a/config/rules.mk_sec3 to have the "target" rule, my build then fails with:
error: error opening '.deps/nsMain.o.pp': Error opening output file '.deps/nsMain.o.pp': No such file or directory

This looks a lot like bug 1043869. So yeah, I think the patch here works, and we just need to continue porting other changes.
Comment on attachment 8463751 [details] [diff] [review]
Use paths relative to the root topobjdir for the compile targets

Yes, that fixes the issue for me. Thanks.
Attachment #8463751 - Flags: feedback?(iann_bugzilla) → feedback+
(In reply to Ian Neal from comment #11)
> Comment on attachment 8463751 [details] [diff] [review]
> Use paths relative to the root topobjdir for the compile targets
> 
> Yes, that fixes the issue for me. Thanks.

This doesn't fix the issue for me:

c:/t1/hg/objdir-sm/mozilla/_virtualenv/Scripts/python.exe c:/t1/hg/comm-central/mozilla/config/expandlibs_gen.py --depend .deps/db_mork_src.lib.pp -o db_mork_src.lib.desc morkArray.obj morkAtom.obj morkAtomMap.obj morkAtomSpace.obj morkBead.obj morkBlob.obj morkBuilder.obj morkCell.obj morkCellObject.obj morkCh.obj morkConfig.obj morkCursor.obj morkDeque.obj morkEnv.obj morkFactory.obj morkFile.obj morkHandle.obj morkIntMap.obj morkMap.obj morkNode.obj morkNodeMap.obj morkObject.obj morkParser.obj morkPool.obj morkPortTableCursor.obj morkProbeMap.obj morkRow.obj morkRowCellCursor.obj morkRowMap.obj morkRowObject.obj morkRowSpace.obj morkSink.obj morkSpace.obj morkStore.obj morkStream.obj morkTable.obj morkTableRowCursor.obj morkThumb.obj morkWriter.obj morkYarn.obj morkZone.obj orkinHeap.obj morkSearchRowCursor.obj
Usage: expandlibs_gen.py [options]

expandlibs_gen.py: error: no such option: --depend
c:/t1/hg/comm-central/config/rules.mk:766: recipe for target 'db_mork_src.lib.desc' failed
mozmake[2]: *** [db_mork_src.lib.desc] Error 2
mozmake[2]: Leaving directory 'c:/t1/hg/objdir-sm/db/mork/src'
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(iann_bugzilla)
(In reply to Philip Chee from comment #12)

> This doesn't fix the issue for me:
> 
> c:/t1/hg/objdir-sm/mozilla/_virtualenv/Scripts/python.exe
> c:/t1/hg/comm-central/mozilla/config/expandlibs_gen.py --depend
> .deps/db_mork_src.lib.pp -o db_mork_src.lib.desc morkArray.obj morkAtom.obj
> morkAtomMap.obj morkAtomSpace.obj morkBead.obj morkBlob.obj morkBuilder.obj
> morkCell.obj morkCellObject.obj morkCh.obj morkConfig.obj morkCursor.obj
> morkDeque.obj morkEnv.obj morkFactory.obj morkFile.obj morkHandle.obj
> morkIntMap.obj morkMap.obj morkNode.obj morkNodeMap.obj morkObject.obj
> morkParser.obj morkPool.obj morkPortTableCursor.obj morkProbeMap.obj
> morkRow.obj morkRowCellCursor.obj morkRowMap.obj morkRowObject.obj
> morkRowSpace.obj morkSink.obj morkSpace.obj morkStore.obj morkStream.obj
> morkTable.obj morkTableRowCursor.obj morkThumb.obj morkWriter.obj
> morkYarn.obj morkZone.obj orkinHeap.obj morkSearchRowCursor.obj
> Usage: expandlibs_gen.py [options]
> 
> expandlibs_gen.py: error: no such option: --depend
> c:/t1/hg/comm-central/config/rules.mk:766: recipe for target
> 'db_mork_src.lib.desc' failed
> mozmake[2]: *** [db_mork_src.lib.desc] Error 2
> mozmake[2]: Leaving directory 'c:/t1/hg/objdir-sm/db/mork/src'

This looks like a different issue, likely caused by bug 1043869.
What Florian says.
Flags: needinfo?(mh+mozilla)
Attachment #8463751 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/c2cb156aad0b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Flags: needinfo?(iann_bugzilla)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.