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

dotnet pack - version suffix missing from ProjectReference #4337

Closed
natemcmaster opened this Issue Jan 19, 2017 · 41 comments

Comments

Projects
None yet
@natemcmaster

natemcmaster commented Jan 19, 2017

Follow up to #3953

Repro
ProjectA has project reference to ProjectB.
ProjectB contains <VersionPrefix>3.0.0</VersionPrefix>
dotnet pack ProjectA.csproj --version-suffix build001

Expected
ProjectA.nupkg has dependency on ProjectB 3.0.0-build001

Actual
ProjectA.nupkg has dependency on ProjectB 3.0.0

Details
Using 4.0.0-rc3
Worked in project.json, doesn't work here.
Workaround: msbuild "/t:Restore;Pack" /p:VersionSuffix=build001". Set VersionSuffix property on all projects during restore, not pack.

@natemcmaster

This comment has been minimized.

Show comment
Hide comment

natemcmaster commented Jan 19, 2017

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jan 20, 2017

Contributor

moving to 4.0 RTM for visibility

Contributor

rohit21agrawal commented Jan 20, 2017

moving to 4.0 RTM for visibility

@rrelyea rrelyea modified the milestones: 4.0.1, 4.0 RTM Jan 24, 2017

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Jan 24, 2017

Contributor

likely by design, but will take more time to talk it through post-rtm. easy workaround in initial post.

Contributor

rrelyea commented Jan 24, 2017

likely by design, but will take more time to talk it through post-rtm. easy workaround in initial post.

@linkdata

This comment has been minimized.

Show comment
Hide comment
@linkdata

linkdata Feb 21, 2017

If it is by design, I'd very much like to hear the reasoning. But I think it's just a bug.

If I have a project dependency, when packing that project the dependent project needs to packed first, and the actual version thus packaged must be depended upon.

linkdata commented Feb 21, 2017

If it is by design, I'd very much like to hear the reasoning. But I think it's just a bug.

If I have a project dependency, when packing that project the dependent project needs to packed first, and the actual version thus packaged must be depended upon.

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster Mar 9, 2017

More users hitting this: dotnet/cli#5971

natemcmaster commented Mar 9, 2017

More users hitting this: dotnet/cli#5971

@ericwj

This comment has been minimized.

Show comment
Hide comment
@ericwj

ericwj Mar 15, 2017

Its very weird that the VersionSuffix is only respected during pack if it's on disk.

I'm imagining it'd be by design to have build and pack basically concluding the same -- even if you also specify dotnet pack --no-build ... as would likely be preferred on say a build server whenever the build has already ran.

Seeing e.g. aspnet/Logging using a Version.props and Common.props works as long as Version.props is rewritten. The VersionSuffix Condition= they use is basically useless - not for build (or restore, apparently), but just for pack...

ericwj commented Mar 15, 2017

Its very weird that the VersionSuffix is only respected during pack if it's on disk.

I'm imagining it'd be by design to have build and pack basically concluding the same -- even if you also specify dotnet pack --no-build ... as would likely be preferred on say a build server whenever the build has already ran.

Seeing e.g. aspnet/Logging using a Version.props and Common.props works as long as Version.props is rewritten. The VersionSuffix Condition= they use is basically useless - not for build (or restore, apparently), but just for pack...

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Mar 21, 2017

We're hitting this here:
https://dotnet.myget.org/feed/rx/package/nuget/System.Reactive.Core

We set the Version at build time using GitVersionTask. This is causing bad refs in the packages.

We're using msbuild /t:pack since GitVersionTask calcs the version. But something is missing the dependencies versions:

https://github.com/Reactive-Extensions/Rx.NET/blob/5ab4dcab616dd43b9dc938e5b44aeeec22f311c4/Rx.NET/Source/build-new.ps1#L55-L68

onovotny commented Mar 21, 2017

We're hitting this here:
https://dotnet.myget.org/feed/rx/package/nuget/System.Reactive.Core

We set the Version at build time using GitVersionTask. This is causing bad refs in the packages.

We're using msbuild /t:pack since GitVersionTask calcs the version. But something is missing the dependencies versions:

