1045329 - compile_targets needs to take into account comm-central

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ian Neal]

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

Comment 1

[Ian Neal]

that I am unsure how to progress with.

Comment 2

[Ian Neal]

Attachment: 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 ../

Comment 3

[Ian Neal]

Attachment: Example root-deps.mk

root-deps.mk generated. Potential issues with some of the compiles/targets

Comment 4

[Mike Hommey [:glandium]]

(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.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Use paths relative to the root topobjdir for the compile targets

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.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8463751 [details] [diff] [review]
Use paths relative to the root topobjdir for the compile targets

Ian, please test this.

Comment 7

[Florian Quèze [:florian]]

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?

Comment 8

[Mike Hommey [:glandium]]

At the very least bug 1043344, but there's now also 1043862, 1043865, and 1043869.

Comment 9

[Ian Neal]

(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

Comment 10

[Florian Quèze [:florian]]

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 11

[Ian Neal]

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.

Comment 12

[Philip Chee]

(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'

Comment 13

[Florian Quèze [:florian]]

(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.

Comment 14

[Mike Hommey [:glandium]]

What Florian says.

Comment 15

[Mike Hommey [:glandium]]

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

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2cb156aad0b