1051809 - Upload zipped archive of all .gcno files to build directory if code coverage is enabled

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

To get code coverage builds going in tbpl, we need to upload all .gcno files (while preserving directory structure) to ftp.m.o. My understanding is the upload step happens in the build system itself.

From #releng:

<ahal> so with the new mozharness build scripts, does the uploading of the installer/tests to ftp.m.o happen in mozharness? Or is that done by buildbot
<jlund> ahal: so 'mach build' does almost all the post build things :)
<mshal> well, we haven't enabled the 'mach build'-does-all-the-things yet in m-c :)
<mshal> if it's a new step entirely we'll want to add it to moz-automation.mk
<ahal> mshal: I'm trying to get code coverage builds stood up.. after the build we'll need to walk the directory tree, collect all .gcov files, zip them up and upload them to the build dir
<mshal> ahal: ahh. Well you might want to look at the top-level Makefile.in at a target like buildsymbols
<mshal> ahal: and basically create a new target that does the steps you want. Then we can work that into the automation
<mshal> ahal: you can see the output from buildsymbols (SYMBOL_ARCHIVE_BASENAME.zip) gets added to UPLOAD_FILES in m-c/toolkit/mozapps/installer/packager.mk. You'd probably want the same with your gcov zip file if you want it in the same place
<mshal> (same place on ftp, I mean)

Comment 1

[(not currently active) Ted Mielczarek]

A cheap way to do this would be to tack something on to the end of the build that tars up all the files, and puts the resulting tar file in $(DIST), and then you should just be able to list the file in MOZ_UPLOAD_FILES:
http://hg.mozilla.org/mozilla-central/annotate/a9b43778f0c2/toolkit/mozapps/installer/packager.mk#l876

Comment 2

[Mike Hommey [:glandium]]

Are you going to *run* stuff during the build? I don't think you would, so *gcno files wouldn't be a byproduct of the build, but of test runs. So this is quite a different setup to get those back.

Comment 3

[Joshua Cranmer [:jcranmer]]

(In reply to Mike Hommey [:glandium] from comment #2)
> Are you going to *run* stuff during the build? I don't think you would, so
> *gcno files wouldn't be a byproduct of the build, but of test runs. So this
> is quite a different setup to get those back.

.gcno files are a byproduct of the build.
.gcda files are a byproduct of the tests.

Both are needed.

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Add make target for building a .gcno tree

This patch adds a make target for finding, copying and compressing all the .gcno files in the objdir.

A few notes:
1) This assumes a MOZ_CODE_COVERAGE variable gets defined, so depends on bug 1054275.
2) I dumped a python script in tools/code-coverage, but maybe this should be in mozbuild somewhere?
3)  I'm still trying to figure out how to get the build system to invoke this automatically after the build is complete (I think it's something to do with build/moz-automation.mk).

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8473829 [details] [diff] [review]
Add make target for building a .gcno tree

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

::: Makefile.in
@@ +257,5 @@
> +
> +	@echo packing code-coverage gcno tree
> +	$(NSINSTALL) -D $(DIST)/$(PKG_PATH)
> +	cd $(DIST)/code-coverage-gcno && \
> +		  zip -r9D '../$(PKG_PATH)$(CODE_COVERAGE_ARCHIVE_BASENAME).zip' .

We decided a while ago that we're trying to do away with all this "shell in makefile" foo and replace it with Python, preferably Python living in importable modules (as opposed to scripts because reusability).

You can use the zipfile module (https://docs.python.org/2/library/zipfile.html) to produce zip files directly from Python. Using it, you will avoid the I/O of staging .gcno files to a common directory. You can just write out a .zip file into its final location while reading .gcno files from wherever they reside.

This approach is also more testable. We like test coverage [of code coverage].

::: tools/code-coverage/build_gcno_tree.py
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +import argparse

Let's stick the logic in this file and from Makefile.in in mozbuild.codecoverage (python/mozbuild/mozbuild/codecoverage).

@@ +11,5 @@
> +
> +def build_gcno_tree(root, outdir):
> +    finder = FileFinder(root)
> +    copier = FileCopier()
> +    for p, f in finder.find("**/*.gcno"):

Filesystem walking is a mostly legitimate solution. However, the build system already knows what binaries it creates as part of the build. Can we not produce a set of .gcno files during config.status and look for them directly? This has the following advantages:

1) Faster. Filesystem walking can be expensive.
2) Only includes files that are part of the build.

Comment 6

[Mike Hommey [:glandium]]

Instead of using the zipfile module, you should use the Jarrer class in mozbuild.mozpack.copier, with optimize=False.

Comment 7

[Joshua Cranmer [:jcranmer]]

