Closed Bug 1289638 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox50 fixed, firefox51 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(4 files)

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.
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-
(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)
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)
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 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/
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)
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 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.
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/
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+
Blocks: 1290332
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 → ---
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.
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+
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
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.