1260299 - mach should generate VS2015 project files for build-backend VisualStudio at least for VS2015 build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

I have VS2015 build, but mach build-backend still generates VS2013 project files for me.

Comment 1

[Honza Bambas (:mayhemer)]

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.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

It is a problem if you install both VS2013 and VS2015, and the version selector would automatically use VS2013 to open...

Comment 3

[Honza Bambas (:mayhemer)]

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.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

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.

Comment 5

[Gregory Szorc [:gps]]

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.

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Remove references to unsupported Visual Studio versions; r?chmanchester

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/

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Support generating Visual Studio 2015 project files; r?chmanchester

Pretty straightforward.

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

Comment 8

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Generate Visual Studio project files corresponding to current toolchain; r?chmanchester

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/

Comment 9

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Allow _write_file to pass mode argument to FileAvoidWrite; r?chmanchester

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/

Comment 10

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Use FileAvoidWrite when writing Visual Studio files; r?chmanchester

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/

Comment 11

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1260299 - Remove "experimental" label of Visual Studio files; r?chmanchester

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

[Mike Hommey [:glandium]]

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.

Comment 13

[Gregory Szorc [:gps]]

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.

Comment 14

[Chris Manchester (limited bugmail, email directly)]

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

[Chris Manchester (limited bugmail, email directly)]

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

Comment 16

[Chris Manchester (limited bugmail, email directly)]

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

Comment 17

[Chris Manchester (limited bugmail, email directly)]

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

Comment 18

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 19

[Chris Manchester (limited bugmail, email directly)]

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

[Pulsebot]

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

[Carsten Book [:Tomcat]]

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