Added Support for Shared Items Projects #6286

DevFelixFaber · 2024-04-12T08:00:50Z

At work (Areus Gmbh), we use Shared Items Projects in order to deduplicate code. In contrast to conventional project files (.vcxproj), the compilation configuration is set by the "includee", which makes the code-reuse of the project more flexible.

This pull-request adds support for Shared Items Projects (.vcxitems) in Visual studio projects / solutions.

The idea is that for each .vcxproj, we check for referenced shared items projects. When all the source files and include paths are build, we add the ones defined in the referenced .vcxitems.
The implementation probably isn't 100% robust, but this works for our projects and could serve as a starting point :)

I apologize for the commit history - it's a bit wonky as we currently still use 2.11. I tried to merge the changes to the main branch so other could benefit from these changes as well, but seems like the git history looks a bit weird as a result.

lib/importproject.cpp

firewave · 2024-04-14T20:17:43Z

Thanks for a contribution. I will take a closer look in the next few days.

Please add a test with an example project to test/cli/more-projects.py.

Also some initial comments inline.

firewave · 2024-04-14T20:18:55Z

I apologize for the commit history - it's a bit wonky as we currently still use 2.11. I tried to merge the changes to the main branch so other could benefit from these changes as well, but seems like the git history looks a bit weird as a result.

That can be resolved by re-basing the branch on the latest head.

DevFelixFaber · 2024-04-15T05:45:37Z

Thank you for the feedback, I'll try to implement it this week.

DevFelixFaber · 2024-04-19T06:03:56Z

I apologize for the commit history - it's a bit wonky as we currently still use 2.11. I tried to merge the changes to the main branch so other could benefit from these changes as well, but seems like the git history looks a bit weird as a result.

That can be resolved by re-basing the branch on the latest head.

I had the same issue as my merge, but my oversight was that my initial changes came from the 2.11 release branch, meaning it includes the few commits made for the official release.
I re-based this once more, removed the release commits and force-pushed this to my branch. Seems more correct now.

Still working on adding tests.

…SolutionDir)/libs. Without this change, the include path will be "/libs", which is not the same as "./libs" or "libs".

…items project file and making the project redirect paths accordingly

firewave · 2024-04-30T12:56:29Z

Thanks for adding the project. This will help understanding things. It is probably sufficient to have only a single configuration in it. That should make things easier to read as well.

DevFelixFaber · 2024-04-30T13:22:19Z

Thanks for adding the project. This will help understanding things. It is probably sufficient to have only a single configuration in it. That should make things easier to read as well.

Fixed in 10fb4e5

I also added a (crude) test in d34d14c. Seems to run fine on my machine, but I'm not sure I set it up correctly.

I seem to have broken the "changes requested" with my rebase + force push. Sorry!

firewave · 2024-05-27T11:55:04Z

Sorry for the late reply.

Please fix the CI and I will finally have a proper look.

…hared items project implementation)

…ed items project files are checked

DevFelixFaber · 2024-06-05T12:21:39Z

My apologies, I borked the pipeline with the code removed in a00dd2a.
That was some special code which (should be) unrelated to the shared items support.
I fixed some more formatting issues (thanks Visual Studio) and adjusted the assertion on the new shared items project test.

firewave · 2024-06-05T12:23:54Z

My apologies, I borked the pipeline with the code removed in [a00dd2a]

It happens.

As the CI seems to be happy now, I hope to finish up the review this week.

danmar · 2024-06-09T17:28:44Z

At a glance this looks very interesting. Please make the CI happy. 👍

… locally???

danmar

nits

danmar · 2024-06-11T12:37:01Z

lib/importproject.cpp

@@ -203,7 +203,8 @@ ImportProject::Type ImportProject::import(const std::string &filename, Settings
        }
    } else if (endsWith(filename, ".vcxproj")) {
        std::map<std::string, std::string, cppcheck::stricmp> variables;
-        if (importVcxproj(filename, variables, emptyString, fileFilters)) {
+        std::vector<SharedItemsProject> sharedItemsProjects{};


I suggest we skip the redundant {}

danmar · 2024-06-11T12:43:27Z

lib/importproject.cpp

@@ -698,14 +699,15 @@ static void loadVisualStudioProperties(const std::string &props, std::map<std::s
    }
 }

-bool ImportProject::importVcxproj(const std::string &filename, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters)
+bool ImportProject::importVcxproj(const std::string &filename, std::map<std::string, std::string, cppcheck::stricmp> &variables, const std::string &additionalIncludeDirectories, const std::vector<std::string> &fileFilters, std::vector<SharedItemsProject> &cache)


hmm the name cache is short. I feel like 50/50 if I think the name is fine or not.

danmar · 2024-06-11T12:48:46Z

lib/importproject.cpp

+                            }
+                            if (!simplifyPathWithVariables(pathToSharedItemsFile, variables)) {
+                                printError("Could not simplify path to referenced shared items project");
+                                exit(-1);


why don't you return false here?

For information I fear that in the GUI this would mean that the whole program would just terminate without any indication to the user.

firewave requested changes Apr 14, 2024

View reviewed changes

lib/importproject.cpp Outdated Show resolved Hide resolved

lib/importproject.cpp Outdated Show resolved Hide resolved

lib/importproject.cpp Outdated Show resolved Hide resolved

DevFelixFaber force-pushed the ff/pullRequest branch from fb382dd to d1d4697 Compare April 19, 2024 05:38

firewave mentioned this pull request Apr 22, 2024

extracted trim() and findAndReplace() to utils.{cpp|h} #6325

Merged

Felix Faber added 13 commits April 30, 2024 13:12

Rebasing

d2766d6

This change is crucial when $(SolutionDir) = "" and includePath is $(…

ea13186

…SolutionDir)/libs. Without this change, the include path will be "/libs", which is not the same as "./libs" or "libs".

Rebasing...

372ab8d

Removing duplicate code, storing source files relative to the shared …

9fa0c94

…items project file and making the project redirect paths accordingly

Proper error reporting instead of "exit(-1)" call

ebcb64a

Rebasing...

eb95799

skipping string constructor

641e22c

Trying to fix up whitespace and formatting

ab5d5d5

Even more formatting fixes

5cb8b81

Restoring TODO

7933939

Fixing up errors I introduced during rebasing, sorry!

c115c52

Restoring TODO and fixing whitespace

1907ed7

More whitespace fixes...

49e9dba

DevFelixFaber force-pushed the ff/pullRequest branch from 006bff3 to 49e9dba Compare April 30, 2024 11:15

Felix Faber added 3 commits April 30, 2024 13:27

Remove local stringReplace, adjust FileSettings usage

fbc066b

Adding Solution containing Shared items project for test

6c36aea

Placeholder for tests - cannot check windows project at the moment

7effc12

Felix Faber added 2 commits April 30, 2024 15:12

Test should be runnable

d34d14c

Removing all but one configuration

10fb4e5

DevFelixFaber requested a review from firewave May 9, 2024 07:33

Felix Faber added 2 commits June 5, 2024 12:47

Merge branch 'main' into ff/pullRequest

d94c062

Undoing change (which fixed an issue locally, but is not related to s…

a00dd2a

…hared items project implementation)

DevFelixFaber marked this pull request as draft June 5, 2024 10:57

DevFelixFaber marked this pull request as ready for review June 5, 2024 10:57

Felix Faber added 2 commits June 5, 2024 13:05

Fix whitespace formatting for formatter pipeline

b704b84

Fix test: dont care about specific flags, we want to ensure that shar…

c58a467

…ed items project files are checked

Felix Faber added 3 commits June 11, 2024 11:08

Fix issue: Reducing scope of variable

76f6cfe

Trying to appease the Pipeline

61434c5

Fix whitespace, hopefull fixing CI compilation error which works fine…

d0f9894

… locally???

danmar reviewed Jun 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Support for Shared Items Projects #6286

Added Support for Shared Items Projects #6286

DevFelixFaber commented Apr 12, 2024 •

edited

firewave commented Apr 14, 2024

firewave commented Apr 14, 2024

DevFelixFaber commented Apr 15, 2024

DevFelixFaber commented Apr 19, 2024 •

edited

firewave commented Apr 30, 2024

DevFelixFaber commented Apr 30, 2024 •

edited

firewave commented May 27, 2024

DevFelixFaber commented Jun 5, 2024

firewave commented Jun 5, 2024

danmar commented Jun 9, 2024

danmar left a comment

danmar Jun 11, 2024

danmar Jun 11, 2024

danmar Jun 11, 2024

danmar Jun 11, 2024

Added Support for Shared Items Projects #6286

Are you sure you want to change the base?

Added Support for Shared Items Projects #6286

Conversation

DevFelixFaber commented Apr 12, 2024 • edited

firewave commented Apr 14, 2024

firewave commented Apr 14, 2024

DevFelixFaber commented Apr 15, 2024

DevFelixFaber commented Apr 19, 2024 • edited

firewave commented Apr 30, 2024

DevFelixFaber commented Apr 30, 2024 • edited

firewave commented May 27, 2024

DevFelixFaber commented Jun 5, 2024

firewave commented Jun 5, 2024

danmar commented Jun 9, 2024

danmar left a comment

Choose a reason for hiding this comment

danmar Jun 11, 2024

Choose a reason for hiding this comment

danmar Jun 11, 2024

Choose a reason for hiding this comment

danmar Jun 11, 2024

Choose a reason for hiding this comment

danmar Jun 11, 2024

Choose a reason for hiding this comment

DevFelixFaber commented Apr 12, 2024 •

edited

DevFelixFaber commented Apr 19, 2024 •

edited

DevFelixFaber commented Apr 30, 2024 •

edited