[Visual Studio] header files with exclude tag and remove from project if all configurations ignore it

[DJLink]

Hello,

This refers to a issue I opened a couple months ago #33

I made some changes so that Include files (.h etc) that match exclude from build regex also get the Template.Project.ProjectFilesSourceExcludeFromBuild tag, previously it was only .cpp files. I know having the .h be part of the build or not doesn't impact the configuration but I think this makes it easier to see the solution tree, now both .h and .cpp will show as not part of build.

Another change I made is that while looping allFiles and deciding if a source or header, if all requested Project configurations happen to exclude a certain file then it doesn't get added at all. So in case someone builds only a win32 project it would exclude all mac files for example. But if they add both to configuration it adds the files but marks as ignore from build.

Let me know thoughts and suggestions, if this is helpful or not.

[DJLink]

Fixed failure in tests, was missing the updated samples.

[belkiss]

Heya!

I agree with you that generally marking .h files as excluded or not doesn't have any impact, except if for instance a custom build was set for headers! So it's good to have it fixed!

About the other change, I would prefer the files ending up in ResolvedSourceFilesBuildExclude remain in the projects, even though they are not built in any configuration. Instead, we could add a equivalent to the Project property SourceFilesExclude to Project.Configuration. What do you think?

Thanks for sharing!

[DJLink]

Hey,
Thanks for the suggestion.

Let me see if I understood correctly. So adding a new property to Project.Configuration so that all files matching there would not be added in any way to the project, and leaving ResolvedSourceFilesBuildExclude as before.

I think it could work especially because we can configure it on ConfigureAll() step.

I'll try to make the changes later and see where it leads.

Without much thought I'm thinking it would be nice to add a
Strings SourceFilesExclude = new Strings();
Strings SourceFilesIncludeRegex = new Strings();

to Project.Configuration

[belkiss]

If you meant:

Strings SourceFilesExclude = new Strings();
Strings SourceFiles*Exclude*Regex = new Strings();

Then yes, exactly :)
Those properties would only apply if the files are excluded in all configurations that share the same vcxproj as an output.

[DJLink]

Yes made a typo there, but got it right on the code :)

Well I made the changes and my own project is exactly the same with the files I wanted to exclude, I just had to change my configs to SourceFilesExcludeRegex instead of SourceFilesBuildExcludeRegex.

Let me know if this was what you were mentioning and if it's acceptable.

[belkiss]

Yes sounds good! Could you reapply your changes on top of the dev branch? I rebased it on master to cleanup the history, and had to force push it again... apologies!

[DJLink]

Can I just resolve the conflicts and push a new commit or do you want me to reapply from a "clean" dev branch as it is right now by opening a new pull request? Not sure if I can edit the commits already in this pull request, sorry kinda newbie with github

[belkiss]

Yes, you'll be able to edit the commits, as it is the branch as a whole you are requesting to integrate :)
Here are the steps you can follow to update the MR.

From your current workspace in the dev branch:

    > git checkout -b saved_dev  # this will backup your dev branch in its current state
    > git checkout dev # go back to dev
    > git reset --hard upstream/dev # resets the branch to the upstream state, considering upstream is the official sharpmake github repo
    > git cherry-pick SHA1_Commit  # repeat for every commit you want to integrate, taking the SHA1s from the git log in the saved_dev branch
    > git push --force origin dev # rewrite the history of the dev branch in your fork, which will automatically update this merge request :)
    > git branch -D saved_dev # to cleanup

[DJLink]

Great, thank you for the steps, just did and I think it worked :)

[belkiss]

Looks good! but I'm concerned about the cost of reiterating all the configurations for each file and include.
I'll profile with your changes on some of our codebases, and answer back.

[DJLink]

Hello,
Was just curious if you had some update/data on the impact of these changes?