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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

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.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1289638 - Don't rename the DIA SDK directory in the MSVC tooltool package.

Review commit: https://reviewboard.mozilla.org/r/67308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67308/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1289638 - Don't remove the SDK version from the SDK paths in the MSVC tooltool package.

Review commit: https://reviewboard.mozilla.org/r/67310/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67310/

Comment 3

[Mike Hommey [:glandium]]

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 4

[Gregory Szorc [:gps]]

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).

Comment 5

[Gregory Szorc [:gps]]

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

Comment 6

[Gregory Szorc [:gps]]

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.

Comment 7

[Mike Hommey [:glandium]]

(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 :(

Comment 8

[Gregory Szorc [:gps]]

Flagging myself to produce a new tooltool archive. Will likely do tomorrow since it is EOD for me.

Comment 9

[Mike Hommey [:glandium]]

Attachment: Bug 1289638 - Update the MSVC tooltool package.

Review commit: https://reviewboard.mozilla.org/r/67652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67652/

Comment 10

[Mike Hommey [:glandium]]

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/

Comment 11

[Mike Hommey [:glandium]]

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/

Comment 12

[Gregory Szorc [:gps]]

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

Comment 13

[Gregory Szorc [:gps]]

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.

Comment 14

[Mike Hommey [:glandium]]

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.

Comment 15

[Mike Hommey [:glandium]]

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/

Comment 16

[Mike Hommey [:glandium]]

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/

Comment 17

[Mike Hommey [:glandium]]

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 18

[Gregory Szorc [:gps]]

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

https://reviewboard.mozilla.org/r/67652/#review64940

Comment 19

[Pulsebot]

Pushed by [e-mail]:
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

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f6396641036
https://hg.mozilla.org/mozilla-central/rev/ada66cbedd75
https://hg.mozilla.org/mozilla-central/rev/07e0af7fc5d9

Comment 21

[Carsten Book [:Tomcat]]

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

Comment 22

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/3d54c9412b98
Backed out changeset 07e0af7fc5d9 
https://hg.mozilla.org/mozilla-central/rev/e0af0c1cf097
Backed out changeset ada66cbedd75

Comment 23

[Gregory Szorc [:gps]]

The failures in SpiderMonkey builds ring a bell. That might be why I eliminated spaces from "DIA SDK" in the archive.

Comment 24

[Mike Hommey [:glandium]]

Attachment: Set WINDOWSSDKDIR for autospider builds

Mozreview won't let me push this, so here it is, old fashion

Comment 25

[Pulsebot]

Pushed by [e-mail]:
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

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fad8e9d59d3b
https://hg.mozilla.org/mozilla-central/rev/d0e304a1d639
https://hg.mozilla.org/mozilla-central/rev/ad744de69904
https://hg.mozilla.org/mozilla-central/rev/f0b5125375e6