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

Improvements in sfproj file format #885

Open
am11 opened this issue Oct 21, 2017 · 43 comments
Open

Improvements in sfproj file format #885

am11 opened this issue Oct 21, 2017 · 43 comments
Assignees
Labels
area-Tooling Relates to questions or issues related to experience enabled in VS to deploy or manage SF apps type-code-defect Something isn't working

Comments

@am11
Copy link

am11 commented Oct 21, 2017

The CPS-based csproj and vbproj has brought a revolution not only in project system, but also the project file format. For instance:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>

</Project>

It would be a wonderful improvement if all default target calls are wrapped into a ServiceFabric SDK (with option to override them in infrequent scenarios), so that the template project file will looks like:

<Project Sdk="Microsoft.Azure.ServiceFabric.Sdk">

  <ItemGroup>
    <ProjectReference Include="{path-to-my-csproj}" />
  </ItemGroup>

</Project>

This will:

  • reduce the 47 lines of XML in template to 5-7 lines
  • implicitly include all configs that are under ApplicationPackageRoot, ApplicationParameters, PublishProfiles and Scripts directories.
    • user can override it with a property like DefaultItemExcludes (that new csproj provides) for this purpose.
  • waive the requirement of having a separate packages.config that typically has Microsoft.VisualStudio.Azure.Fabric.MSBuild.

PS: is SF team considering to upstream the MSBuild patches to https://github.com/Microsoft/msbuild, so we are not required to pull Microsoft.VisualStudio.Azure.Fabric.MSBuild separately?

@Mardoxx
Copy link

Mardoxx commented Oct 21, 2017

Can't for the life of me find it... but I am sure I read that one of the SF team said that this was in the works and would cone once better xplat support was released? (Sf on linux?)

@mkosieradzki
Copy link
Contributor

Just to make sure that one important option is not removed.

I am changing inide all sfprojs DefaultTargets to "Package". It's very important because I am building multiple Service Fabric Projects as part of one VS Solution.

So in the new format I need to enable building packages on CI/CD pipeline easily.

@mikkelhegn
Copy link

@dbreshears

@dbreshears dbreshears self-assigned this Oct 24, 2017
@Igorbek
Copy link

Igorbek commented Dec 21, 2017

I was sure that microsoft/service-fabric-issues#194 was about that change, but apparently it only covered services projects and not .sfproj.
Is any plans to support SDK-based format of .sfproj that would use modern packaging (through PackageReference)?
/cc @ravipal @dbreshears

@NickDarvey
Copy link

