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

Define spec for NuGet packages providing .editorconfig defaults #19028

Open
3 tasks
jasonmalinowski opened this issue Apr 26, 2017 · 46 comments
Open
3 tasks

Define spec for NuGet packages providing .editorconfig defaults #19028

jasonmalinowski opened this issue Apr 26, 2017 · 46 comments

Comments

@jasonmalinowski
Copy link
Member

There are some scenarios that exist where a NuGet package should be able to provide "default" .editorconfigs for a project. Consider the scenarios of:

  1. A package like StyleCop wants to change the defaults to the "traditional" StyleCop defaults.
  2. A company wants a package to specify their company-wide defaults that can be installed on all repos.

We need to design the following parts:

  • The compiler switches needed to pass such a flag along.
  • What the content is specified like in the NuGet package.
  • How such a package, when installed, knows it's running on a version of the compiler that actually supports this.

For the latter, NuGet content support might be sufficient since we could define a new item group that it produces items for.

@jasonmalinowski
Copy link
Member Author

(FYI to @sharwell)

@sharwell
Copy link
Member

sharwell commented Apr 27, 2017

The semantics I'm considering would be the following:

  • The .editorconfig files passed through this mechanism are evaluated without inheritance (it assumes without checking that root=true).
  • For evaluating paths in the .editorconfig sections, only the filename is provided. In other words, the .editorconfig file is assumed to be located in the same directory as the source file, regardless of where the source file is.
  • These files provide values for settings only in cases (at a per-property level) where the /editorconfig option (Add a (very formal) spec for exactly how .editorconfig applies in the compiler environment #18805) fails to provide a value.
    • The input is an ordered list of additional configuration files. The first file to provide a value is used.

Open questions:

  • How is this embedded in a NuGet package?
  • The order of inputs must be deterministic with respect to the set of sources of these additional files provided. But what is that order?

@jasonmalinowski
Copy link
Member Author

Your last two questions are the ones that interest me most.

@sharwell
Copy link
Member

@jasonmalinowski Are NuGet packages referenced by a project ordered when using <PackageReference>?

@jasonmalinowski
Copy link
Member Author

MSBuild ItemGroups implicitly have an order. Whether that order is preserved through the tooling is definitely not something I'd bet a dollar on.

@AdamWillden
Copy link

AdamWillden commented Nov 7, 2018

In relation to what @sharwell proposed above:

The .editorconfig files passed through this mechanism are evaluated without inheritance (it assumes without checking that root=true).
For evaluating paths in the .editorconfig sections, only the filename is provided. In other words, the .editorconfig file is assumed to be located in the same directory as the source file, regardless of where the source file is.

I suggested something different in the proposed solution in #29592:

The issue lies in how the solution structure and physical folder structures combine to be interpreted for the purposes of inheritance. The easiest solution here is to only allow virtual files to be found at a project root. Then if a physical file also exists at the project root, the virtual file would take the role of the parent such that a project may override the distributed (virtual) version using a physical file.

As for the question:

How is this embedded in a NuGet package?

See this linked stylecop example also mentioned in #29592

@andersarpi
Copy link

Any update on this? Being able to centralize .editorconfig generation seems to be very sought-after by .net core developers.

@TheXenocide
Copy link

It seemed like we had some luck with including one in a NuGet package, but now I'm now sure if it's quite working consistently. That said, I have some thoughts on how this might work and be prioritized. Here's what I propose:

A new MSBuild ItemDefinition, EditorConfig, with Priority (for sorting the layering as it would if they were in different tiers of the hierarchical folder tree) and ConfiguredPath Metadata (for specifying directories deeper than the project file, or maybe for specifying configuration for source files included in the NuGet package). A NuGet package could package the file in their build/buildTransitive directory along with a props/targets file that Includes the EditConfig Item. I think by default an .editorconfig in the project directory or a configured path would apply last and override any settings supplied by a NuGet package; maybe this would be done by having the Microsoft.NET.Sdk glob **/.editorconfig with a very high Priority number? Perhaps something predictable so if someone wanted to make a package that overrides local settings it could? Perhaps other items would default to 0 and simply apply in the order they are processed. Here's a thought of what a Sample.Package.targets file would look like:

<Project>

  <PropertyGroup>
    <SamplePackageEditorConfigPriority Condition="'$(SamplePackageEditorConfigPriority)' == ''">0</SamplePackageEditorConfigPriority>
  </PropertyGroup>

  <ItemGroup>
    <EditorConfig Include="@(MSBuildThisFileDirectory)Sample.Package.editorconfig"> 
      <ConfiguredPath>$(MSBuildProjectDirectory)</ConfiguredPath>
      <Priority>$(SamplePackageEditorConfigPriority)</Priority>
    </EditorConfig>
  </ItemGroup>

</Project>

Thoughts?

@JimBobSquarePants
Copy link

I'm confused. I thought allowing a custom location was already allowed in VS 16.7?

https://twitter.com/davkean/status/1296314723512639490/

@AdamWillden
Copy link

I'm confused. I thought allowing a custom location was already allowed in VS 16.7?

https://twitter.com/davkean/status/1296314723512639490/

I wish they would document this stuff, least I missed it.

So, I have had a colleague try to do this. He says it definitely works for C# rules but not others. For instance we had a rule that states csproj files must use indent size 2. This apparently has no effect:

It's roslyn that enforces all the dotnet_ and csharp_ rules, the rest need to stay where they are for VS.

So it is a partial solution, meaning we can centralise the important stuff (so thank you for sharing) but I'd like to see it support all rules.

Also to note, we had issues with is_global so have ommitted this entirely in our setup.

@JimBobSquarePants
Copy link

I wish they would document this stuff, least I missed it.

I raised an issue against the docs for it a while back, hopefully it'll get added soon. https://github.com/MicrosoftDocs/visualstudio-docs/issues/6051

@julealgon
Copy link

While this is still under discussion, are there any known workarounds for sharing an .editorconfig file across multiple repositories without incurring the code duplication?

Should git submodules be considered for this in the meantime? Has anyone tried that?

I'm not a fan of creating a dependency between repositories like that, but I fail to see any other alternative at this point. Would be interested if anybody has come up with a different solution.

@deuko
Copy link

deuko commented May 7, 2021

I am also curious about some workaround, as I am currently in the process of migrating a giant Perforce Monorepo to Git. This scenario hit me right now, because all projects were referencing a single ruleset file (which is deprecated anyway now) in this Monorepo. Splitting those up to multiple repos create the need for distributing those rules (= .editorconfig files) to multiple repos.

From what I've read git submodules is not applicable because it references the repo as a subfolder, therefore the .editorconfig file would not lay in the project root and therefore not work?

Distributing the file via NuGet package sounds interesting, but it would create the need for creating a custom build target that copies the file back to the project root on build, or am I wrong here?

A minimalistic solution would be best, because those changes quickly affect a very large number of repos

@julealgon
Copy link

From what I've read git submodules is not applicable because it references the repo as a subfolder, therefore the .editorconfig file would not lay in the project root and therefore not work?

@deuko i feel like this should be doable somehow, perhaps leveraging a symbolic link that references the editor config inside the submodule folder in the root of the repository. IIRC you can have symbolic links merged into the repo and apply automatically to all developers.

It's far from ideal but maybe it's an option. I have not tried it though as I've since changed projects into a monorepo solution.

@JimBobSquarePants
Copy link

@deuko I copy on build as a workaround for now.

https://github.com/SixLabors/SharedInfrastructure/blob/0ea21d9e2a76d307dae9cfb74e33234b259352b7/msbuild/targets/SixLabors.Src.targets#L8-L18

@cmenzi
Copy link

cmenzi commented May 7, 2021

We achieved it with .props and .targets file with our CodingGuideline nuget.

We added everything into two .globalconfig files, one for codestyle and one for the all rules of the different analyzers and their severities.

And added a check that the .editorconfig is not compliant. The .editorconfig does not have any technology or C# analyzer specific stuff in and only defines spacing for the different file types.

So, everything are in the .globalconfigs coming from our nuget.

@MisinformedDNA
Copy link

@cmenzi Do you have an example of this?

@sharwell
Copy link
Member

If anything, it would make more sense for .sln files to become msbuild project files before any changes are made in NuGet in this regard.

This would generally be a request for dotnet/msbuild, but in this case solution files are already treated as a special kind of project file. If you build with /bl, the Files tab will show a .sln.metaproj file corresponding to the build representation of a solution file.

image

@jmarolf
Copy link
Contributor

jmarolf commented May 20, 2021

@jasonmalinowski @mavasani @chsienki hasn't this item been completed via globalconfig files?

@sharwell
Copy link
Member

sharwell commented May 20, 2021

@jmarolf Is there a document describing the semantics for .globalconfig when it comes from a NuGet file? Specifically, I'd be looking for a document explaining how rules in a .globalconfig coming from a NuGet package can be overridden by a .globalconfig in the solution directory without reporting warnings about multiple configurations for the same key. Once the spec exists, we can link it here and close the issue.

@mavasani
Copy link
Contributor

mavasani commented May 20, 2021

@sharwell I believe you can get some information from #49834. Tagging @chsienki and @gewarren as well - we should add information about global_level property for precedence between globalconfig files to the documentation at https://docs.microsoft.com/dotnet/fundamentals/code-analysis/configuration-files#global-analyzerconfig

@chsienki
Copy link
Member

@sharwell @mavasani @jmarolf Yeah, this is covered by globalconfig, and the global_level rules do now allow for local overrides of nuget provided config. Agree we need to get it doc'd.

@gewarren whats the best approach to getting this updated in the docs?

@gewarren
Copy link
Contributor

I've created a docs issue here: dotnet/docs#24312. I'm happy to take a stab at it and you can review, or the other way around @chsienki.

@MisinformedDNA
Copy link

I created a very simple example of building distributable .editorconfig defaults. I also published it to NuGet.

@justinmchase
Copy link

Eslint solves this by having a formal "extends" semantic in the config.

{
    "extends": [
        "eslint:recommended",
        "plugin:@typescript-eslint/recommended"
    ],
}

Ideally the editor config would have a similar semantic, which would have knowledge of how to resolve configs from nuget packages, perhaps something like:

root = true
extends = nuget:ExampleSharedPackage

That would resolve that package reference, and then look for a .editorconfig file in the root of that package. You'd probably have to merge them in reverse order and possibly solve for circular references but something like this would be very helpful.

@binki
Copy link

binki commented Jun 6, 2022

.editorconfig isn’t meant to be discovered this way. It is meant to be project/environment agnostic. Having VisualStudio know to load NuGet packages to automatically find .editorconfig means using project/environment specific means. .editorconfig is discovered through searching directory parents.

If there is a way to share .editorconfig across multiple repos, it’d probably make more sense to use npmjs if anything at all. And this should be part of the .editorconfig specification itself. Copying the files in as part of your build process and adding .editorconfig to your .gitignore is a much less wrong way of doing things, considering how things work in general.

Also, you’re already doing something to keep your various projects’ .gitignore synchronized (such as a common merge source repository). You could just use the same approach for .editorconfig.

@markusroessler
Copy link

after several hours of trail and error I ended up with the following solution/workaround (I hope this helps someone):
create a folder with the following structure:

  • build
    • {PackageId}.props
    • {PackageId}.globalconfig
  • {PackageId}.nuspec

{PackageId}.props

<Project>
    <PropertyGroup>
      <AnalysisMode>All</AnalysisMode>
      <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
    </PropertyGroup>

    <ItemGroup>
      <GlobalAnalyzerConfigFiles Include="$(MSBuildThisFileDirectory)\{PackageId}.globalconfig" />
    </ItemGroup>
  </Project>

{PackageId}.nuspec

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
  <metadata>
    <id>{PackageId}</id>
    <description>{PackageId}</description>
    <version>1.0.0-local.1</version>
    <authors>mroessler</authors>
    <repository type="git" />
  </metadata>
</package>

{PackageId}.globalconfig
configure your default rules here

  • pack and publish this folder using nuget.exe.
  • consume the package using Directory.Build.props in target solution
<Project>
    <ItemGroup>
      <PackageReference Include="{PackageId}" Version="1.0.0-alpha.6"/>
    </ItemGroup>
</Project>

note: you can overwrite rules provided by the package using an .editorconfig file in the target solution

@justinmchase
Copy link

justinmchase commented Jan 2, 2024

<GlobalAnalyzerConfigFiles>

Awesome, had no idea that was there. Thank you for dropping that here.

@julealgon
Copy link

I'm extremely happy to see the new .slnx format for solutions in VS 2022 Preview 4. I wonder when that will get proper documentation (not even the Preview 4 release notes have been added yet).

Hopefully the transition to this new format will finally allow us to have first-class support for "solution packages".

@justinmchase
Copy link

Can you tell us more? Where did you hear about it if there is no docs? And most importantly when? :)

