refactor Visual Studio file importing - #8803
Conversation
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
I just quickly glanced at this. sounds promising!
| } | ||
|
|
||
| void ImportProject::fsSetIncludePaths(FileSettings& fs, const std::string &basepath, const std::list<std::string> &in, std::map<std::string, std::string, cppcheck::stricmp> &variables) | ||
| static std::string toAbsolute(const std::string &path) |
There was a problem hiding this comment.
can you check if Path::getAbsoluteFilePath works instead?
There was a problem hiding this comment.
It doesn't. I can figure out later why. I would like to get this basic implementation committed now so I can start working on finding out what still needs to be implemented.
This reverts commit ee53801.
added support for nested folders in .sln files added support for ProjectConfigurations in .props files added support for reading .targets files added more properties with known values added debug logging of unsupported properties and unresolved paths replaced Condition parser to support arbitrary chaining rather than pattern matching refactored similar code for better reuse added more Condition parser tests extended existing test for better coverage of new features
|
This is now a 90% solution. There are still some unimplemented or partially implemented features regarding being able to change the order of .props and .targets evaluation that I don't fully understand yet. With these changes it is now possible to check PowerToys.slnx which has a very complicated build. |
| variables[var] = envValue; | ||
| it = variables.find(var); |
There was a problem hiding this comment.
we can perform the insert+find more efficiently in one statement:
| variables[var] = envValue; | |
| it = variables.find(var); | |
| it = variables.emplace(var, envValue).first; |
There was a problem hiding this comment.
I keep finding new stuff while testing so I don't think it's ready right now. This is a very deep rabbit hole. I am finding problems with Path::stripDirectoryPart not working on windows with unix paths and there is no way to get a files name without the path an extension. Thanks for looking at it.
|
I just hit a big problem. I'm storing properties and metadata in the same map. The really need to be in separate maps to be MSBuild compatable. It almost works the way it is. The original implementation was sort of doing the right thing. |
2 participants
After many aborted attempts to add new features to the Visual Studio importers I came to the realization that the existing code was fundamentally flawed in 4 ways.
This PR collects all properties and metadata and simplifies them.
It simplifies and evaluates conditions when encountered.
It handles file paths properly by making them all absolute internally.
It handles self-referencing properties.
This is all done just like Visual studio and verified using MSBuild log files.
importVcxproj now only supports reading real files. The testVcxprojUnicode test in testimportproject.cpp passed a tinyxml2::XMLDocument to importVcxproj and
that required many hacks to make work. That test was moved to a real file in test/cli.
New tests were added for props files and for ForcedIncludeFiles. Support for many missing XML elements were added. The code is now simpler and just works with a lot more properties.
This refactor is about a 75% solution. Support for more uncommon Visual Studio stuff can be added later. Some things can never be implemented and some things can only be supported with a Visual Studio compiler installed on the machine and adding some way to tell cppcheck where it's installed. That's probably not worth the effort.
We still just ignore stuff that we can't handle. I would like to add support for generating debug message when we ignore things we don't understand so they can be fixed but that can be implemented later.