(In reply to Gregory Szorc [:gps] from comment #5)
> Filesystem walking is a mostly legitimate solution. However, the build
> system already knows what binaries it creates as part of the build. Can we
> not produce a set of .gcno files during config.status and look for them
> directly? This has the following advantages:

That implies that you absolutely know every source file [that you care about] that gets compiled from moz.build. Note that NSPR and NSS, among other libraries, are not in this category. Whether or not you care about not being able to measure coverage from NSPR, NSS, et al is a different question.

Comment 8

[Andrew Halberstadt [:ahal]]

Thanks for the feedback! I was just copying the buildsymbols make target as it was doing something similar to what I needed.

(In reply to Gregory Szorc [:gps] from comment #5)
> ::: Makefile.in
> @@ +257,5 @@
> > +
> > +	@echo packing code-coverage gcno tree
> > +	$(NSINSTALL) -D $(DIST)/$(PKG_PATH)
> > +	cd $(DIST)/code-coverage-gcno && \
> > +		  zip -r9D '../$(PKG_PATH)$(CODE_COVERAGE_ARCHIVE_BASENAME).zip' .
> 
> We decided a while ago that we're trying to do away with all this "shell in
> makefile" foo and replace it with Python, preferably Python living in
> importable modules (as opposed to scripts because reusability).
> 
> Let's stick the logic in this file and from Makefile.in in
> mozbuild.codecoverage (python/mozbuild/mozbuild/codecoverage).

Sounds good. To get this running as a build step, would I invoke this from http://mxr.mozilla.org/mozilla-central/source/build/moz-automation.mk ? Or is there some place in python I can hook in from?

> Filesystem walking is a mostly legitimate solution. However, the build
> system already knows what binaries it creates as part of the build. Can we
> not produce a set of .gcno files during config.status and look for them
> directly? This has the following advantages:
> 
> 1) Faster. Filesystem walking can be expensive.
> 2) Only includes files that are part of the build.

Assuming the point :jcranmer raised isn't a concern, this makes sense to me. I have no idea how to do that though :p.

Comment 9

[Michael Shal [:mshal]]

(In reply to Andrew Halberstadt [:ahal] from comment #8)
> > Let's stick the logic in this file and from Makefile.in in
> > mozbuild.codecoverage (python/mozbuild/mozbuild/codecoverage).
> 
> Sounds good. To get this running as a build step, would I invoke this from
> http://mxr.mozilla.org/mozilla-central/source/build/moz-automation.mk ? Or
> is there some place in python I can hook in from?

The moz-automation.mk file is something new that we're trying, but it currently is only used on cedar. You'll want to add it in as a new step in buildbot (see: http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#1081 ). Though if you are able to add it to moz-automation.mk as well, that would be appreciated :)

Comment 10

[Gregory Szorc [:gps]]

Do we need to add this to buildbot? I would think that we could overload |make package| to create the .zip and add it to the uploaded files set. But I'm not an expert in this part of the build system...

Comment 11

[Michael Shal [:mshal]]

(In reply to Gregory Szorc [:gps] from comment #10)
> Do we need to add this to buildbot? I would think that we could overload
> |make package| to create the .zip and add it to the uploaded files set. But
> I'm not an expert in this part of the build system...

Sure, you can probably do that instead. It would be simpler since you don't have to fiddle with buildbot/moz-automation, but just might be overloading the term "package". I don't have a strong preference.

Comment 12

[(not currently active) Ted Mielczarek]

There's precedent here, FWIW. We use stage-package to package the JS shell which gets uploaded as part of UPLOAD_FILES:
http://hg.mozilla.org/mozilla-central/annotate/cbbc380f1e1c/toolkit/mozapps/installer/packager.mk#l738

Comment 13

[Andrew Halberstadt [:ahal]]

If I go the non-buildbot route, then this depends on bug 1054275 (which I've run into a wall with). That's fine by me, but I'll need some help from someone with more build experience than I.

Comment 14

[Andrew Halberstadt [:ahal]]

Attachment: WIP upload_gcno_tree

Here's a WIP that moves the logic into mozbuild/codecoverage. I'm hitting a UnicodeDecodeError in mozjar.py though:

Traceback (most recent call last):
  File "/home/ahal/tmp/test.py", line 2, in <module>
    package_gcno_tree()
  File "/home/ahal/hg/mozilla-central/python/mozbuild/mozbuild/codecoverage/packager.py", line 23, in package_gcno_tree
    jarrer.copy(name)
  File "/home/ahal/hg/mozilla-central/python/mozbuild/mozpack/copier.py", line 484, in copy
    jar.preload(self._preload)
  File "/home/ahal/hg/mozilla-central/python/mozbuild/mozpack/mozjar.py", line 486, in __exit__
    self.finish()
  File "/home/ahal/hg/mozilla-central/python/mozbuild/mozpack/mozjar.py", line 556, in finish
    self._data.write(headers[entry].serialize())
  File "/home/ahal/hg/mozilla-central/python/mozbuild/mozpack/mozjar.py", line 146, in serialize
    serialized += self[name]
UnicodeDecodeError: 'ascii' codec can't decode byte 0x9b in position 14: ordinal not in range(128)

Comment 15

[Mike Hommey [:glandium]]

Why is there a gcno file with a non-ascii name?

Comment 16

[Andrew Halberstadt [:ahal]]

(In reply to Mike Hommey [:glandium] from comment #15)
> Why is there a gcno file with a non-ascii name?

I'm not really sure what is going on. It doesn't seem to be the name that is non-ascii, but the serialized variable here:
http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozpack/mozjar.py#134

"PK!<��I�Ut��"

Comment 17

[Andrew Halberstadt [:ahal]]

I discovered there is a bug in JarWriter when the FileRegistry contains unicode strings. I filed bug 1056859 for it. Casting build_obj.topobjdir to <str> fixed the problem for me.

Comment 18

[Andrew Halberstadt [:ahal]]

Attachment: upload_gcno_tree

This patch seems to work for me locally (though haven't verified that it gets uploaded yet). Pushed it to Cedar to test it out:
https://tbpl.mozilla.org/?tree=Cedar&rev=074f50a4533e

Comment 19

[Andrew Halberstadt [:ahal]]

Attachment: upload_gcno_tree

There was a small typo in the last patch preventing the .zip from being uploaded. This patch fixes it. I also passed in 'PKG_BASENAME' from the Makefile to the python script instead of re-building it in MozbuildObject.

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8478993 [details] [diff] [review]
upload_gcno_tree

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

Almost.

::: python/mozbuild/mozbuild/codecoverage/__init__.py
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +from .packager import package_gcno_tree

I'm not a huge fan of this Python pattern of aliasing everything into the package. Please make this an empty file unless you can point me at best practices that say I'm wrong.

::: python/mozbuild/mozbuild/codecoverage/packager.py
@@ +9,5 @@
> +import sys
> +
> +from mozpack.files import FileFinder
> +from mozpack.copier import Jarrer
> +from buildconfig import topobjdir

buildconfig is only available after we run configure and/or config.status. We therefore try to avoid importing it at module level in modules.

@@ +13,5 @@
> +from buildconfig import topobjdir
> +
> +def package_gcno_tree(output_file):
> +    # XXX JarWriter doesn't support unicode strings, see bug 1056859
> +    finder = FileFinder(topobjdir.encode('utf-8'))

topobjdir should be a function argument. Whether it comes from CLI arguments or you load buildconfig from cli() is your call.

::: toolkit/mozapps/installer/packager.mk
@@ +745,5 @@
> +ifdef MOZ_CODE_COVERAGE
> +	# Package code coverage gcno tree
> +	@echo 'Packaging code coverage data...'
> +	$(RM) $(CODE_COVERAGE_ARCHIVE_BASENAME).zip
> +	$(PYTHON) $(MOZILLA_DIR)/python/mozbuild/mozbuild/codecoverage/packager.py \

$(PYTHON) -mmozbuild.codecoverage.packager ...

Comment 21

[Andrew Halberstadt [:ahal]]

Attachment: upload_gcno_tree

Fixes review comments.

Comment 22

[Gregory Szorc [:gps]]

Comment on attachment 8479991 [details] [diff] [review]
upload_gcno_tree

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

Looks good! Thanks for persevering.

BTW, I'm not sure what you are using to produce patches, but there are no patch headers in this attachment. It would be nice to see commit message, etc.

Comment 23

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/688501a435ac

Sorry, I usually format my patches just before pushing, which is a bad habit because the uploaded patches don't have the message. I'll try to remember to format them pre-upload in the future.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/688501a435ac

Comment 25

[Joshua Cranmer [:jcranmer]]

I don't know where to file a bug on this, so I'll mention it here instead.

I've noticed that the code-coverage builds on Cedar seem to be failing due to the installer name picking up the code-coverage.gcno.zip as the installer instead of the actual installer:
<https://treeherder.mozilla.org/ui/logviewer.html#?job_id=80333&repo=cedar>.

Comment 26

[Andrew Halberstadt [:ahal]]

Hmm, I vaguely remember running into this and fixing it awhile back. For the record, I haven't been working on this due to other priorities popping up, but I plan to pick it back up again soon. Thanks for pointing it out!