Skip to content

Removing 'path' parameter from 'git log' command - #118

Merged
RamblingCookieMonster merged 2 commits into
PowerShellOrg:masterfrom
raandree:master
Sep 19, 2019
Merged

Removing 'path' parameter from 'git log' command#118
RamblingCookieMonster merged 2 commits into
PowerShellOrg:masterfrom
raandree:master

Conversation

@raandree

@raandree raandree commented Sep 16, 2019

Copy link
Copy Markdown
Contributor

This fixes #109 and #110.

As far as I have understood the git documentation, the path parameter for 'git log' is more like a filter to get the last commit for a specific directory or file. So here the path is it used twice and the two values may contradict each other . The current path must be set to somewhere in the git repo anyway and then - if things are as expected - the path (SYSTEM_DEFAULTWORKINGDIRECTORY) shows to the same repo again. In my case, I am having more than one artifact to deal with and the path in SYSTEM_DEFAULTWORKINGDIRECTORY does not point to the repo. The repo is in a separate folder in SYSTEM_DEFAULTWORKINGDIRECTORY.

And if the commit message could not be retrieved, we get in in line 224 without the path.

I don't see any value in defining the path as it is done for the 'git log' calls. Please correct me if I am wrong.

if($WeCanGit)
{
Invoke-Git @IGParams -Arguments "log --format=%B -n 1 $( (Get-Item -Path "ENV:$_").Value )"
Invoke-Git @IGParams -Arguments "log --format=%B -n 1"

@RamblingCookieMonster RamblingCookieMonster Sep 17, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So! In this section (getting commit message), the deleted bit is the commit sha. If you leave this off, AFAICT it will get the current commit. Potential corner case: Someone has been moving around in their build (checking out a different commit), but still want to refer to the commit message of the build's commit.

Do these lines ever error out for you? Maybe I'm grabbing a wrong environment variable? Intention in this section is to grab commit to do this: https://stackoverflow.com/a/3357357

if($WeCanGit)
{
(Invoke-Git @IGParams -Arguments "log --format=%B -n 1 $( (Get-Item -Path "ENV:$_").Value )").split([Environment]::NewLine,[System.StringSplitOptions]::RemoveEmptyEntries) -join " "
(Invoke-Git @IGParams -Arguments "log --format=%B -n 1").split([Environment]::NewLine,[System.StringSplitOptions]::RemoveEmptyEntries) -join " "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this one is weird. It should be a commit sha, not a path, not sure what happened.

@RamblingCookieMonster

Copy link
Copy Markdown
Collaborator

Okay, added comments! Long story short, intention is to use an environment variable that indicates the current build's commit sha, and to get the commit message associated with that commit via this method.

As mentioned, you could imagine a hypothetical, perhaps ill-advised scenario where someone move around in their repo (to a different commit), and still wants to refer to the current commit message. Removing the git commit sha from the command in that case will AFAIK get the commit message for what they have checked out, rather than for the commit he build is running against

Let me double check, but yeah, we should totally scrap SYSTEM_DEFAULTWORKINGDIRECTORY. Is there a commit sha environment variable we can point to instead of that?

@raandree

Copy link
Copy Markdown
Contributor Author

Ok, got the point of that. But what happens using the path is actually this:

Like you do in your Init task, my build process sets the location to $ProjectRoot. Then getting the log with the path as an argument fails like that.

PS C:\BuildWorkerSetupFiles\_work\r2\a\CommonTasks CI\SourcesDirectory> git branch
* (HEAD detached at 417779a)
PS C:\BuildWorkerSetupFiles\_work\r2\a\CommonTasks CI\SourcesDirectory> git log -n 1 C:\BuildWorkerSetupFiles\_work\r2\a

fatal: C:\BuildWorkerSetupFiles\_work\r2\a: 'C:\BuildWorkerSetupFiles\_work\r2\a' is outside repository

If we use the Git Commit Id, things are much better:

PS C:\BuildWorkerSetupFiles\_work\r2\a\CommonTasks CI\SourcesDirectory> git log -n 1 417779a652535f229430e6289a229b85fba
f6b5f
commit 417779a652535f229430e6289a229b85fbaf6b5f (HEAD, origin/dev)
Author: Install <Install@contoso.com>
Date:   Tue Sep 17 09:50:20 2019 +0530

    Test Commit 2

For Azure Pipelines, according to Release variables and debugging there are two variables storing the Git Commit Id:

  • Build.SourceVersion
  • Release.Artifacts.{Primary artifact alias}.SourceVersion

In AppVoyer the variable is the varialble 'APPVEYOR_REPO_COMMIT' but one can get the commit message from 'APPVEYOR_REPO_COMMIT_MESSAGE'.

I am not sure about all the other build systems that you support.

@RamblingCookieMonster

Copy link
Copy Markdown
Collaborator

Perfect! So! The only case where that would be a path in the code is for SYSTEM_DEFAULTWORKINGDIRECTORY.

If we leave $( (Get-Item -Path "ENV:$_").Value ) in each spot, and change SYSTEM_DEFAULTWORKINGDIRECTORY to Build_SourceVersion (it appears underscores are used in place of periods in environment variable names), then all of those cases should evaluate to variables that store a commit hash, rather than a path.

@raandree

Copy link
Copy Markdown
Contributor Author

OK, changed the code to the previous state and replaced the variable 'SYSTEM_DEFAULTWORKINGDIRECTORY' with 'BUILD_SOURCEVERSION'.

@RamblingCookieMonster

Copy link
Copy Markdown
Collaborator

Works for me, merging, thanks, and shooot forgot the conf started, sorry!

@RamblingCookieMonster
RamblingCookieMonster merged commit e7c4fc6 into PowerShellOrg:master Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get-BuildVariable: inconsistent commit message detection

2 participants