stan-dev / jenkins-shared-libraries

Libraries for our Jenkinsfiles
1 stars 1 forks source link

VerifyChanges fails when user's fork has different name #7

Closed WardBrian closed 10 months ago

WardBrian commented 10 months ago

See https://github.com/stan-dev/math/pull/2979#issuecomment-1840959152

@serban-nicusor-toptal - how difficult is this to support?

serban-nicusor-toptal commented 10 months ago

Hey @WardBrian I hope to be just a slight change, I have a few hours free later and I'll sort all these out.

serban-nicusor-toptal commented 10 months ago

Looks like the behavior of CHANGE_FORK is a bit wobbly https://issues.jenkins.io/browse/JENKINS-58450 Take a look at the errored line:

fatal: repository 'https://github.com/chvandorp/stan_math/math/' not found

and that respective line in the code

git remote add forkedOrigin https://${GIT_USERNAME}:${GIT_PASSWORD}@github.com/${env.CHANGE_FORK}/${currentRepository}

This leads me to believe that CHANGE_FORK comes now with the user and the fork name, thus I just removed the current repository that we extract and use after it, in theory, this should fix this edge case. If it doesn't (it still returns different if the fork name differs from the original), I might have to come back to it and put a conditional, figure it out and set the value accordingly. They talk about a CHANGE_FORK_FULL variable but that seems to not be present so I hope this got patched -> https://jenkins.flatironinstitute.org/pipeline-syntax/globals

If this will be the case, I also found a possible implementation that could accommodate for the different values of this variable https://github.com/intel/pmem-csi/blob/devel/Jenkinsfile#L418

            forkedRepository = env.CHANGE_FORK.matches('.*/.*') ?
                    env.CHANGE_FORK :
                    env.CHANGE_FORK + "/${currentRepository}"

Edit: Added the above conditional in our code so we're safe for the long run.