Closed
Bug 1260299
Opened 8 years ago
Closed 8 years ago
mach should generate VS2015 project files for build-backend VisualStudio at least for VS2015 build
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox49 fixed)
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: xidorn, Assigned: gps)
References
Details
Attachments
(6 files)
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
MozReview Request: Bug 1260299 - Support generating Visual Studio 2015 project files; r?chmanchester
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
MozReview Request: Bug 1260299 - Use FileAvoidWrite when writing Visual Studio files; r?chmanchester
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
chmanchester
:
review+
|
Details |
I have VS2015 build, but mach build-backend still generates VS2013 project files for me.
Comment 1•8 years ago
|
||
Yes, but only after we drop the VS2013 support. Now you can easily load older version solutions in newer version IDEs, there is a conversion wizard. So it's not a problem.
Reporter | ||
Comment 2•8 years ago
|
||
It is a problem if you install both VS2013 and VS2015, and the version selector would automatically use VS2013 to open...
Comment 3•8 years ago
|
||
Then I don't think mach can help you here, it's about file associations. I have the same setup, actually. Having an option on mach which version to generate is worth, tho.
Reporter | ||
Comment 4•8 years ago
|
||
Well... I believe it opens VS2013 if the version of the file is 12, and VS2015 if it's 14, so mach can help here. And I think it makes lots of sense for mach to automatically generate version 12 if the current environment is VS2013, and version 14 if that is VS2015.
Assignee | ||
Comment 5•8 years ago
|
||
I think we should look at which version of the toolchain the build is configured to use and have that determine the Visual Studio solution and project file versions.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•8 years ago
|
||
We only support building with VS2013 and VS2015. Remove references to older versions in the Visual Studio build backend. Review commit: https://reviewboard.mozilla.org/r/54146/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54146/
Attachment #8754662 -
Flags: review?(cmanchester)
Attachment #8754663 -
Flags: review?(cmanchester)
Attachment #8754664 -
Flags: review?(cmanchester)
Attachment #8754665 -
Flags: review?(cmanchester)
Attachment #8754666 -
Flags: review?(cmanchester)
Attachment #8754667 -
Flags: review?(cmanchester)
Assignee | ||
Comment 7•8 years ago
|
||
Pretty straightforward. Review commit: https://reviewboard.mozilla.org/r/54148/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54148/
Assignee | ||
Comment 8•8 years ago
|
||
If we're running VS2013, we generate VS2013 files. If we're running VS2015, we generate VS2015 files. If we don't have a Visual Studio version defined, refuse to generate project files (hopefully this doesn't happen in the real world but I'm sure someone will complain if it does). Review commit: https://reviewboard.mozilla.org/r/54150/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54150/
Assignee | ||
Comment 9•8 years ago
|
||
Currently, self._write_file() instantiates FileAvoidWrite instances with the default mode of 'rU' which uses universal newlines (\n). Visual Studio project files use CRLF newlines. We want to use self._write_file() in the Visual Studio backend (which predates self._write_file). Prepare for this by passing the mode argument through. Review commit: https://reviewboard.mozilla.org/r/54152/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54152/
Assignee | ||
Comment 10•8 years ago
|
||
By using self._write_file() and FileAvoidWrite, we avoid writing files unless they change. This means that Visual Studio won't want you to reload the solution and projects whenever the backend is generated. This means you can regenerate the backend all you want and chances are it won't disrupt your Visual Studio experience. Since self._write_file() creates parent directories for us, we were able to remove this code. If you run `mach build-backend --diff` with this commit, output will change. For reasons I don't understand, we were producing XML with e.g. \r\r\n sequences. This patch appears to restore \r\n. How we got \r\r I don't know because I can't find anywhere in the code where that can occur. But this commit does appear to restore sanity. Also, it appears modern versions of Visual Studio (perhaps only VS2015) doesn't write your project files. When I initially implemented Visual Studio project generation several years ago, as soon as you loaded the solution and hit "Save All" Visual Studio would re-save your files using a slightly different formatting (it did some gnarly things with XML indentation). VS2015 doesn't do this. So your files on disk should be unmodified for longer, making Visual Studio a more viable development environment. Yay. Review commit: https://reviewboard.mozilla.org/r/54154/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54154/
Assignee | ||
Comment 11•8 years ago
|
||
The label has been there for years. It isn't really experimental. The Visual Studio solution still leaves a lot to be desired. But let's not scare people away by calling it experimental. Review commit: https://reviewboard.mozilla.org/r/54156/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54156/
Comment 12•8 years ago
|
||
https://reviewboard.mozilla.org/r/54156/#review50852 ::: python/mozbuild/mozbuild/config_status.py:55 (Diff revision 1) > -You are building Firefox on Windows. Please help us test the experimental > -Visual Studio project files (yes, IntelliSense works) by running the > +You are building Firefox on Windows. You can generate Visual Studio > +files by running: Here's for a followup: if the backend generation time is low overhead enough, we could enable this by default when building with msvc.
Assignee | ||
Comment 13•8 years ago
|
||
https://reviewboard.mozilla.org/r/54156/#review50852 > Here's for a followup: if the backend generation time is low overhead enough, we could enable this by default when building with msvc. I considered that. There's still a possibility for the solution file to get out of sync: VS2015 inserts a few lines with the VS build version. I want to have a crafty solution so we don't rewrite the solution for trivial changes. The backend does take a second or two. I blame XML.
Updated•8 years ago
|
Attachment #8754662 -
Flags: review?(cmanchester) → review+
Comment 14•8 years ago
|
||
Comment on attachment 8754662 [details] MozReview Request: Bug 1260299 - Remove references to unsupported Visual Studio versions; r?chmanchester https://reviewboard.mozilla.org/r/54146/#review51292
Comment 15•8 years ago
|
||
Comment on attachment 8754663 [details] MozReview Request: Bug 1260299 - Support generating Visual Studio 2015 project files; r?chmanchester https://reviewboard.mozilla.org/r/54148/#review51294
Attachment #8754663 -
Flags: review?(cmanchester) → review+
Comment 16•8 years ago
|
||
Comment on attachment 8754664 [details] MozReview Request: Bug 1260299 - Generate Visual Studio project files corresponding to current toolchain; r?chmanchester https://reviewboard.mozilla.org/r/54150/#review51298
Attachment #8754664 -
Flags: review?(cmanchester) → review+
Comment 17•8 years ago
|
||
Comment on attachment 8754665 [details] MozReview Request: Bug 1260299 - Allow _write_file to pass mode argument to FileAvoidWrite; r?chmanchester https://reviewboard.mozilla.org/r/54152/#review51300
Attachment #8754665 -
Flags: review?(cmanchester) → review+
Comment 18•8 years ago
|
||
Comment on attachment 8754666 [details] MozReview Request: Bug 1260299 - Use FileAvoidWrite when writing Visual Studio files; r?chmanchester https://reviewboard.mozilla.org/r/54154/#review51302 ::: python/mozbuild/mozbuild/backend/visualstudio.py:167 (Diff revision 1) > build_command='$(SolutionDir)\\mach.bat build-backend -b VisualStudio') > projects[basename] = (project_id, basename, 'visual-studio') > > # Write out a shared property file with common variables. > props_path = os.path.join(out_proj_dir, 'mozilla.props') > - with open(props_path, 'wb') as fh: > + with self._write_file(props_path, mode='rb') as fh: It's odd to see all these go from 'wb' to 'rb', but I guess it's correct based on reading FileAvoidWrite.
Attachment #8754666 -
Flags: review?(cmanchester) → review+
Updated•8 years ago
|
Attachment #8754667 -
Flags: review?(cmanchester) → review+
Comment 19•8 years ago
|
||
Comment on attachment 8754667 [details] MozReview Request: Bug 1260299 - Remove "experimental" label of Visual Studio files; r?chmanchester https://reviewboard.mozilla.org/r/54156/#review51304
Comment 20•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b64d6172f34e https://hg.mozilla.org/integration/mozilla-inbound/rev/a50ca75ea08f https://hg.mozilla.org/integration/mozilla-inbound/rev/35199c26010a https://hg.mozilla.org/integration/mozilla-inbound/rev/817ccd8233bc https://hg.mozilla.org/integration/mozilla-inbound/rev/647d6ebe863d https://hg.mozilla.org/integration/mozilla-inbound/rev/46cb2900f4ec
Comment 21•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b64d6172f34e https://hg.mozilla.org/mozilla-central/rev/a50ca75ea08f https://hg.mozilla.org/mozilla-central/rev/35199c26010a https://hg.mozilla.org/mozilla-central/rev/817ccd8233bc https://hg.mozilla.org/mozilla-central/rev/647d6ebe863d https://hg.mozilla.org/mozilla-central/rev/46cb2900f4ec
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•