https://github.com/Reactive-Extensions/Rx.NET/blob/5ab4dcab616dd43b9dc938e5b44aeeec22f311c4/Rx.NET/Source/build-new.ps1#L55-L68

@NickCraver

This comment has been minimized.

Show comment
Hide comment
@NickCraver

NickCraver Mar 21, 2017

I just hit this with MiniProfiler, since I only have an alpha, people are unable to restore their packages. As an example: what should work is a simple reference in MiniProfiler.AspNetCore, which has a <ProjectReference> to MiniProfiler.Shared.

Though the build passed --version-suffix="alpha23" to both of these packages, you can see the MiniProfiler.AspNetCore package depends on MiniProfiler.Shared 4.0.0 (which doesn't exist), rather than the 4.0.0-alpha23 it's supposed to.

This is a fairly big bug, as it breaks all pre-releases with any dependencies. And this is a time period with a lot of pre-releases for core.

You can repro this easily by cloning the MiniProfiler repo at this commit and running .\build.ps1 -VersionSuffix "ahlpa23" in the root. Packages are output in .\nupkgs.

NickCraver commented Mar 21, 2017

I just hit this with MiniProfiler, since I only have an alpha, people are unable to restore their packages. As an example: what should work is a simple reference in MiniProfiler.AspNetCore, which has a <ProjectReference> to MiniProfiler.Shared.

Though the build passed --version-suffix="alpha23" to both of these packages, you can see the MiniProfiler.AspNetCore package depends on MiniProfiler.Shared 4.0.0 (which doesn't exist), rather than the 4.0.0-alpha23 it's supposed to.

This is a fairly big bug, as it breaks all pre-releases with any dependencies. And this is a time period with a lot of pre-releases for core.

You can repro this easily by cloning the MiniProfiler repo at this commit and running .\build.ps1 -VersionSuffix "ahlpa23" in the root. Packages are output in .\nupkgs.

@NickCraver NickCraver referenced this issue Mar 21, 2017

Closed

MiniProfiler v4 #144

41 of 43 tasks complete

NickCraver added a commit to MiniProfiler/dotnet that referenced this issue Mar 21, 2017

Change build to use MSBuild
See NuGet/Home#4337 for details,
--version-suffix for dotnet pack basically doesn't work for
pre-releases.
@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Mar 21, 2017

Contributor

@onovotny @NickCraver why can't you pass /p:VersionSuffix when you do a restore?

arguments to build and restore should be similar as far as possible.

CC: @emgarten

Contributor

rohit21agrawal commented Mar 21, 2017

@onovotny @NickCraver why can't you pass /p:VersionSuffix when you do a restore?

arguments to build and restore should be similar as far as possible.

CC: @emgarten

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Mar 21, 2017

I don't use VersionSuffix I set Version during the build step. Is it documented anywhere that Version needs to be calculated during a Restore? Is there a Target we can hook before into to make this happen automatically?

onovotny commented Mar 21, 2017

I don't use VersionSuffix I set Version during the build step. Is it documented anywhere that Version needs to be calculated during a Restore? Is there a Target we can hook before into to make this happen automatically?

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Mar 21, 2017

/cc @AArnott for NB.GV too.

onovotny commented Mar 21, 2017

/cc @AArnott for NB.GV too.

@NickCraver

This comment has been minimized.

Show comment
Hide comment
@NickCraver

NickCraver Mar 21, 2017

@rohit21agrawal I wouldn't expect that I need to - since there's no --version-suffix on dotnet restore, that's a very strong indicator that it's not needed in any way.

NickCraver commented Mar 21, 2017

@rohit21agrawal I wouldn't expect that I need to - since there's no --version-suffix on dotnet restore, that's a very strong indicator that it's not needed in any way.

@AArnott

This comment has been minimized.

Show comment
Hide comment
@AArnott

AArnott Mar 21, 2017

Contributor

This sounds super similar (although the repro steps are wildly different) to the underlying problem behind #4790. I suggest we keep both in mind when developing a fix.

Contributor

AArnott commented Mar 21, 2017

This sounds super similar (although the repro steps are wildly different) to the underlying problem behind #4790. I suggest we keep both in mind when developing a fix.

asimmon added a commit to asimmon/Pillar that referenced this issue Jul 11, 2017

appveyor: tweaked "dotnet pack" command with "/t:Restore;Pack" tasks …
…so the resulting nuspec file will include the version suffix in projectreference dependencies (NuGet/Home#4337)

asimmon added a commit to asimmon/Pillar that referenced this issue Jul 11, 2017

appveyor: tweaked "dotnet pack" command with "/t:Restore;Pack" tasks …
…so the resulting nuspec file will include the version suffix in projectreference dependencies (NuGet/Home#4337)

qmfrederik added a commit to qmfrederik/remoteviewing that referenced this issue Aug 14, 2017

qmfrederik added a commit to qmfrederik/remoteviewing that referenced this issue Aug 14, 2017

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Aug 28, 2017

Bump? What is the status of getting this resolved. We have to double-restore currently to workaround this.

onovotny commented Aug 28, 2017

Bump? What is the status of getting this resolved. We have to double-restore currently to workaround this.

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Aug 28, 2017

It would be great if this could be resolved in a way that also fixes #4790 (allow build logic acquired via nuget to set the version - like GitVersion - which is impossible during restore), which is in the 4.4 milestone.

dasMulli commented Aug 28, 2017

It would be great if this could be resolved in a way that also fixes #4790 (allow build logic acquired via nuget to set the version - like GitVersion - which is impossible during restore), which is in the 4.4 milestone.

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Aug 28, 2017

Contributor

with SDK 2.0, a restore is automatically implied on build (and pack builds by default, unless you specify the nobuild option), so you shouldn't have to do a double restore.

Contributor

rohit21agrawal commented Aug 28, 2017

with SDK 2.0, a restore is automatically implied on build (and pack builds by default, unless you specify the nobuild option), so you shouldn't have to do a double restore.

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Aug 28, 2017

This doesn't help since I need versions to calculate for interpackage dependencies. Also, I need to use msbuild /t:restore and msbuild /t:build for things that dotnet cannot build. This is also related to #4790

onovotny commented Aug 28, 2017

This doesn't help since I need versions to calculate for interpackage dependencies. Also, I need to use msbuild /t:restore and msbuild /t:build for things that dotnet cannot build. This is also related to #4790

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Aug 28, 2017

Calling a target per p2p to get $(PackageId) and $(PackageVersion) would be great although it might slow down projects using the generate-package-on-bulid functionality a little.

The integrated restore is great and solves a lot of these edge case problems yet this does not solve the imported targets/props issue (e.g. distributing this target in a NuGet package) and isn't very obvious when directly using other build systems (mono's msbuild, cake to invoke (dotnet/mono) msbuild, …)

btw There also are instances when users want to change their package IDs (.Signed versions) - Microsoft/msbuild#2463

dasMulli commented Aug 28, 2017

Calling a target per p2p to get $(PackageId) and $(PackageVersion) would be great although it might slow down projects using the generate-package-on-bulid functionality a little.

The integrated restore is great and solves a lot of these edge case problems yet this does not solve the imported targets/props issue (e.g. distributing this target in a NuGet package) and isn't very obvious when directly using other build systems (mono's msbuild, cake to invoke (dotnet/mono) msbuild, …)

btw There also are instances when users want to change their package IDs (.Signed versions) - Microsoft/msbuild#2463

ryangribble added a commit to TattsGroup/octokit.net that referenced this issue Sep 3, 2017

ryangribble added a commit to octokit/octokit.net that referenced this issue Sep 3, 2017

Fix assembly versioning/properties and handle platform exception (#1660)
* Use assembly version instead of hard-coded ones

Builds happening on AppVeyor specify the assembly version with `dotnet build /p:Version=<gitversion>`

* Set default version for dev time

This is to avoid that the default 1.0.0 version be assigned to the assemblies

* Move various package/assembly properties from AssemblyInfo into csproj files so dotnet build can set them all

* Get rid of SolutionInfo and move assembly version function into Connection class

* Rework FormatUserAgent to use InformationalVersion and guard against exceptions determining platform OS/arch (fixes #1617)

* Update assembly descriptions

* Reword dotnetcore to .NET Core

* Attempted workaround for package version dependency issue by specifying version on dotnet restore
see NuGet/Home#4337
@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Sep 6, 2017

Contributor

@natemcmaster @onovotny @NickCraver others - if anyone wants to try out private bits to validate this fix, please let me know.

Contributor

rohit21agrawal commented Sep 6, 2017

@natemcmaster @onovotny @NickCraver others - if anyone wants to try out private bits to validate this fix, please let me know.

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Sep 6, 2017

I am interested, but what does the fix do, how does it work? Looking at the PR, adding VersionSuffix alone doesn't seem like it'd fix it. I don't set/directly use VersionSuffix at all, I use Version/PackageVersion and that's calculated on build. It needs to be calculated/used on pack. It's not a matter of using a different variable.

onovotny commented Sep 6, 2017

I am interested, but what does the fix do, how does it work? Looking at the PR, adding VersionSuffix alone doesn't seem like it'd fix it. I don't set/directly use VersionSuffix at all, I use Version/PackageVersion and that's calculated on build. It needs to be calculated/used on pack. It's not a matter of using a different variable.

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Sep 6, 2017

Contributor

these test cases should explain the behavior: https://github.com/NuGet/NuGet.Client/pull/1688/files#diff-0a8ea4346824f4b1f738f32446ce80caR412

but if not, let me know and i'll explain the behavior.

Contributor

rohit21agrawal commented Sep 6, 2017

these test cases should explain the behavior: https://github.com/NuGet/NuGet.Client/pull/1688/files#diff-0a8ea4346824f4b1f738f32446ce80caR412

but if not, let me know and i'll explain the behavior.

@onovotny

This comment has been minimized.

Show comment
Hide comment
@onovotny

onovotny Sep 6, 2017

As mentioned in the PR thread, this does not solve the issue. Pack cannot read from the assets file since the assets file is written on restore. This behavior is forcing an extra restore if the version is calculated during a build step.

The pack targets need to do the same calculation on p2p refs.

onovotny commented Sep 6, 2017

As mentioned in the PR thread, this does not solve the issue. Pack cannot read from the assets file since the assets file is written on restore. This behavior is forcing an extra restore if the version is calculated during a build step.

The pack targets need to do the same calculation on p2p refs.

eerhardt added a commit to eerhardt/core-setup that referenced this issue Sep 25, 2017

Fix VersionSuffix for the managed projects
VersionSuffix is getting set before $(BuildNumberMajor) and $(BuildNumberMinor) are being set.  When creating the DependencyModel nupkg, it is getting a bad version on its p2p reference to PlatformAbstractions.

The fix is to ensure VersionSuffix is defined correctly in the projects themselves - after the obj\BuildVersion.props file is created.

Workaround NuGet/Home#4337

@rohit21agrawal rohit21agrawal modified the milestones: Backlog, 4.,6, 4.6 Jan 5, 2018

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jan 6, 2018

Contributor

FYI this fix is going to be release in Update 6 of Visual Studio aka 15.6

Contributor

rohit21agrawal commented Jan 6, 2018

FYI this fix is going to be release in Update 6 of Visual Studio aka 15.6

@dazhao-msft

This comment has been minimized.

Show comment
Hide comment
@dazhao-msft

dazhao-msft Jan 25, 2018

@rohit21agrawal, I assume the fix is actually in .NET Core SDK and will be integrated in VS 15.6. Is that correct? We're using .NET Core SDK to build in CI.

dazhao-msft commented Jan 25, 2018

@rohit21agrawal, I assume the fix is actually in .NET Core SDK and will be integrated in VS 15.6. Is that correct? We're using .NET Core SDK to build in CI.

@rohit21agrawal

This comment has been minimized.

Show comment
Hide comment
@rohit21agrawal

rohit21agrawal Jan 25, 2018

Contributor

@dazhao-msft that is correct. the nightly builds of .NET Core SDK have this fix.

Contributor

rohit21agrawal commented Jan 25, 2018

@dazhao-msft that is correct. the nightly builds of .NET Core SDK have this fix.

NickCraver added a commit to NickCraver/StackExchange.Exceptional that referenced this issue Feb 17, 2018

Attempt to fix versioning dependencies from NuGet/Home #4337
See NuGet/Home#4337 for the issue. Kudos to @onovotny for an assist here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment