:Ms2ger (he/him; ⌚ UTC+1/+2) Reporter
	Description • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/614052b6cbcc
https://tbpl.mozilla.org/php/getParsedLog.php?id=27018465&branch=mozilla-inbound

	Gregory Szorc [:gps]
	Comment 1 • 11 years ago

Possible regression from bug 850380. The .xpt is regenerated but the (presumably changed) .h file isn't copied into $(DIST)/include.

	Masatoshi Kimura [:emk]
	Comment 2 • 11 years ago

Bug 908977 didn't fix this yet?

	Gregory Szorc [:gps]
	Comment 3 • 11 years ago

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Bug 908977 didn't fix this yet?

No. This is probably a dupe of that bug. We'll keep this one open just in case.

	Gregory Szorc [:gps]
	Comment 4 • 11 years ago

I am unable to reproduce this behavior locally. I built 614052b6cbcc^1 then performed a build of the XPIDL subtier:

make.py[0]: Entering directory 'c:\dev\src\mozilla-central\obj-ff\config/makefiles/precompile'
make.py[1]: Entering directory 'c:\dev\src\mozilla-central\obj-ff\config\makefiles\precompile\../../../xpcom/idl-parser'
make.py[1]: Leaving directory 'c:\dev\src\mozilla-central\obj-ff\config\makefiles\precompile\../../../xpcom/idl-parser'
c:\dev\src\mozilla-central\obj-ff\config\makefiles\precompile\Makefile:43:0$ c:/dev/src/mozilla-central/obj-ff/_virtualenv/Scripts/python.exe -m mozbuild.action.process_install_manifest ../../../dist/idl ../../../_build_manifests/install/dist_idl
From ../../../dist/idl: Kept 1151 existing; Added/updated 2; Removed 0 files and 0 directories.
make.py[1]: Entering directory 'c:\dev\src\mozilla-central\obj-ff\config\makefiles\precompile\../../../config/makefiles/xpidl'
dom_camera.xpt
c:\dev\src\mozilla-central\obj-ff\config\makefiles\xpidl\Makefile:248:0$ c:/dev/src/mozilla-central/obj-ff/_virtualenv/Scripts/python.exe c:/dev/src/mozilla-central/config/pythonpath.py -Ic:/dev/src/mozilla-central/other-licenses/ply -Ic:/dev/src/mozilla-central/xpcom/idl-parser -I../../../xpcom/idl-parser c:/dev/src/mozilla-central/python/mozbuild/mozbuild/action/xpidl-process.py --cache-dir ../../../xpcom/idl-parser ../../../dist/idl headers xpt .deps dom_camera nsIDOMCameraManager
c:\dev\src\mozilla-central\obj-ff\config\makefiles\xpidl\Makefile:1080:0$ c:/dev/src/mozilla-central/obj-ff/_virtualenv/Scripts/python.exe c:/dev/src/mozilla-central/config/nsinstall.py -t headers/nsIDOMCameraManager.h ../../../dist/include
make.py[1]: Leaving directory 'c:\dev\src\mozilla-central\obj-ff\config\makefiles\precompile\../../../config/makefiles/xpidl'
make.py[0]: Leaving directory 'c:\dev\src\mozilla-central\obj-ff\config/makefiles/precompile'

My local build is exhibiting the proper behavior: the xpt is regenerated and the changed .h file is copied into dist/include.

I'm not sure what to make of this. Are we looking at a timing bug?

	Mike Hommey [:glandium]
	Comment 5 • 11 years ago

(In reply to Gregory Szorc [:gps] from comment #4)
> I am unable to reproduce this behavior locally. I built 614052b6cbcc^1

Note that the build slave was most probably not on that changeset before updating its tree.

	Phil Ringnalda (:philor)
	Comment 6 • 11 years ago

The opt build's slave was previously on https://hg.mozilla.org/integration/mozilla-inbound/rev/8df4a9443d38, the debug build's slave was previously on https://hg.mozilla.org/integration/mozilla-inbound/rev/05670d699bdc.

	Masatoshi Kimura [:emk]
	Comment 7 • 11 years ago

The build system failed to detect nsIMemoryReporter.idl update on my local environment. Although it copied the idl to dist/idl, dist/include/nsIMemoryReporter.h was not updated.

	Gregory Szorc [:gps]
	Comment 8 • 11 years ago

I performed a clobber of 8df4a9443d38, updated to and built 614052b6cbcc, and the output was the same as comment #4. I'm still unable to reproduce this.

If someone can reliably reproduce this, please attach the output from the following:

  $ path/to/pymake/make.py -d -C config/makefiles/precompile xpidl 2>&1

	Honza Bambas (:mayhemer)
	Comment 10 • 11 years ago

Gregory, are you sure the duplication is right?  I have a problem with my local build, I was reporting a generic issue that makes my life as a developer much harder.

	Gregory Szorc [:gps]
	Comment 11 • 11 years ago

(In reply to Honza Bambas (:mayhemer) from comment #10)
> Gregory, are you sure the duplication is right?  I have a problem with my
> local build, I was reporting a generic issue that makes my life as a
> developer much harder.

Yes. There's apparently a dependency issue where .idl updates aren't getting propagated on Windows machines.

	Gregory Szorc [:gps]
	Comment 12 • 11 years ago

I was thinking that we could potentially use mozpack's FileCopier along with the work in bug 911362 to manage installing the .h files into dist/include.

Or, once bug 896797 lands (complete with optional files listed for the .idl-derived headers, I believe we can go back to generating the .h files directly into dist/include. That would avoid having to copy the .h files after generation. Wins all around.

	Honza Bambas (:mayhemer)
	Comment 13 • 11 years ago

What I did:
- added a simple method void foo() + iid change at network/cache/nsICacheService.idl
- ./mach build netwerk/cache/
  - expected is to pick up the iid change here and rebuild but ends up with 'Foo' not defined in the nsCacheService (the class implementing the interface)
- pymake -d -C _obj-browser-debug/config/makefiles/precompile xpidl 2>precompile
  - 'precompile' is the attached file
- ./mach build netwerk/cache/
  - now it works, idl change picked up


So, do I really need to rebuild idls with that horrible magic command 'pymake -d -C _obj-browser-debug/config/makefiles/precompile xpidl' since now?  I hope not...

True is that on Linux this works for me (to build the network/cache submodule is enough to pick the iid changes up).

	Gregory Szorc [:gps]
	Comment 14 • 11 years ago

Relevant lines from log:

INFO:pymake.data:Remaking xpt/necko_cache.xpt using rule at c:\Mozilla\src\mozilla-central\_obj-browser-debug\config\makefiles\xpidl\Makefile:684:0 because ../../../dist/idl/nsICacheService.idl is newer.
DEBUG:pymake.process:c:\Mozilla\src\mozilla-central\_obj-browser-debug\config\makefiles\xpidl\Makefile:685:0: using shell: command starts with shell primitive 'echo': 'echo "necko_cache.xpt"'
INFO:pymake.data: Considering target '../../../dist/include/nsICacheService.h'
INFO:pymake.data: Considering target 'headers/nsICacheService.h'
INFO:pymake.data: Searching for implicit rule to make 'headers/nsICacheService.h'
INFO:pymake.data: Couldn't find implicit rule to remake 'headers/nsICacheService.h'
INFO:pymake.data:Not remaking headers/nsICacheService.h ubecause it would have no effect, even though xpt/necko_cache.xpt is newer.
INFO:pymake.data:Remaking ../../../dist/include/nsICacheService.h using rule at c:\Mozilla\src\mozilla-central\_obj-browser-debug\config\makefiles\xpidl\Makefile:1085:0 because headers/nsICacheService.h is newer.

This /seems/ to be doing all the right things. We reprocess the XPIDL module because nsICacheService.idl has changed. And, it detects the dist/include header is out of date, so it says it's going to install it. However - and this is the weird bit - I don't see a reference to nsinstall in the log! I'm not sure if I'm just missing it, if it wasn't captured in this log, if pymake just isn't printing it, or if it never actually ran. But, since the dist/include header is not updated (otherwise the build should work), I'm inclined to think pymake isn't executing the command. This therefore smells like a pymake bug! Although, I'm still unable to reproduce this locally. Most weird.

	Mike Hommey [:glandium]
	Comment 15 • 11 years ago

I think pymake doesn't output anything for native commands.

	Masatoshi Kimura [:emk]
	Comment 16 • 11 years ago

Why pymake is executing the native nsinstall instead of config/nsinstall.py?

	Masatoshi Kimura [:emk]
	Comment 17 • 11 years ago

I renamed mozilla-build\moztools\bin, but no error is still produced.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 18 • 11 years ago

I have seen a similar problem with WebIDL.  The generated header is updated in objdir/dom/bindings but not copied to dist/include.

I don't see any reason to think this is a pymake bug.  It's not going to manifest on unix platforms because of symlinking ...

	Mark Hammond [:markh] [:mhammond]
	Comment 19 • 11 years ago

This has been biting me regularly for the last week or so...

	Honza Bambas (:mayhemer)
	Comment 20 • 11 years ago

So... to put you bit on a right way (and repeat my self):

- modifying an .idl file in sub-module and building that sub-module doesn't pick the .idl change
- pymake config/makefiles/precompile xpidl DOES the right thing:
  - detects .idl change
  - recompiles it
  - nsinstalls the .h file to all the right places

- building the sub-module AFTER runnign `pymake ... xpidl` WORKS, since the .h file has updated as expected


If something is wrong here, then it's `mach build submodule/` not catching the idl change (i.e. PROBABLY not running the `xpidl` target properly).

Unfortunatelly it's hard to collect debug output from `pymake -d -C _obj/netwerk/cache` for my idl change test case... 9999 lines of console window is not enough, redirect doesn't work...

	Ed Morley [:emorley]
	Comment 21 • 11 years ago

This is failures (and in some cases mistaken backouts) in automation - please can you take a look/advise what this is depending on? :-)

	Ed Morley [:emorley]
	Comment 22 • 11 years ago

s/failures/causing failures/

	Rodrigo Silveira [:rsilveira] [:rodms]
	Comment 23 • 11 years ago

(In reply to Honza Bambas (:mayhemer) from comment #13)
> Created attachment 798260 [details]
> precompile.zip
> 
> What I did:
> - added a simple method void foo() + iid change at
> network/cache/nsICacheService.idl
> - ./mach build netwerk/cache/
>   - expected is to pick up the iid change here and rebuild but ends up with
> 'Foo' not defined in the nsCacheService (the class implementing the
> interface)
> - pymake -d -C _obj-browser-debug/config/makefiles/precompile xpidl
> 2>precompile
>   - 'precompile' is the attached file
> - ./mach build netwerk/cache/
>   - now it works, idl change picked up
> 
> 
> So, do I really need to rebuild idls with that horrible magic command
> 'pymake -d -C _obj-browser-debug/config/makefiles/precompile xpidl' since
> now?  I hope not...
> 
> True is that on Linux this works for me (to build the network/cache
> submodule is enough to pick the iid changes up).

The fix in bug 921070 removed the precompile tier, so the "pymake -d -C _obj-browser-debug/config/makefiles/precompile xpidl" workaround stopped working.

Replacing with a "pymake -C obj-i686-pc-mingw32/xpcom/xpidl/" worked for me.

	Gregory Szorc [:gps]
	Comment 24 • 11 years ago

I'm a bit confused as to what the current state is.

The initial comment dealt with a failure in release automation. edmorley needinfo'd me in comment #21 because of impact on automation. However, I believe all impact on automation was fixed with bug 908977. There have been no reports of XPIDL issues in automation since bug 908977 landed AFAICT.

There are a few other people complaining about local builds. However, the backend.mk for directories containing XPIDL files contains a rule to invoke |make -C xpcom/xpidl export|. This should do everything necessary to rebuild XPIDLs.

That being said, I'm running things locally and pymake doesn't appear to be running the rules from nonrecursive.mk! I'll dig into this.

	Gregory Szorc [:gps]
	Comment 25 • 11 years ago

Bug 922685 just landed in inbound. This bug should be fixed.

I verified bug 922685's patch fixed the purported problem on pymake and OS X. However, I stopped short of verifying on Windows because I'm away from my Windows machine.

Could one of the original reporters please verify and close?

	Honza Bambas (:mayhemer)
	Comment 26 • 11 years ago

- applied https://hg.mozilla.org/integration/mozilla-inbound/raw-rev/f90880fb5248 to my m-c tree (the patch is not there by now)
- ran |./mach build build/| to rebuild pymake
- ran |./mach build network/cache2| while there were a modified .idl file
- the changes to idl file took affect, it was recompiled as expected :)

VERIFIED

(And w/o the patch, the changes are not picked up)

	Gregory Szorc [:gps]
	Comment 27 • 11 years ago

Then it sounds like this can be closed.