Change the path to the DIA SDK in the MSVC tooltool archive

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla50
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed, firefox51 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

3 years ago
The MSVC tooltool archive has the DIA SDK in a DIASDK subdirectory (without space), which differs from a typical MSVC install, which has it in a DIA SDK directory (with a space).

Since there is no technical reason for the removal of that space (the build works with one there), let's make things look more a normal MSVC install.
Assignee

Comment 3

3 years ago
I'd update the package if the script didn't require to have the exact right versions of MSVC and the SDK to be installed locally, or if I could download the existing archive to just edit it in place.
Comment on attachment 8774963 [details]
Bug 1289638 - Don't rename the DIA SDK directory in the MSVC tooltool package.

https://reviewboard.mozilla.org/r/67308/#review64344

I swear I ran into issues with the space: there is a reason I put it there. But if your Try push is green, then whatever the underlying problem was, it appears fixed (it could have been configure issues which got resolved by converting things from shell to Python).
Attachment #8774963 - Flags: review?(gps) → review+
Comment on attachment 8774964 [details]
Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

https://reviewboard.mozilla.org/r/67310/#review64348
Attachment #8774964 - Flags: review?(gps) → review+
Comment on attachment 8774964 [details]
Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

https://reviewboard.mozilla.org/r/67310/#review64350

Actually, I can't r+ this without the corresponding tooltool manifest changes, since this will break the build.
Attachment #8774964 - Flags: review+ → review-
Assignee

Comment 7

3 years ago
(In reply to Gregory Szorc [:gps] from comment #6)
> Comment on attachment 8774964 [details]
> Bug 1289638 - Update mozconfig paths to reflect MSVC tooltool package
> changes.
> 
> https://reviewboard.mozilla.org/r/67310/#review64350
> 
> Actually, I can't r+ this without the corresponding tooltool manifest
> changes, since this will break the build.

And per comment 3, I can't do the corresponding tooltool manifest changes :(
Flagging myself to produce a new tooltool archive. Will likely do tomorrow since it is EOD for me.
Flags: needinfo?(gps)
Assignee

Comment 9

3 years ago
Review commit: https://reviewboard.mozilla.org/r/67652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67652/
Attachment #8774964 - Attachment description: Bug 1289638 - Update mozconfig paths to reflect MSVC tooltool package changes. → Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.
Attachment #8775474 - Flags: review?(gps)
Attachment #8774964 - Flags: review- → review?(gps)
Assignee

Comment 10

3 years ago
Comment on attachment 8774963 [details]
Bug 1289638 - Don't rename the DIA SDK directory in the MSVC tooltool package.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67308/diff/1-2/
Assignee

Comment 11

3 years ago
Comment on attachment 8774964 [details]
Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67310/diff/1-2/
Assignee

Updated

3 years ago
Blocks: 1290026
Comment on attachment 8774964 [details]
Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

https://reviewboard.mozilla.org/r/67310/#review64848
Attachment #8774964 - Flags: review?(gps) → review+
I provided glandium with a copy of the vs2015u2.zip so he can upload a new version to tooltool and update the patches with the new archive.
Flags: needinfo?(gps)
Assignee

Comment 14

3 years ago
Uploaded to tooltool:
{
"size": 332442800,
"digest": "995394a4a515c7cb0f8595f26f5395361a638870dd0bbfcc22193fe1d98a0c47126057d5999cc494f3f3eac5cb49160e79757c468f83ee5797298e286ef6252c",
"algorithm": "sha512",
"filename": "vs2015u2.zip"
}

Here's how I created the package:
- Took vs2015u2.zip (validated it has the same sha512sum as the one referenced in-tree)
- unzipped it.
- went into the vs2015u2 directory and:
  - renamed "DIASDK" to "DIA SDK"
  - moved "SDK/Include/*" into "SDK/Include/10.0.586.0/*"
  - moved "SDK/Lib/*" into "SDK/Lib/10.0.586.0/*"
- Modified windows_toolchain.py's find_vs_paths to return the path to vs2015u2 and vs2015u2/SDK.
- Ran windows_toolchain.py without the patches from this bug, and validated I ended up with a zip with the same sha512 as the original one
- Applied the patches from this bug
- Re-ran windows_toolchain.py.
Assignee

Comment 15

3 years ago
Comment on attachment 8774964 [details]
Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67310/diff/2-3/
Attachment #8775474 - Attachment description: Bug 1289638 - Update mozconfig paths to reflect MSVC tooltool package changes. → Bug 1289638 - Update the MSVC tooltool package.
Assignee

Comment 16

3 years ago
Comment on attachment 8775474 [details]
Bug 1289638 - Update the MSVC tooltool package.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67652/diff/1-2/
Assignee

Comment 17

3 years ago
Note that I'm not touching the nss automation files that still use the previous msvc archive. I will file a separate bug for them to pick up the new one.
Comment on attachment 8775474 [details]
Bug 1289638 - Update the MSVC tooltool package.

https://reviewboard.mozilla.org/r/67652/#review64940
Attachment #8775474 - Flags: review?(gps) → review+
Assignee

Updated

3 years ago
Blocks: 1290332

Comment 19

3 years ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/0f6396641036
Don't rename the DIA SDK directory in the MSVC tooltool package. r=gps
https://hg.mozilla.org/integration/autoland/rev/ada66cbedd75
Don't remove the SDK version from the SDK paths in the MSVC tooltool package. r=gps
https://hg.mozilla.org/integration/autoland/rev/07e0af7fc5d9
Update the MSVC tooltool package. r=gps
backed this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=465d150bc8be5bbf9f02a8607d4552b6a5e1697c since one this 2 bugs caused issues on windows xp spidermonkey builds like https://treeherder.mozilla.org/logviewer.html#?job_id=32982163&repo=mozilla-inbound#L879
Status: RESOLVED → REOPENED
Flags: needinfo?(mh+mozilla)
Resolution: FIXED → ---

Comment 22

3 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/3d54c9412b98
Backed out changeset 07e0af7fc5d9 
https://hg.mozilla.org/mozilla-central/rev/e0af0c1cf097
Backed out changeset ada66cbedd75
The failures in SpiderMonkey builds ring a bell. That might be why I eliminated spaces from "DIA SDK" in the archive.
Assignee

Comment 24

3 years ago
Mozreview won't let me push this, so here it is, old fashion
Assignee: nobody → mh+mozilla
Flags: needinfo?(mh+mozilla)
Attachment #8776766 - Flags: review?(gps)
Attachment #8776766 - Flags: review?(gps) → review+

Comment 25

3 years ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fad8e9d59d3b
Set WINDOWSSDKDIR for "autospider" builds. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0e304a1d639
Don't rename the DIA SDK directory in the MSVC tooltool package. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad744de69904
Don't remove the SDK version from the SDK paths in the MSVC tooltool package. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0b5125375e6
Update the MSVC tooltool package. r=gps

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.