-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add support for the VSTS dotnet build task #5
Comments
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 |
Actually, just did some reading and it seems like it'd be possible using the path-based extensibility model. I'll look into it. |
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 - microsoft/azure-pipelines-tasks#5977 |
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. |
Is something like |
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?
That is how the build task will call it. |
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. |
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 GitHub<#5 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AB8xg87PHTlwnN7u503RUJXmxY8fwOLLks5tFRZZgaJpZM4RMcjX>.
|
What other project types do you have in mind?
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 |
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 <Project><PropertyGroup><Version>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#5 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AB8xg2zETH_D0R1qen4eDo8sinTWWKYQks5tFltmgaJpZM4RMcjX>.
|
Looks like that guy isn't going to respond. Still need this implemented? |
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. |
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. |
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 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 ( |
Adding |
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. |
Where is this package hosted? It's not on nuget or myget. |
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. |
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 microsoft/azure-pipelines-tasks#6249 |
Yeah I'm not sure if you can do that unless you add the actual |
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? |
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. |
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. |
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.
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:
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
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.
The text was updated successfully, but these errors were encountered: