1138250 - Generated Visual Studio solution do not contain cpp files

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[andershol]

When running "mach build-backend -b VisualStudio" in a source dir, a Visual Studio solution will be generated, but it does not contain the source cpp-files (it has some "Unified_cpp_layout_tables0.cpp"). It seems the solution is generated based on the files in the obj-dir not the src-dir, but the path to files do point to the src-dir (this means that the "Unified_cpp_layout_tables0.cpp" files can not be opened since they do not exist in the src-dir).

Comment 1

[Mike Hommey [:glandium]]

Relevant: https://groups.google.com/d/msg/mozilla.dev.platform/10QULbAgQZM/6XHCWeAKmm8J

Comment 2

[andershol]

Thanks for the pointer, that do seem to be relevant.

But I completely fail to see the reason for the change. That is, why you I want to manually edit the "Unified_cpp_layout_tables0.cpp"? The VS solution/project files are not being used during a build, but for making the source files visible to the VS editor (that can shell out to mach to build), AFAIK.

Comment 3

[andershol]

Attachment: Wordaround

A small workaround that seem to allow you to create usable project files in case others are wasting time on this.

Comment 4

[Nathan Froyd [:froydnj]]

So, just to be clear:

- The visual studio project doesn't get used to actually build things; but
- The visual studio editor will actually parse and generate intellisense data from the files listed in the project?
- The visual studio editor won't generate intellisense data for files that are #include'd into other files?

Comment 5

[andershol]

Attachment: Comparison of editor behaviour without and with the workaround

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4)
> - The visual studio project doesn't get used to actually build things; but
That is my understanding (and the project files have ConfigurationType set to Makefile). But :gps might be one to ask about how things are supposed to work.

> - The visual studio editor will actually parse and generate intellisense
> data from the files listed in the project?
> - The visual studio editor won't generate intellisense data for files that
> are #include'd into other files?

I think it will use both files listed and files included (I believe it helps with code from e.g. standard libraries as well). But it is not primarily the intellisense that is the problem here. It is what files are made available in the editor as the attached image tries to show. Without the workaround the cpp-files (that I care about) are left out, while the "Unified_cpp_layout_tables0.cpp", that seems to be an auto-generated intermediate file (a bit like an .obj file is), is included (but as mentioned in comment 0, it is included with a wrong path, so it is not found or used).

(Also note the workaround patch is just a hack that solved the problem for me, while looking at something else. I don't know if it includes to many files, if it does it the right way, or what was attempted to be accomplished)

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: file unified files under the correct directory

Ah, excellent.  Thanks for the explanation and the screenshot.  Your changes that you made are the exact same ones we would make.

But I'd rather try something else first.  Would you please try regenerating the visual studio backend with this patch applied?

Comment 7

[andershol]

Attachment: Diff of msvc folder in obj dir with and without msvc.patch

As I sure you saw for yourself when you tested it, it removed the Unified_cpp-files from the project files (see attached). Not sure what you needed me to test.

Looking at the description of your patch, it seems you wanted to fix the path to the Unified_cpp-files. But if so, please explain why you would want to put temp files in the project-files.

Note that both Windows (enterprise edition - 90 days eval) and Visual Studio (Community aka. Standard edition) are free downloads from Microsoft if you want to play around with it.

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to andershol from comment #7)
> As I sure you saw for yourself when you tested it, it removed the
> Unified_cpp-files from the project files (see attached). Not sure what you
> needed me to test.

It removed the unified files?  That's odd...

(BTW, please use the -u option to diff; it makes the diffs signficantly more readable.)

> Looking at the description of your patch, it seems you wanted to fix the
> path to the Unified_cpp-files. But if so, please explain why you would want
> to put temp files in the project-files.

Because, ideally, giving Visual Studio the correct path to the unified files will enable it to discover the actual source files as well (since it will see those files as being included from the unified file), and enable those files to show up in the editor.

Comment 9

[andershol]

Thanks for looking into the problem. The VS-backend is very useful.

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #8)
> (BTW, please use the -u option to diff; it makes the diffs signficantly more
> readable.)
Sure thanks, but this diff was mostly just a quick illustration.

> > Looking at the description of your patch, it seems you wanted to fix the
> > path to the Unified_cpp-files. But if so, please explain why you would want
> > to put temp files in the project-files.
> 
> Because, ideally, giving Visual Studio the correct path to the unified files
> will enable it to discover the actual source files as well (since it will
> see those files as being included from the unified file), and enable those
> files to show up in the editor.

Why would that be ideal? It seems to me that the unified files are implementation details of build system (trying to workaround a supposedly deficient compiler/linker) and having the build-systems intermediate files show up in my editor do not seem ideal. Nor would showing obj-files, also intermediate build-system files, as alluded to above.

Also I doubt that it would make the files show up in the "Solution Explorer" in VS. While I might be able to navigate to the files by choosing "Open Document" while having the include statement in focus, VS do not put every included file into the "Solution Explorer" (listing e.g. the whole standard library would not be helpful).

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to andershol from comment #9)
> > > Looking at the description of your patch, it seems you wanted to fix the
> > > path to the Unified_cpp-files. But if so, please explain why you would want
> > > to put temp files in the project-files.
> > 
> > Because, ideally, giving Visual Studio the correct path to the unified files
> > will enable it to discover the actual source files as well (since it will
> > see those files as being included from the unified file), and enable those
> > files to show up in the editor.
> 
> Why would that be ideal? It seems to me that the unified files are
> implementation details of build system (trying to workaround a supposedly
> deficient compiler/linker) and having the build-systems intermediate files
> show up in my editor do not seem ideal. Nor would showing obj-files, also
> intermediate build-system files, as alluded to above.

Unified files are how we compile things, for better or for worse.  So I believe it makes sense to tell Visual Studio about exactly how we are trying to build things and go from there.  (Individual .cpp files might have compile errors that don't show up in unified mode due to header leakage from other files, etc.)  If it turns out that Visual Studio can't be made to understand unified files, then we can go back to putting the individual .cpp files in the project.

> Also I doubt that it would make the files show up in the "Solution Explorer"
> in VS. While I might be able to navigate to the files by choosing "Open
> Document" while having the include statement in focus, VS do not put every
> included file into the "Solution Explorer" (listing e.g. the whole standard
> library would not be helpful).

Sure, Visual Studio probably has some knowledge about what are "system" headers and what are "user" headers.  And I'm betting that even if you #include .cpp files, it will correctly see those as "user headers" and make them available for editing.  (It's possible, of course, that there are heuristics that limit the number of headers shown, etc.  So if the heuristics aren't doing the right things, we can go back to the way things were before.)

Comment 11

[andershol]

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #10)
> Unified files are how we compile things, for better or for worse.  So I
> believe it makes sense to tell Visual Studio about exactly how we are trying
> to build things and go from there.  (Individual .cpp files might have
> compile errors that don't show up in unified mode due to header leakage from
> other files, etc.)  If it turns out that Visual Studio can't be made to
> understand unified files, then we can go back to putting the individual .cpp
> files in the project.

That the use of unified-files might introduce bugs, seems like a stawman since it will hopefully be the least of our problems when it comes to compile failures. The moz.build files, with far wider risks of  influencing the build, aren't even included in the project (even though that would actually make sense, as they are meant to be edited and e.g. included in the source control). VS is currently being used for editing files only, so I don't know why you would teach it about a intermediately artifact of the build system (it doesn't even know if gcc/cl/clang is being used). If VS is ever to used to build the product and unified-files where still to be used, it would seem that the unified-files would be something that msbuild generated.

It seems you are not yourself a VS-user. Have you had any requests for including the unified-files in the project? or where does this idea come from? are you trying to create a better editing experience?

> Sure, Visual Studio probably has some knowledge about what are "system"
> headers and what are "user" headers.  And I'm betting that even if you
> #include .cpp files, it will correctly see those as "user headers" and make
> them available for editing.  (It's possible, of course, that there are
> heuristics that limit the number of headers shown, etc.  So if the
> heuristics aren't doing the right things, we can go back to the way things
> were before.)

I have never seen files appear in in the solution explorer without being added to the project. On what are you basing your belief? Have you ever used VS?

Comment 12

[Nathan Froyd [:froydnj]]

Comment on attachment 8571122 [details] [diff] [review]
Wordaround

I was wrong, VS is only going to display what we tell it about, so we should be telling it about the actual source files here.  Someday we might be able to build entirely from within VS projects, so we'll have to revisit this decision.

Comment 13

[Nathan Froyd [:froydnj]]

Landed directly on central with RyanVM's approval:

remote:   https://hg.mozilla.org/mozilla-central/rev/1acb666c4613

I tweaked the commit message and added bug #, r=/a=, but the author was kept as Anders.  Thanks for the fix and explanations here, Anders!

Noticed that the visualstudio tests don't test for anything to do with *SOURCES; going to file that as a followup bug.

Comment 14

[andershol]

Thanks. It seems to be working for me.
Could you mark the two duplicate bugs, that  I marked as depended on this, as resolved as well?

Comment 15

[Nathan Froyd [:froydnj]]

(In reply to andershol from comment #14)
> Thanks. It seems to be working for me.
> Could you mark the two duplicate bugs, that  I marked as depended on this,
> as resolved as well?

Done, thanks.