Code Monkey home page Code Monkey logo

Comments (24)

cmeeren avatar cmeeren commented on May 17, 2024 1

Would love to see support for .NET 7! In general, I'd like to see something that is likely to work with future .NET versions, so that consumers are not forced to use older .NET versions for a long time because an important CI tool does not work with the most recent .NET. This tool otherwise looks to be what I need, but the possibility of being stuck for months on old .NET versions seems like too big a tradeoff.

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024 1

I'll try it out when I have the time. 👍

As for supporting newer .NET versions automatically: I see that nx-dotnet advertises the following (emphasis mine):

Built using the .NET SDK + CLI, nx-dotnet is easy to update and should never break due to a new relase of .NET. Using a preview version? No worry, since nx-dotnet uses your installed CLI you can choose exactly what to run.

Perhaps you could investigate what they're doing and see if it's possible to do something similar?

(The reason I'm not using nx-dotnet is that it is a plugin for a build ecosystem I'm not using and would rather not have to get into.)

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024 1

Took a very quick look, I think the reason nx-dotnet can claim that "should never break due to a new relase of .NET." is because it is developed on javascript and executes the dotnet-cli under the hood.
image
So, as long as the dotnet CLI doesn't change its API, and nodejs/NX does not introduce any breaking changes, nx-dotnet should work.

Dotnet-affected is developed 100% on dotnet, hence it may be affected by the .NET runtime executing the process.
image

That being said, I would love to be less dependent on framework version and I'm open to suggestions on how we can improve that. I'll do some research as well of other CLIs to see how they handle it.

An alternative, which may be more resilient is to use the MSBuild Task instead of the CLI. Gonna research if it is actually more resilient or not.

from dotnet-affected.

AgentEnder avatar AgentEnder commented on May 17, 2024 1

I would like to give @AgentEnder a chance to reply as well in case I am missing something or said something incorrectly. @AgentEnder, sorry if I'm saying anything wrong, kindly correct me if I'm missing something :)

Hey yall! This is certainly an interesting conversation. nx-dotnet is only able to maintain its SDK compatibility range due to us calling the CLI rather than invoking anything programmatically when it comes to .NET. This has benefits and drawbacks, on both sides.

In regards to the internal mechanisms, we currently do parse .csproj / vbproj / fsproj files to determine project dependencies. There are some serious limitations to that (in addition to the code just being ugly), so I'd like to use https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-list-reference in the future to remove some of the XML parsing + lookup.

I think there is room for both tools, since they achieve the same results differently, one may be more appropriate for some teams than the other.

💯, nx-dotnet requires the usage of Nx. Some teams which do not use JS at all will currently take issue with this, but we are working on breaking down some of those barriers. Nx 15.8 includes a new "Encapsulated" mode that works a bit like gradle's wrapper. See our demo repo that uses .NET in this new mode: https://github.com/agentender/encapsulated-dotnet-repo

Using Nx gets you access to task-level caching, affected, the graph visualization, nx-cloud, etc. This stuff is all really great IMHO, but some teams may be adverse to changing their build tooling so far away from the current ecosystem. Definitely room for both tools :)

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024 1

Nx 15.8 includes a new "Encapsulated" mode that works a bit like gradle's wrapper. See our demo repo that uses .NET in this new mode: https://github.com/agentender/encapsulated-dotnet-repo

Ooh, that's interesting. I'll check that out.

By the way, does nx-dotnet and/or dotnet-affected take into account, say, changes to files in wwwroot for ASP.NET Core projects? Or including projects that are affected by changed Directory.Build.props?

from dotnet-affected.

AgentEnder avatar AgentEnder commented on May 17, 2024 1

nx-dotnet uses Nx's default mechanism for affected, its something we can't really change right now as there are not hooks available for plugins in that mechanism (this is something we are actively discussing if it should be available within Nx's core team).

Any file under the project root changing will mark the project as 'touched' for affected currently. Nx offers inputs configuration for targets, which you could then use to get higher cache hit rates for the individual projects if you wanted to exclude some files from under the project configuration.

from dotnet-affected.

AgentEnder avatar AgentEnder commented on May 17, 2024 1

So, i.r.t files outside of the project root, with nx-dotnet they can also still be used to mark things as affected if they are specified as an input for the task. We don't do that automatically though, its something you'd have to keep track of.

Generally, I don't think its a huge issue since aside from Directory.build.props and similar files your source code / assets should be under the project directory.


I think the dotnet list reference should give us what we want, since we are only looking at project -> project level dependencies, rather than file level deps. It could be interesting to see how the DotnetAffected.Core package may be leveraged, but sticking with the CLI tooling does have the benefits of being mostly SDK agnostic. Every now and then the .NET team will do some weird swapping of flags or similar, but we can work around that kind of thing pretty easily instead of waiting on a dependency to handle it.

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024 1

Generally, I don't think its a huge issue since aside from Directory.build.props and similar files your source code / assets should be under the project directory.

Common build pipeline steps is an important exception to this. As is common .NET tools. For example, if I install a linter as a local dotnet tool, then updates to the linter version may be reason to trigger a rebuild.

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024 1

I'm still a bit fuzzy on exactly what dotnet-affected detects. For any of the following, does it either detect and consider projects changed, or allow users to manually specify which projects should be changed (using e.g. a glob pattern)?

  1. Changes to a common build pipeline step (factored out to a separate file)
  2. Updates to a local .NET tool in the repo root
  3. Changing a compiler warning to a compiler error (or other non-obvious stuff that may influence a build) in a Directory.Build.props at some level (remember, these files may be hierarchical if you import from a higher level)

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Hi @omer-za , sorry for the late reply.

This seems to be the same issue as my PR here. It should be a dumb package version issue, I'm hoping to find a few hours to fix it soo.

Plan is to release v3 with dotnet 7 support.

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Hi @cmeeren , I agree with everything you've said, however supporting newer dotnet versions without any changes may be harder said than done, although I would love if we could achieve that.

Support for dotnet 7 was implemented on PR #27

dotnet-affected v3.0.0 was just released adding support for dotnet 7.
Feedback appreciated.

Regards,
Leo.

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Nice, I'll take a look. I know the project but haven't checked it in a while.

I think it is a great alternative for dotnet-afected for projects already using NX for angular/react/etc.

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024

But assuming that this tool invokes the .NET CLI where possible (like nx-dotnet), I think this won't be much of an issue. You could have a fairly simple tool that, while compiled for, say, .NET 7, is highly likely to work without problems on .NET 8+, because it does not directly use more than fairly basic .NET functionality/packages and therefore are unlikely to be hit by breaking changes between .NET versions.

And if that does not suffice, you can create a self-contained, single-file exe that includes the correct runtime of .NET (and invokes the .NET CLI like nx-dotnet does). That should put an end to this issue once and for all.

In short: As long as you invoke whatever .NET CLI the user has, like nx-dotnet, you will likely only be hit by breaking changes to the .NET CLI API.

Or have I misunderstood?

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Thanks for the feedback @cmeeren!
You are definitively on the right track, however, I think there's a fundamental difference on how nx-dotnet and dotnet-affected determines which projects are affected by the changed projects/files.

I am not familiar with nx-dotnet internals so I may be missing something, however, according to my previous discussion with nx-dotnet's author, and after taking a look at the nx-dotnet parsing code I think nx-dotnet parses and interprets dotnet csproj,etc, XML project files.

dotnet-affected uses MSBuild to interpret project files and compose the dependency project graph. This means that dotnet-affected is able to "read a project like MSBuild does". In practice, this means dotnet-affected doesn't need to know all of the possible XML tags that dotnet supports. Instead, we leverage MSbuild.Prediction to predict the files that MSBuild will need for each project. When any of those files have a change, the project/s referencing those files are affected, and projects referencing those projects (by whatever means, Directory.Build.props and other dotnet thingies) will get detected.
(An example of this is #67, where we add support for fsharp/vbproj quite easily, (touching wood))

In short: As long as you invoke whatever .NET CLI the user has, like nx-dotnet, you will likely only be hit by breaking changes to the .NET CLI API.

I think that is the main difference: dotnet-affected does not invoke dotnet CLI: the user does so by using dotnet build affected.proj. dotnet-affected, being a tool written in dotnet, can leverage all the tools that dotnet uses internally to build a project, but uses for affected detection instead.

As I've mentioned before, I really like nx-dotnet. I think there is room for both tools, since they achieve the same results differently, one may be more appropriate for some teams than the other.

I would like to give @AgentEnder a chance to reply as well in case I am missing something or said something incorrectly. @AgentEnder, sorry if I'm saying anything wrong, kindly correct me if I'm missing something :)

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024

Thanks for the excellent clarification.

Perhaps users having problems can invoke the tool using dotnet-affected --roll-forward Disable (or another suitable value) to ensure that dotnet-affected is run with a supported .NET version?

I'm not sure how MSBuild.Prediction works. Would the above cause problems if running dotnet-affected using .NET 6 (with MSBuild.Prediction built for .NET 6), but the projects you run it on target .NET 7?

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Perhaps users having problems can invoke the tool using dotnet-affected --roll-forward Disable (or another suitable value) to ensure that dotnet-affected is run with a supported .NET version?

I'm not sure how MSBuild.Prediction works. Would the above cause problems if running dotnet-affected using .NET 6 (with MSBuild.Prediction built for .NET 6), but the projects you run it on target .NET 7?

So, when having a different dotnet version installed than the ones dotnet-affected targets, I think the only issue should be if there are breaking changes in MSBuild packages public API, in which case it may fail miserably.. This is definitively a point to improve.

dotnet-affected also tries to use new features of newer MSBuild APIs, for example, we implement a virtual file system to read all projects of a commit at a certain time, that can only be done with msbuild > 16. We have checks to see what MSBuild version is being used and react accordingly.

That being said, it took a while to get v3 out with dotnet 7 support due to my personal agenda. The changes themselves were easy, and the PR for supporting dotnet 7 was created when dotnet 7 was on preview..


Hi @AgentEnder, thank yo so much for chiming in 👍 your opinion is greatly appreciated. I agree with everything you've said, and I think it would be awesome for nx-dotnet to leverage the new list-reference API, 🚀

By the way, does nx-dotnet and/or dotnet-affected take into account, say, changes to files in wwwroot for ASP.NET Core projects? Or including projects that are affected by changed Directory.Build.props?

I think this is a great question, where dotnet-affected shines:

dotnet-affected will detect projects included from Directory.Build.props and Directory.Build.Targets and any other file that is referenced directly / indirectly by the MSBuild project. These are the predictors we use:

private static readonly IProjectPredictor[] ProjectPredictors = new[]
{
new AvailableItemNameItemsPredictor(), new ContentItemsPredictor(), new NoneItemsPredictor(),
(IProjectPredictor)new CopyTaskPredictor(), new CompileItemsPredictor(),
new IntermediateOutputPathPredictor(), new ProjectFileAndImportsPredictor(),
new CodeAnalysisRuleSetPredictor(), new AssemblyOriginatorKeyFilePredictor(),
new EmbeddedResourceItemsPredictor(), new ReferenceItemsPredictor(), new ArtifactsSdkPredictor(),
new StyleCopPredictor(), new ManifestsPredictor(), new VSCTCompileItemsPredictor(),
new EditorConfigFilesItemsPredictor(), new ApplicationIconPredictor(),
new GeneratePackageOnBuildPredictor(), new DocumentationFilePredictor(), new XamlAppDefPredictor(),
new TypeScriptCompileItemsPredictor(), new ApplicationDefinitionItemsPredictor(), new PageItemsPredictor(),
new ResourceItemsPredictor(), new SplashScreenItemsPredictor(), new TsConfigPredictor(),
new MasmItemsPredictor(), new ClIncludeItemsPredictor(), new SqlBuildPredictor(),
new AdditionalIncludeDirectoriesPredictor(), new AnalyzerItemsPredictor(),
new ModuleDefinitionFilePredictor(), new CppContentFilesProjectOutputGroupPredictor(),
new LinkItemsPredictor()

Initial versions of dotnet-affected used to assume that the only files a project depended on were the ones located inside the project directory. In order to proper support Directory.Build.props and Directory.Build.Targets , Directory.Packages.props, signing keys, etc, we had to go as close as MSBuild as possible. That's where the predictors come in.
For example, the CompilePredictor will take care of detecting all files to be compiled independently if they were specified with the <Compile /> element or inferred (like csproj does)

By the way @AgentEnder , we now have DotnetAffected.Core package which includes this this all logic in a class library for other projects to use. Thinking out loud, but if dotnet list-refernece API does not give you what you need for nx-dotnet, perhaps we may be able to quickly write a CLI tool that exposes what you need using DotnetAffected.Core package and leveraging the internals of dotnet-affected for change detection, etc. Again just thinking outl oud.
Also, JIC, we also have an MSBuild Task that can be used by MSBuild directly!

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Thank you @AgentEnder for the clarification 👍

Generally, I don't think its a huge issue since aside from Directory.build.props and similar files your source code / assets should be under the project directory.

So when dotnet-affected supported only files inside each project without MSBuild.Prediction, we used to have a build step that would do git diff over the "global files" of our monorepo. If those files had changed, we just built everything without dotnet-affected.
Unfortunately, that list grew too big, and in many cases we were building projects that were not really affected by the changed files.

I know nx works differently, since I think you can specify for each project the dependent files, if I'm not mistaken.
We also had some other cases:

  1. multiple Directory.Build.props / .targets
  2. .props/.targets referencing other .props/.targets

So we basically made it our goal to be able to detect the files that MSBuild needs to build each project.

Also, dotnet-affected detects package version changes from Directory.Pacakge.props and will determine which projects are affected by those packages as well.
We basically try to make it work using all the information that MSBuild already uses.


I think the dotnet list reference should give us what we want, since we are only looking at project -> project level dependencies, rather than file level deps.

Makes sense, we work at file level since we use git diff to determine changed files, then we map those files to each MSBuild Project discovered.

It could be interesting to see how the DotnetAffected.Core package may be leveraged, but sticking with the CLI tooling does have the benefits of being mostly SDK agnostic. Every now and then the .NET team will do some weird swapping of flags or similar, but we can work around that kind of thing pretty easily instead of waiting on a dependency to handle it.

Definitively, I agree. Build pipeline tools should be as resilient as possible. We try to keep dependencies thin: DotnetAffected.Core depends on MSBuild, Lib2GitSharp and Microsoft.Build.Predictions.

I am glad for this exchange of ideas @AgentEnder , I think with the information in this issue we may be able to write a comparison of both our tools to be able to reference users when they ask this question. Hoping to do so when I find the time.

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Sure thing, I am happy to answer @cmeeren .

  1. Are we talking an MSBuild task here? It should detect all files that are directly or indirectly referenced in the cs/fs/vb proj file
  2. When the .config/dotnet-tools.json changes? I don't think so (but i'll confirm), since there's no directly/indirectly reference from MSBuild projects to this file, and AFAIK, having the tools is not needed to build a project using dotnet build.
  3. Yes, hierarchical changes to Directory.Build.props/targets is supported, and is validated by this test

EDIT:

allow users to manually specify which projects should be changed (using e.g. a glob pattern)?

Only way to specify which projects have changed manually is using the --assume-changes flag, which does not support glob patterns

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024
  1. Are we talking an MSBuild task here? It should detect all files that are directly or indirectly referenced in the cs/fs/vb proj file

I was thinking a GItHub actions or Azure Pipelines yaml file. I know of course this can't be automatically picked up, but my desire is that it should be possible to specify in some manner to do a full rebuild (or mark certain projects/paths as changed) when certain otherwise unrelated files change. (This is also relevant for point 2.)

  1. Yes, hierarchical changes to Directory.Build.props/targets is supported, and is validated by this test

Thanks! Do they pick up any change to d.b.props, or only a subset?

Only way to specify which projects have changed manually is using the --assume-changes flag, which does not support glob patterns

I see! Would be fantastic if it did (and allows multiple paths). That would make it possible (if a bit cumbersome) to trigger a build of all relevant projects for changes to arbitrary unrelated files.

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

I see.

For 1 and 2, would you mind adding those unrelated files to the project file using something like <Content Include="../../../my-urealted-file.smth" /> ? If you do that, dotnet-affected should detect it.

I am obliged to mention, that if I'm not mistaken, 1 and 2 should be easy for nx-affected 👍

Thanks! Do they pick up any change to d.b.props, or only a subset?

Should pick up any changes. We don't really read the file, we just know which projects hierarchically depend on it (and it's referencing projects) and mark them affected.

I see! Would be fantastic if it did (and allows multiple paths). That would make it possible (if a bit cumbersome) to trigger a build of all relevant projects for changes to arbitrary unrelated files.

It does support multiple paths yes, by adding multiple --assume-changes <project name> (not paths)

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024

would you mind adding those unrelated files to the project file using something like `"?

Not sure what you mean by `". Is that a typo?

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Sorry @cmeeren it was a formatting error.
I meant adding them to the csproj using something like <Content Include="../../../my-urealted-file.smth" />

EDIT: perhaps better to use <None Include="../../../my-urealted-file.smth" />

from dotnet-affected.

cmeeren avatar cmeeren commented on May 17, 2024

If that works, then I'm happy. It seems like a fairly simple solution.

Does that work if adding such lines to Directory.build.props, too? Will all "external" files referenced there cause the project to be considered changed when the external files change?

from dotnet-affected.

leonardochaia avatar leonardochaia commented on May 17, 2024

Good day @cmeeren

Does that work if adding such lines to Directory.build.props, too? Will all "external" files referenced there cause the project to be considered changed when the external files change?

I think it should work, but we would need to confirm. Basically we already have a test that covers "when the directory build props has imports to other props files" so I think it should work.

I propose we continue the discussion on the other issue you've created 👍

from dotnet-affected.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.