Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Best way to exclude *.sln files? #1525

Closed
bricelam opened this issue Dec 10, 2018 · 22 comments
Closed

Best way to exclude *.sln files? #1525

bricelam opened this issue Dec 10, 2018 · 22 comments

Comments

@bricelam
Copy link
Contributor

bricelam commented Dec 10, 2018

We have different *.sln files in the root of our repo. We have one solution that includes every project, and we use additional solution files to exclude projects that are irrelevant to certain development activities. Excluding these projects improves dev-time perf in VS.

Since the shell scripts include all files matching *.sln in the root, the best way I found to exclude these dev-only solution is via this eng\Build.props file:

<Project>
  <ItemGroup>
    <ProjectToBuild Remove="$(RepoRoot)EFCore.Runtime.sln" />
    <ProjectToBuild Remove="$(RepoRoot)EFCore.Cosmos.sln" />
    <ProjectToBuild Remove="$(RepoRoot)Microsoft.Data.Sqlite.sln" />
  </ItemGroup>
</Project>

Is there a better way? Should it be documented? Do most repos really want to include all solution files in the root by default?

@tmat
Copy link
Member

tmat commented Dec 12, 2018

This is the way. What better way would you propose? Many repos have a single solution.
Have you tried using solution filters (new feature in VS) instead of creating multiple solutions?
What perf issues are you seeing in the solution that contains all projects? Have you reported them?

@markwilkie
Copy link
Member

@bricelam - yes, we should document this.

Regarding all the sln files in the root by default, if we see it's a problem then we'll address. For now is it OK?

For now, I'm adding this to our preview 2 epic so it can get documented.

@tmat
Copy link
Member

tmat commented Dec 12, 2018

@markwilkie
Copy link
Member

Thank @tmat - closing

@markwilkie
Copy link
Member

Sorry, closed too soon - waiting for @bricelam for any suggestions for a better way to approach this.

@bricelam
Copy link
Contributor Author

bricelam commented Dec 13, 2018

It’s no worse than KoreBuild.

One idea was to include a single .sln in the root and throw if there are 0 or more than 1 forcing you to specify the defaults. Could even have a by-convention default like <repo-name>.sln or build.proj.

@tmat
Copy link
Member

tmat commented Dec 13, 2018

Right now the convention is that by default we build all the solutions in the root.

@tmat
Copy link
Member

tmat commented Dec 13, 2018

If you have one that has all projects you want to build on CI and don't want to specify what to build in eng\Build.props then move the other ones somewhere else.

@bricelam
Copy link
Contributor Author

bricelam commented Dec 13, 2018

It’s kind of documented... But removing items in MSBuild is tricky I don’t think using the syntax in the docs would work (the items identity would have an extra .. and not be removed)

@tmat
Copy link
Member

tmat commented Dec 14, 2018

I'm not sure what syntax are you referring to.

<ItemGroup>
  <ProjectToBuild Remove="@(ProjectToBuild)"/>
  <ProjectToBuild Include="..." />
</ItemGroup>

Removes all items that were added implicitly and then adds whatever you want.

@bricelam
Copy link
Contributor Author

Ah, sorry. Since the thing I wanted was already included, I didn't think to "start fresh" like that. But yes, clearing the collection and re-adding the ones you want is certainly easier than just removing the ones you don't.

@natemcmaster
Copy link
Contributor

@bricelam I believe #1567 takes steps to address this. It simplifies what you need to do when you have multiple .sln files. Example in the docs:
https://github.com/dotnet/arcade/blob/f657be5cb7cd4920334dd9162173b131211a1e17/Documentation/ArcadeSdk.md#example-specifying-a-solution-to-build

<!-- eng/Build.props -->
<Project>
  <ItemGroup>
    <ProjectToBuild Include="$(RepoRoot)MySolution1.sln" />
  </ItemGroup>
</Project>

That said, do we really have anyone who expects to build multiple solution files? If not, a more sensible default might be to error when the default glob picks up more than one .sln and produce an error linking to the documentation.

@jonfortescue
Copy link
Contributor

@markwilkie -- confused on this one's relation to the helix epic as well

@markwilkie
Copy link
Member

My opinion is that this one probably still seems like it's worth doing. Anyone else?

If so, it probably belongs in the arcade sdk backlog.

@chcosta
Copy link
Member

chcosta commented Apr 1, 2021

What is left to do here that isn't covered by Nate's commit?

@MattGal
Copy link
Member

MattGal commented Apr 1, 2021

[Async Triage] I think this can be closed.

@dougbu
Copy link
Member

dougbu commented Apr 1, 2021

I don't have the unrecorded history here but suspect this was left open for

a more sensible default might be to error when the default glob picks up more than one .sln and produce an error linking to the documentation

On the other hand, Nate's question about whether any repo actually needs to build more than one .sln hasn't been answered in this issue.

@chcosta
Copy link
Member

chcosta commented Apr 1, 2021

IMO, expecting a single sln, but erroring if more than one is detected (with a link to the doc) and providing a model for explicitly changing the default to include more than one sln, sounds ideal. However, this seems like a "nice to have" at the moment or an "Up for grabs" type issue as I'm not sure of any business need here.

@dougbu
Copy link
Member

dougbu commented Apr 1, 2021

However, this seems like a "nice to have" at the moment or an "Up for grabs" type issue as I'm not sure of any business need here.

Makes sense @chcosta

@riarenas
Copy link
Member

riarenas commented Apr 1, 2021

Given that we still think it's nice to have, but no pressing business need, I think that makes it a good candidate to go in the backlog of the arcade sdk epic as Mark suggested #2053

@bricelam
Copy link
Contributor Author

bricelam commented Apr 2, 2021

Have you tried using solution filters instead of creating multiple solutions?

We eventually switch to this and removed our Build.proj file. As far as I'm concerned, this can be closed...

@markwilkie
Copy link
Member

Thanks @bricelam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants