TAGC / dotnet-setversion

.NET Core CLI tool to update the version information in .NET Core *.csproj files
MIT License
75 stars 16 forks source link

Add support for the VSTS dotnet build task #5

Closed roryprimrose closed 6 years ago

roryprimrose commented 6 years ago

Thanks for writing this CLI. I've been trying to find a good solution to this for a very long time.

The VSTS dotnet build task (v2 preview) supports a custom command that could be used to execute setversion.

image

The issue is that the setversion CLI looks for a csproj in the current directory which does not support the way that VSTS execute dotnet in a build.

The VSTS dotnet build task will identify one or more projects using the glob */.csproj. It then passes this as the second parameter to the dotnet CLI. For example:

"C:\Program Files\dotnet\dotnet.exe" setversion C:\B\_work\4\s\MySolution\MyProject.csproj 0.1.0-hostfixes0005

The only way I've been able to dotnet-setversion to work with the current implementation is to use a command line step that executes with the current directory set to the project folder and then call

dotnet setversion 0.1.0-hostfixes0005

The main disadvantage here is that this would require build multiple steps and hard-coded references to project folders to support multiple projects. The build definition then needs to be updated to add more build steps as projects are added to the solution.

One way to solve this would be to identify an absolute path to a csproj in one of the command line parameters and then find the version from one of the other parameters. If no csproj path is specified then the fallback would use what the current implementation does.

TAGC commented 6 years ago

Hey @roryprimrose.

I started work on this request but I just remembered - you necessarily have to have the project directory as the current working directory in order to call "dotnet setversion". This is the case with all dotnet CLI tools. If you try to call from another directory, you get No executable found matching command "dotnet-setversion".

TAGC commented 6 years ago

Actually, just did some reading and it seems like it'd be possible using the path-based extensibility model. I'll look into it.

roryprimrose commented 6 years ago

This might not need to be fixed in your package. All the dotnet CLI verbs require this as well. Weird that they OOTB tasks work on a VSTS build.

I did just find this in the VSTS build tasks repo - https://github.com/Microsoft/vsts-tasks/pull/5977

roryprimrose commented 6 years ago

Hmm, to clarify, this would mean that you wouldn't need to figure out the current working directory for the CLI to find your custom setversion verb. You would still need to modify the code to be adaptive to whether a proj file path was one of the arguments.

TAGC commented 6 years ago

You would still need to modify the code to be adaptive to whether a proj file path was one of the arguments.

Is something like dotnet setversion -f /path/to/your.csproj 0.1.2-beta0003 okay? That's what I've implemented at the moment.

roryprimrose commented 6 years ago

The only issue with this is that the VSTS dotnet build task does not include the -f qualifier. Does your change support the following format?

dotnet setversion absolute/path/to/your.csproj 0.1.2-beta0003

That is how the build task will call it.

TAGC commented 6 years ago

VSTS build tasks are that inflexible? There's no other way to get them to pass the csproj file to the command?

If so, I guess I can make it so it checks if the first argument ends in *.csproj and if so, treats that as the csproj path and expects an argument after that for the version string.

roryprimrose commented 6 years ago

Well it's more that it's just how they developed that task to invoke dotnet. I would probably use File.Exists on the parameter to be sure otherwise you are restricting to just csproj which won't support other project types.


From: David notifications@github.com Sent: Saturday, December 30, 2017 3:42:01 AM To: TAGC/dotnet-setversion Cc: Rory Primrose; Mention Subject: Re: [TAGC/dotnet-setversion] Add support for the VSTS dotnet build task (#5)

VSTS build tasks are that inflexible? There's no other way to get them to pass the csproj file to the command?

If so, I guess I can make it so it checks if the first argument ends in *.csproj and if so, treats that as the csproj path and expects an argument after that for the version string.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/TAGC/dotnet-setversion/issues/5#issuecomment-354469320, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AB8xg87PHTlwnN7u503RUJXmxY8fwOLLks5tFRZZgaJpZM4RMcjX.

TAGC commented 6 years ago

you are restricting to just csproj which won't support other project types.

What other project types do you have in mind? dotnet-setversion is designed specifically for projects following the new csproj style (with version information stored in XML under <Project><PropertyGroup><Version>.

I would probably use File.Exists on the parameter to be sure

The problem is that the changes you're suggesting would already technically be considered a breaking change if we went the way I suggest by checking if the first argument ends in *.csproj. Currently if someone were to use dotnet-setversion to set a version that for whatever reason ended in ".csproj", it would break. But that's a very, very unlikely situation and I think we can safely ignore that possibility.

OTOH, if I use File.Exists on the argument, then this'd cause unexpected breaking behaviour if they happened to have a file with the same name as the version they were attempting to set. For example, if someone tries to use dotnet-setversion 1.0.2 right now, then that'll set the version of the csproj to 1.0.2 as expected. OTOH, if they actually did have some file called 1.0.2 and attempted to run that command, it would treat the first argument as the file to store the updated version within and look for a second argument as the actual version to set.

roryprimrose commented 6 years ago

We probably need to read up on this more but I suspect the version element is an msbuild one, not one that is restricted to csproj.

@davkean is that correct?


From: David notifications@github.com Sent: Sunday, December 31, 2017 2:48:54 AM To: TAGC/dotnet-setversion Cc: Rory Primrose; Mention Subject: Re: [TAGC/dotnet-setversion] Add support for the VSTS dotnet build task (#5)

you are restricting to just csproj which won't support other project types.

What other project types do you have in mind? dotnet-setversion is designed specifically for projects following the new csproj style (with version information stored in XML under .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/TAGC/dotnet-setversion/issues/5#issuecomment-354552858, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AB8xg2zETH_D0R1qen4eDo8sinTWWKYQks5tFltmgaJpZM4RMcjX.

TAGC commented 6 years ago

Looks like that guy isn't going to respond. Still need this implemented?

roryprimrose commented 6 years ago

I'm not overly bothered because I'm only using this on csproj files :) Let me know if there is a beta package I can test for you.

roryprimrose commented 6 years ago

I've had a look at your pull request and it looks like supporting how the VSTS/TFS dotnet build task invokes dotnet.exe will still be an issue. From what I can see in the code, the use of the command line parsing library would require two positional arguments where the first argument is the csproj path and the second is the version. This breaks the prior implementation where the first argument was the version.

Could you use that library with positional arguments but then determine which is the csproj from those arguments rather than assuming that the position is significant? I think that would be the only way you could get this to work without a breaking change.

TAGC commented 6 years ago

The stuff in the pull request is not relevant anymore, just something I threw together trying to pre-empt the implementation.

How about using a flag to differentiate between the two possible modes of operation? Maybe like dotnet <csproj path> <version> --override-csproj or dotnet <csproj path> <version> --vsts?

This will avoid any breaking changes while also involving potentially hairy issues/ambiguities that could arise from trying to figure out if a parameter represents a valid path or not. It also looks like you can just append that modifier flag (--override-csproj, --vsts or whatever we decide on) to your VSTS Argument field.

roryprimrose commented 6 years ago

Adding --vsts as an additional argument on the build task should work fine. Let's go with that and see how it works.

TAGC commented 6 years ago

I've given this a shot. Try installing dotnet-setversion 1.0.3-issue-5-0008. I guess you need to do something similar to this to test it out.

roryprimrose commented 6 years ago

Where is this package hosted? It's not on nuget or myget.

TAGC commented 6 years ago

It's on NuGet but unlisted: https://www.nuget.org/packages/dotnet-setversion/1.0.3-issue-5-0008

I think you should still be able to install unlisted packages though if you specify the exact version.

roryprimrose commented 6 years ago

This doesn't work yet because the task still doesn't get resolved correctly by the dotnet custom beta build step. I've raised an issue at https://github.com/Microsoft/vsts-tasks/issues/6249

TAGC commented 6 years ago

Yeah I'm not sure if you can do that unless you add the actual dotnet-setversion executable to a directory under $PATH, like /usr/local/bin on *nix or C:\Program Files\ on Windows. I'm basing that off of this.

roryprimrose commented 6 years ago

I think this is where I'm at. My scenario falls in a gap here. The setversion CLI is developed to be a project specific CLI. The dotnet VSTS build step is designed to execute globally installed CLI packages. Does that sound right?

TAGC commented 6 years ago

Yeah but these changes make it so that the setversion CLI can be used globally. I think it's just a matter of finding a way to install it globally during VSTS builds, if that's possible.

TAGC commented 6 years ago

Going to close this for housekeeping. Let me know if you want it re-opened. I think someone else is working on a feature branch that might turn this into a global tool anyway.