1252713 - Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Gah mid-air :(

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c89eae0e5686

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8725497 [details] [diff] [review]
Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

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

Thanks! Sorry you got stuck with the short straw this time.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

Fix the bad header include order in the last patch.

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 8725741 [details] [diff] [review]
Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

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

Yup, still good to go.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Woops sorry for the duplicate review!

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3c3ecbd56f1

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b2af03fba36

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b2af03fba36

Comment 10

[Mike Hommey [:glandium]]

Somehow this also affects building on ppc with unified sources in 46. Can this be uplifted?

Comment 11

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #10)
> Somehow this also affects building on ppc with unified sources in 46. Can
> this be uplifted?

In file included from /«PKGBUILDDIR»/build-browser/js/src/jsapi-tests/Unified_cpp_js_src_jsapi-tests5.cpp:11:0:
/«PKGBUILDDIR»/js/src/jsapi-tests/testXDR.cpp: In member function 'virtual bool cls_testXDR_sourceMap::run(JS::HandleObject)':
/«PKGBUILDDIR»/js/src/jsapi-tests/testXDR.cpp:133:9: error: 'UniqueTwoByteChars' was not declared in this scope
         UniqueTwoByteChars expected_wrapper(js::InflateString(cx, *sm, &len));
         ^
/«PKGBUILDDIR»/js/src/jsapi-tests/testXDR.cpp:133:9: note: suggested alternatives:
In file included from /«PKGBUILDDIR»/build-browser/dist/include/js/HeapAPI.h:15:0,
                 from /«PKGBUILDDIR»/build-browser/dist/include/js/GCAPI.h:12,
                 from /«PKGBUILDDIR»/build-browser/dist/include/js/RootingAPI.h:19,
                 from /«PKGBUILDDIR»/build-browser/dist/include/js/GCVector.h:12,
                 from /«PKGBUILDDIR»/js/src/jscntxt.h:14,
                 from /«PKGBUILDDIR»/js/src/vm/RegExpObject.h:13,
                 from /«PKGBUILDDIR»/js/src/builtin/RegExp.h:10,
                 from /«PKGBUILDDIR»/js/src/jscompartment.h:15,
                 from /«PKGBUILDDIR»/js/src/jsapi-tests/testWeakMap.cpp:8,
                 from /«PKGBUILDDIR»/build-browser/js/src/jsapi-tests/Unified_cpp_js_src_jsapi-tests5.cpp:2:
/«PKGBUILDDIR»/build-browser/dist/include/js/Utility.h:489:56: note:   'JS::UniqueTwoByteChars'
 typedef mozilla::UniquePtr<char16_t[], JS::FreePolicy> UniqueTwoByteChars;
                                                        ^
/«PKGBUILDDIR»/build-browser/dist/include/js/Utility.h:489:56: note:   'JS::UniqueTwoByteChars'

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8725741 [details] [diff] [review]
Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

Approval Request Comment
[Feature/regressing bug #]:
Unclear; probably many.

[User impact if declined]:
PPC builds will apparently break?

[Describe test coverage new/current, TreeHerder]:
Didn't introduce any changes, just added missing forward declarations and #includes. So all the existing tests which have been passing since this was landed.

[Risks and why]:
Very little, this is all trivial changes and assessing potential breakage is as simple as checking that the build completes successfully.

[String/UUID change made/needed]:
None.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8725741 [details] [diff] [review]
Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

I was told that this is needed for Linux Distro builds. Aurora47+

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Mike Hommey [:glandium] from comment #10)
> Somehow this also affects building on ppc with unified sources in 46. Can
> this be uplifted?

Nick, if we need this on 46, you need to request an uplift to Beta (46).

Comment 15

[Mike Hommey [:glandium]]

It's already on aurora, by virtue of having landed during on m-c during the 47 cycle.

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8725741 [details] [diff] [review]
Fix FILES_PER_UNIFIED_FILE=1 bustage in js/

Ok since this is already in 47, it needs uplifting to Beta46. Taking it.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6d3033d429e5

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6d3033d429e5

Comment 19

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=902094&repo=mozilla-beta

Comment 20

[Mike Hommey [:glandium]]

No wonder it failed, Policy.h didn't exist in beta so the patch actually didn't apply.

Comment 21

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #20)
> No wonder it failed, Policy.h didn't exist in beta so the patch actually
> didn't apply.

In fact, the build went through, so I presume the only thing that was a problem is Policy.h existing. Carsten, can you reland without Policy.h (how did you end up adding the file, instead of bailing, btw?)

Comment 22

[Mike Hommey [:glandium]]

https://hg.mozilla.org/releases/mozilla-beta/rev/053873b9c759