RamblingCookieMonster / BuildHelpers

Helper functions for PowerShell CI/CD scenarios
MIT License
214 stars 47 forks source link

Get-BuildVariable: inconsistent commit message detection #109

Closed peppekerstens closed 4 years ago

peppekerstens commented 5 years ago

Build environment Azure Devops

Tested PS5.1/PS6core build stage

Issue the commit message detection code (https://github.com/RamblingCookieMonster/BuildHelpers/blob/303eebee0bf92f9e086f131d89cf478a6ff87275/BuildHelpers/Public/Get-BuildVariable.ps1#L154) fails on detecting 'BUILD_SOURCEVERSIONMESSAGE'

root cause After testing, the root cause seems to be two-fold: Cause 1:

  1. 'SYSTEM_DEFAULTWORKINGDIRECTORY' exists in both the BUILD and RELEASE stages of Azure DevOps. In both cases the default value are the 'artifacts' directories of the used agents (d:\a\r1\a when using hosted agents). So they are directory paths(!)
  2. Somehow, PS does not obey the order of the 'switch' command anymore; it 'hits' on 'SYSTEM_DEFAULTWORKINGDIRECTORY' instead of 'BUILD_SOURCEVERSIONMESSAGE'. This is a PS 'thing' and beyond the scope of this issue.

Cause 2: The detection does not obey the provided path parameter. I will create separate issue (https://github.com/RamblingCookieMonster/BuildHelpers/issues/110) for this.

solution The simplest solution is to use 'AGENT_RELEASEDIRECTORY' instead of 'SYSTEM_DEFAULTWORKINGDIRECTORY'.

FISHMANPET commented 5 years ago

This one is a little more interesting. I don't have a release pipeline or a hosted agent to play around with. I understand the problem in #110 and how that's manifesting here as well where we're grabbing the wrong directory regardeless. I'll take a look at this as well and at the very least use AGENT_RELEASEDIRECTORY instead of SYSTEM_DEFAULTWORKINGDIRECTORY.

Do you think specifying the Path to Set-BuildEnvironment is having impact here, or should have any impact?

FISHMANPET commented 5 years ago

So looking at a MS hosted YAML pipeline, I don't have AGENT_RELEASEDIRECTORY in the environment, so that's not a universal directory, but as I said in the other issue that's supposed to be a commit hash not a directory anyway. Also looking at my pipeline, SYSTEM_DEFAULTWORKINGDIRECTORY is D:\a\1\s and I have BUILD_ARTIFACTSTAGINGDIRECTORY, BUILD_STAGINGDIRECTORY, and SYSTEM_ARTIFACTSDIRECTORY, all of which point to D:\a\1\a. So maybe that's a difference between a YAML and a classic pipeline?

So I think what I described in the other issue will take care of Part 1 of your Case 1. Part 2 is a little trickier. I think what's happening isn't that order is being obeyed, but when you pass a collection into a switch statement it checks every item in order. So If you have both SYSTEM_DEFAULTWORKINGDIRECTORY and BUILD_SOURCEVERSIONMESSAGE in environment, what happens is first the switch catches on BUILD_SOURCEVERSIONMESSAGE and properly finds the message, then it catches again on SYSTEM_DEFAULTWORKINGDIRECTORY and since that's bugged it fails.

I may have to reevaluate what I'm doing here because I hadn't though hard enough about it, but using switch like this only works when exactly 1 of the environment variables in the switch statement is in your environment. Using 2 that may be in the environment can make things weird and non-determinant, though i'll have to do some testing in my branch and my test pipeline to verify that behavior.

Certainly it makes me thinks there should be some more tests written to try and catch some of these edge cases as well

peppekerstens commented 5 years ago

Currently, within the Azure DevOps interface, there a two pipelines; 'build' and 'release'.

I am able to use YAML for the 'build' section of the pipeline. Documentation hints towards the fact that you can also use YAML for release. I however, have no direct interface to create/use a YAML file for the release section of Azure Devops.

Looking at your remarks, the use of YAML and returned environment var's, I tend to assume that you are testing/running things in the 'build' stage/environment of Azure DevOps.

Create a dummy release, request the env: you'll notice that there are other variables populated in the env:

Like AGENTRELEASEDIRECTORY. Everything with BUILD is absend... :)

Looking at the documentation there is a hint that this is 'classic' functionality.

Looking at this, the 'modern way' seems to be (1) using only build and (2) using YAML. Which is (also) covered/solved by current code.

So it is not a mistake; it is on purpose. Using AGENT_RELEASEDIRECTORY makes it unique for release. There is no variable available for the Git hash so we do need that path again... :)