Closed Bug 1275522 Opened 4 years ago Closed 4 years ago

Make a Python YAML library available in-tree for build scripts like histogram_tools.py

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: Dexter, Assigned: Dexter)

References

(Blocks 1 open bug)

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

Attached patch bug1275522.patchSplinter Review
With this patch I can successfully |import yaml| from histogram_tools.py, as we're referencing that script from |GENERATED_FILES| in moz.build, thus running it in virtualenv.
Comment on attachment 8756385 [details] [diff] [review]
bug1275522.patch

This patch adds pyYAML support in virtualenv. We need this to support the parsing of the scalar types definition file (see bug 1275517 for context).
Attachment #8756385 - Flags: review?(gps)
This bug is about verifying that we can get pyyaml into tree so build python scripts like histogram_tools.py can use it (and eventually implementing the required changes).
Blocks: 1275517
Priority: -- → P1
Whiteboard: [measurement:client]
Comment on attachment 8756385 [details] [diff] [review]
bug1275522.patch

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

There is a C extension in pyyaml. Please note that this simple solution won't compile that extension and therefore YAML performance may not be great. So hopefully you aren't using this for performance critical code.
Attachment #8756385 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #3)
> Comment on attachment 8756385 [details] [diff] [review]
> bug1275522.patch
> 
> Review of attachment 8756385 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> There is a C extension in pyyaml. Please note that this simple solution
> won't compile that extension and therefore YAML performance may not be
> great. So hopefully you aren't using this for performance critical code.

We will be using pyyaml to parse a definition file and produce cpp/h code that will be compiled by the build system (basically, the same thing that's done by histogram_tools.py et al).
Summary: Verify that we can get pyyaml into tree for using it in scripts like histogram_tools.py → Make a Python YAML library available in-tree for build scripts like histogram_tools.py
Points: --- → 2
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
The file is also not intended to be complex in structure, see the bottom here for an example:
https://docs.google.com/document/d/13mcY17RzpE3BsB4CgR6DidytWzl4tMpAEmrvf8fOSJ8/edit#heading=h.k3sce0uvqbmy
Keywords: checkin-needed
The bugs from https://hg.mozilla.org/mozilla-central/rev/ea15028498ed didn't get marked as fixed.
Flags: needinfo?(wkocher)
https://hg.mozilla.org/mozilla-central/rev/d64d3cadae8f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(In reply to Guilherme Lima from comment #8)
> The bugs from https://hg.mozilla.org/mozilla-central/rev/ea15028498ed didn't
> get marked as fixed.

yes you are right, fixed this now, sorry for this
Flags: needinfo?(wkocher)
You need to log in before you can comment on or make changes to this bug.