@anreton
Copy link

anreton commented Apr 17, 2024

Can you tell us more? Where did you hear about it if there is no docs? And most importantly when? :)

About .slnx?

Visual Studio 2022 17.10 Preview 3 added this feature.

Some videos:
https://www.youtube.com/watch?v=wzMMclD8QsI
https://www.youtube.com/watch?v=D0MxmDWk4t0

@julealgon
Copy link

@justinmchase

Can you tell us more? Where did you hear about it if there is no docs? And most importantly when? :)

I got to know about it from Nick's video (posted by @anreton above).


@anreton

Visual Studio 2022 17.10 Preview 3 added this feature.

Oh, was it already available on 3? I guess that makes sense... Apparently, Preview 4 dropped yesterday (release notes are up now)

@timmkrause
Copy link

I'm extremely happy to see the new .slnx format for solutions in VS 2022 Preview 4. I wonder when that will get proper documentation (not even the Preview 4 release notes have been added yet).

Hopefully the transition to this new format will finally allow us to have first-class support for "solution packages".

Directory.Build.props do not work for you? Not convenient, but it works.

@julealgon
Copy link

@timmkrause

I'm extremely happy to see the new .slnx format for solutions in VS 2022 Preview 4. I wonder when that will get proper documentation (not even the Preview 4 release notes have been added yet).
Hopefully the transition to this new format will finally allow us to have first-class support for "solution packages".

Directory.Build.props do not work for you? Not convenient, but it works.

I would suggest reading through this whole thread here. Directory.Build.props is not the same as "solution packages": all it does is avoid the duplication of code to install the same packages in multiple projects. Project-level packages don't solve some issues such as this one, where you need something that is outside of any single project, like putting an .editorconfig file in the root of the solution to apply to all projects.

If the new .slnx format is MSBuild-compatible (which I really hope is the case), then now adding support for <PackageReference> elements for the solution alongside the creation of "solution packages" in the NuGet spec, would completely solve this problem.

@sharwell
Copy link
Member

Project-level packages don't solve some issues such as this one, where you need something that is outside of any single project, like putting an .editorconfig file in the root of the solution to apply to all projects.

The package can provide a .globalconfig instead. These files are not bound by the hierarchical nature of the file system. All of our own features that provide configuration as part of a NuGet package do so with .globalconfig files.

@TheXenocide
Copy link

Seeing there's some revitalized conversation here about .globalconfig I just want to be sure:

We shouldn't expect .globalconfig to be usable for specifying things like indentation/new lines/brace preferences and other non-analyzer editor settings, right?

As for

Project-level packages don't solve some issues

I do agree with this sentiment, though perhaps its a bit tangential to this conversation. Having an option like this would make for a much better experience for several scenarios we encounter, some of which are currently (less-than-ideally) "solved" using Command Task Runner for pre/post-solution build events (which do not even know if the build succeeded or not). Incrementing local dev package versioning (particularly in non-git code bases) can be challenging to orchestrate at the project-level because each project calculates its own version but uses that version for all dependencies on packages inferred from project references, though when those projects build the incrementing code has a difficult time sharing previously calculated information and so it calculates a new version and then the packages in a single solution build do not correctly use the version numbers of packages actually produced. Doing this outside in command task runner at least allows a script to calculate the version once and then pass it in to all projects, but the task runner explorer extension is not exactly actively maintained and does not work consistently so we consider the mentioned approach a workaround we're trying to get away from.

Many of our challenges in this regard come from local development workflows in a code base where many separate solutions produce outputs which need to be composited together.

Other examples include:

  • Loose reference management (i.e. "plugins")
  • Stopping/starting services (especially in conjunction with deploying loose references over potentially locked existing files)

I'm sure there's more, but those are the ones that first come to mind off the top of my head.

@TheXenocide
Copy link

I created a very simple example of building distributable .editorconfig defaults. I also published it to NuGet.

@MisinformedDNA Just going through and looking at messages I may have missed on this thread, I just took a look at your package and am wondering if this still works today? It looks like it might solve the problem we're looking to solve the way we're trying to solve it.

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

No branches or pull requests