If there's any work being done on the project system, please also consider making it compatible with props and targets from NuGet packages. Right now NuGet doesn't generate a obj/*.sfproj.nuget.g.targets or obj/*.sfproj.nuget.g.props file like it would for a csproj.

cc @ravipal

@lkts
Copy link

lkts commented May 13, 2018

Hi everyone, are there any updates on this? Current sfproj format causes annoying issues like https://github.com/Azure/service-fabric-issues/issues/840 when using ASP.NET Core projects with Fabric. Basically recommended for such projects "dotnet restore" functionality is not available to use now.

@am11
Copy link
Author

am11 commented May 13, 2018

With built-in custom SDK resolution implemented in MSBuild and recent improvements in NuGet APIs, SDK trend has started to gain more traction.
One concise example is: https://github.com/Microsoft/MSBuildSdks/tree/5580d48/src/Traversal. Another example which doesn't require an extra .nuspec file: https://github.com/aspnet/Razor/tree/a13a0aa/src/Microsoft.NET.Sdk.Razor

@mikkelhegn, @jeffkl, would it be a good idea to include sfproj's SDK in Microsoft/MSBuildSdks repo or in a separate directory under https://github.com/Microsoft/service-fabric/tree/15b6be4/src?

@jeffkl
Copy link

jeffkl commented May 14, 2018

I would keep your SDK in your own repo since you own and maintain it. Be sure to keep your SDK package as small as possible. Your build logic with tasks and a DLL should still be a NuGet package and your SDK can be a small Sdk.props and Sdk.targets that has a <PackageReference />.

MSBuild will not fully evaluate a project until the SDKs are resolved. If your SDK package is large, project load times will be bad and VS could hang. Packages are cached but the resolver still has to query the NuGet API which queries the disk. NuGet package references are restored more asynchronously so they can be much bigger.

@bklooste
Copy link

Still an issue

@pdylanross
Copy link

Jumping in on the pile here to say that this would improve our workflow a ton. Adding dotnet cli templates would also be hugely beneficial.

@aL3891
Copy link

aL3891 commented Oct 21, 2018

Any word on this one? would love to see it as well.
We've extended the sf packaging with nuget packages and now we have to manually edit the sfproj files to include the targets. having the new project system eliminate that step

@shacal
Copy link

shacal commented Oct 23, 2018

Kick - make it happen ! 🥇

@esbenbach
Copy link

Get going already :(

@chuanboz
Copy link

chuanboz commented Jan 4, 2019

It's 2019 now, any new update for this or plan to support this?

@chuanboz
Copy link

chuanboz commented Jan 4, 2019

Or, even this is not officially supported, has someone found ways to make sfproj to support package reference at a minimal? I am asking because then I could leverage default package props and targets from NuGet packages

@BigMorty
Copy link
Member

BigMorty commented Jan 8, 2019

Mike here from the VS team, I am the PM for the Service Fabric & Service Fabric Mesh tools in Visual Studio. We will be investigating this request soon, with the hope of supporting the new SDK-based project format for our .sfproj in the future.

@namvan
Copy link

namvan commented Jan 10, 2019

It would be nice, BigMorty. We are in the middle of migrating all our ~300 projects to nuget package reference and it proves to be very good and clean. Then here comes this one with service fabric projects stuck in package.config. It is a bit disappointed to find out.

@aL3891
Copy link

aL3891 commented Jan 11, 2019

That's great news @BigMorty, we've got some custom build targets for sf projects and the nuget integration in the new project system makes that so much easier

@chuanboz
Copy link

same as @aL3891 , we also have custom build targets to extend the sfproj to generate a deployable package to handle runtime certificate and other environment specific settings, if would be benificial if the new sfproj improvement could also clear document the extensions point in the built-in sfproj targets, to allow customize the ApplicationManifest.xml and ServiceManifest.xml generation.

@BigMorty
Copy link
Member

Thanks @aL3891 and @chuanboz for the thumbs up on this investigation. @chuanboz - ironically, we are also working on documenting the project properties available to developers to customize Service Fabric projects. We will share those here and/or in the docs.

@aL3891
Copy link

aL3891 commented Jan 19, 2019

Feel free tag me in PRs @BigMorty if you'd like some feedback or other assistance :)

@BigMorty
Copy link
Member

Thanks Allan!

@Onaiplee
Copy link

Really need this feature!

@BigMorty
Copy link
Member

Sorry to say but we are currently blocked from supporting SDK-style sfproj by the following issues:
dotnet/project-system#2491
dotnet/msbuild#4025

@wesleytsai
Copy link

Also looking for this

@amis92
Copy link

amis92 commented Apr 24, 2019

How is dotnet/msbuild#4025 blocking? 'Tis just a performance issue and not a very big one, considering that there is only one sfproj in solution (in most cases I've seen at least).

@abatishchev
Copy link
Contributor

Any update on this, please? Is work in progress? Is anything blocking by now?

@BigMorty
Copy link
Member

The two issues that are blocking this (dotnet/project-system#2491 and
dotnet/msbuild#4025) have still not been resolved. My understanding is 4025 is blocking us not only because of a performance issue but it also has led to hanging Visual Studio.

@aL3891
Copy link

aL3891 commented Oct 24, 2019

But then how could the mesh project files use the new sdk style?

@gkhanna79 gkhanna79 transferred this issue from microsoft/service-fabric-issues Apr 29, 2020
@ghost
Copy link

ghost commented Dec 3, 2020

Speaking as another user of Service Fabric, as a stopgap you might want to try using <RestoreProjectStyle>PackageReference</RestoreProjectStyle> with the existing sfproj format. We've been using that effectively in our repo with Microsoft.VisualStudio.Azure.Fabric.MSBuild/1.6.7.

E.g.: (Updated Jan 14)

    <!-- MyApp.sfproj -->
    <Project>
      <PropertyGroup>
        <RestoreProjectStyle>PackageReference</RestoreProjectStyle>
        <!-- Suppress Microsoft.Common.props from importing project extension props, as we're already importing them here. -->
        <ImportProjectExtensionProps>false</ImportProjectExtensionProps>
      </PropertyGroup>
      <Import Project="$(MSBuildProjectDirectory)\obj\*.g.props" />
      <Import Project="$(PkgMicrosoft_VisualStudio_Azure_Fabric_MSBuild)\build\Microsoft.VisualStudio.Azure.Fabric.Application.props" />
      ...
      <ItemGroup>
        <PackageReference Include="Microsoft.VisualStudio.Azure.Fabric.MSBuild" Version="[1.6.7]" GeneratePathProperty="true" />
      </ItemGroup>
      <Import Project="$(PkgMicrosoft_VisualStudio_Azure_Fabric_MSBuild)\build\Microsoft.VisualStudio.Azure.Fabric.Application.targets" />
    </Project>

This seems to work fine in VS 2017, but I've had problems loading such projects under VS 2019 -- just speculating, but maybe a regression in the sfproj plugin to VS? I just get an error "The operation failed as details for project ExampleApp could not be loaded". But it works fine in VS 2017, and works fine in command line builds and ADO YAML build pipelines.

We use this with dotnetsdk 3.1.405.

@ghost
Copy link

ghost commented Dec 29, 2020

I should add, the original above generates a warning on every build -- Microsoft.Common.props needs to be imported before Microsoft.VisualStudio.Azure.Fabric.Application.props in order to bring in the ProjectName.*.props files that provide the generated nuget paths. However, Microsoft.VisualStudio.Azure.Fabric.Application.props will unconditionally try to import Microsoft.Common.props a second time.

To work around the warning, we manually import the ProjectName.*.props files instead of Microsoft.Common.props and use <ImportProjectExtensionProps>false</ImportProjectExtensionProps> to disable Microsoft.Common.props' import of these files, but it's a bit brittle to maintain.

I've updated the post to reflect the new approach

@kaylumah
Copy link

@aaronla-ms looks promising. Especially the GeneratePathProperty, I did not know about that one. In my opinion, that is 100 times better than the ../../XX/../packages approach that gets generated by default.

Unfortunately, I am not able to get this to work. If I do this in Visual Studio by removing the packages.config and applying your changes, I get a message saying, "The project '..' has incompatible NuGet package installed. Would you like to install the compatible NuGet package?"'; VS2019 then decides to just create the packages.config again and for some reason also bumps the version to 1.7.3.

As for building via the CLI (dotnet build) it does not resolve the $(PkgMicrosoft_VisualStudio_Azure_Fabric_MSBuild) variable, making it impossible to resolve the props files. Tested via csproj variant, then this trick works perfectly.

Am I missing something?

We also use $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)\Service Fabric Tools\Microsoft.VisualStudio.Azure.Fabric.ApplicationProject.targets which ofcourse would also not work crossplatform or via dotnet build since MSBuildExtensionsPath32 resolves differently (see dotnet/sdk#16469 ) do you also have a solution for that?

@ghost
Copy link

ghost commented Mar 26, 2021

Hiya @maxhamulyak , yeah, there might be a few more items you might need -- we have a LOT of workarounds we've accumulated over the years to keep msbuild working for us, and it's not always clear which are needed in each scenario.

Here's all I could find that seemed possibly related. Try each setting in turn to see which might fix the issue.

We import this file immediately at the top of our project, before the first sfproj import.

<Project>
  <!--
    This adaptor helps .sfproj build using the PackageReference mechanism when
    building on retail toolsets

    Notes:
    -   The GetPackageRootFiles target in Microsoft.VisualStudio.Azure.Fabric.Application.targets will dynamically
        generate a project file, and then import the user's project into it. As such, we need to be careful not to
        use the MSBuildProject reserved property, as it will be inaccurate during certain build steps.

    -   The Service Fabric visual studio extension will error out if the first entry in the
        project is not the Microsoft.VisualStudio.Azure.Fabric.Application.props file it
        expects, with the following message:

        > The operation failed as details for project
        > {PROJECTNAME} could not be loaded.

        This is a major problem though. If our props file is not loaded first, then
        we can't import the .nuget.g.props file that provides the package
        restore location for the Microsoft.VisualStudio.Azure.Fabric.MSBuild package.

        However, it turns out if we import our props file using

            ```
            Condition="'$(BuildingInsideVisualStudio)' == ''"
            ```

        Then the Service Fabric visual studio extension will ignore it entirely, and our sfproj can still load.
  -->

  <PropertyGroup>
    <!--
      Placeholder framework version to make package resolution happy.

      Since .sfproj inherits from the C# project base targets, nuget restore using
      the PackageReference mechanism expects the project to have a target framework
      for resolving package versions.
    -->
    <TargetFramework>net462</TargetFramework>

    <!--
      Instruct Service Fabric vs extension to _not_ edit our sfproj without our permission.
      Normally, the sf vs extension will mangle the package imports, but it doesn't understand
      that we're using the new PackageReference mechanism.

      See issue https://github.com/microsoft/service-fabric-issues/issues/1388 for details.
    -->
    <DoNotUpdateNuGetImport>true</DoNotUpdateNuGetImport>

    <!--
      Disable the ResolveNuGetPackageAssets target.

      This target is only loaded under msbuild
      with visual studio installed, when the sfproj ends up importing $(ApplicationProjectTargetsPath).
      It's also broken when using PackageReference. And it's also not needed for sfprojs at all.

      Note that this does not appear to interfere with restore of nuget packages.

      Also note that nuget package restore is VS is busted still, even with this change. However,
      package restore from console (either `msbuild /t:Restore` or `dotnet restore`) works fine.
      Just make sure to restore before opening the solution.
    -->
    <ResolveNuGetPackages>false</ResolveNuGetPackages>
  </PropertyGroup>

  <!--
    Bring in generated props early to define various Pkg* paths.
    See issue https://github.com/microsoft/service-fabric/issues/885 for details.
  -->
  <PropertyGroup>
    <!-- Suppress Microsoft.Common.props from importing project extension props, as we're already importing them here. -->
    <ImportProjectExtensionProps>false</ImportProjectExtensionProps>
  </PropertyGroup>
  <Import Project="$(MSBuildProjectDirectory)\obj\*.g.props" />

  <!--
    ReferenceOutputAssembly=false, SkipGetTargetFrameworkProperties=false

    Note: sfproj should be frameworkless, but nuget.targets doesn't support that. However, since we declared
    our framework as net462, nuget restore may complain that the referenced service uses an incompatible framework.
    Use 'ReferenceOutputAssembly=false' to shut that down.

    This ItemDefinitionGroup section was sourced from Microsoft.Build.Traversal nuget, with the
    following copyright present:
    >
    > Copyright (c) Microsoft Corporation. All rights reserved.
    >
    > Licensed under the MIT license.
    >
  -->
  <ItemDefinitionGroup>
    <ProjectReference>
      <!--
        Setting ReferenceOutputAssembly skips adding the outputs of the referenced project to an item
      -->
      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
      <!--
        Setting SkipGetTargetFrameworkProperties skips target framework cross-project validation in NuGet
      -->
      <SkipGetTargetFrameworkProperties>true</SkipGetTargetFrameworkProperties>
    </ProjectReference>
  </ItemDefinitionGroup>
</Project>

@abatishchev
Copy link
Contributor

hey @masnider, do you know who (if anyone) drives this effort from the SF team's end?

@masnider
Copy link
Member

masnider commented Jul 7, 2021

We don't own the project format ourselves, it is driven by VS, so we need an update from @BigMorty and @dbreshears

@masnider masnider added area-Tooling Relates to questions or issues related to experience enabled in VS to deploy or manage SF apps type-code-defect Something isn't working labels Jul 7, 2021
@BigMorty
Copy link
Member

BigMorty commented Jul 9, 2021

Sorry, no updates from my end. There are the same blocking issues in the way. We will continue to monitor their progress.

@v-karbovnichy
Copy link

Another case to consider.

In my solution I tried to migrate from "nuget restore" to "msbuild /t:Restore" but could not.
When sfproj is a part of the solution, doing "msbuild /t:Restore" on a freshly cloned git repo gives
error : Unable to find the '..\packages\Microsoft.VisualStudio.Azure.Fabric.MSBuild.1.6.8\build\Microsoft.VisualStudio.Azure.Fabric.Application.props' file. Please restore the 'Microsoft.VisualStudio.Azure.Fabric.MSBuild' Nuget package.

This is happening because of the InitialTargets=";ValidateMSBuildFiles" attribute in the root Project element of sfproj.
From docs:

The InitialTargets attribute of the Project element specifies a target that will run first, even if targets are specified on the command line or in the DefaultTargets attribute.

So, the ValidateMSBuildFiles will blindly throw a error despite the fact that I am trying to restore nuget packages with my command.

@ghost
Copy link

ghost commented Sep 2, 2021

In my solution I tried to migrate from "nuget restore" to "msbuild /t:Restore" but could not.
When sfproj is a part of the solution, doing "msbuild /t:Restore" on a freshly cloned git repo gives
error : Unable to find the '..\packages\Microsoft.VisualStudio.Azure.Fabric.MSBuild.1.6.8\build\Microsoft.VisualStudio.Azure.Fabric.Application.props' file. Please restore the 'Microsoft.VisualStudio.Azure.Fabric.MSBuild' Nuget package.

It looks like you failed to update the import path to Microsoft.VisualStudio.Azure.Fabric.Application.props. If you're using msbuild restore, then your imports need to use the $(PkgMicrosoft_VisualStudio_Azure_Fabric_MSBuild) variable, not a hard coded path to ..\packages.

This is happening because of the InitialTargets=";ValidateMSBuildFiles" attribute in the root Project element of sfproj.

Have you tried removing that target? Note in my sample adaptation there was no InitialTargets specified.

@esbenbach
Copy link

@BigMorty and @masnider is this issue being worked on actively?
The fact that we need to go through all sorts of work arounds to get sfproj to build in the same way as "everything else dotnet" is making our CI/CD setup quite complicated.

Where a simple dotnet restore blah.sln && dotnet build blah.sln or similar with msbuild just works for everything but not for sfproj.

It would be really really nice if you could somehow prioritize this in the VS/MSBuild camp so we can get sensible sfproj handling in our CI/CD pipelines. Without building all sort of custom crap the replicates the sfproj concept.

@abatishchev
Copy link
Contributor

abatishchev commented Jan 13, 2022

@masnider doesn't work on the SF team team anymore. @craftyhouse is the PM.

The fact that we need to go through all sorts of work arounds to get sfproj to build in the same way as "everything else dotnet" is making our CI/CD setup quite complicated

+1. The amount of work doubles: first you need to restore nuget packages differently than for the rest of the project (i.e. .NET Core), then build differently.

@craftyhouse
Copy link
Collaborator

Tagging @sukanyamsft who is the PM For the core runtime to see if we can consider this in planning.

@BigMorty
Copy link
Member

This issue is not being worked on at this time by the tooling team.

@santo2
Copy link

santo2 commented Jan 24, 2022

why not? :-) it almost feels like service fabric isn't a priority anymore to get up to date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Tooling Relates to questions or issues related to experience enabled in VS to deploy or manage SF apps type-code-defect Something isn't working
Projects
None yet
Development

No branches or pull requests