Skip to content
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

fix(git-generic) Ensure that git add works with absolute file paths #368

Merged
merged 2 commits into from
Nov 13, 2021

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Nov 13, 2021

Fix #366

The PRs #313 and #363 reworked the resources file and yaml with the side effect of switching the list of file path returned by their TargetFromScm() function to absolute paths, leading to error entry not found when git-adding the files in a target with scm case.

This PR fixes this behavior by treating the file path directly into the git generic plugin: if the file path that should be git added are absolute, then updatecli tries to convert these paths as relative to the scm's working directory (or raise an error otherwise).

Test

To test this pull request, you can run the following commands:

git checkout https://github.com/jenkins-infra/pipeline-library -b c5c4b22
cd pipeline-library
updatecli-next --debug apply --values ./updatecli/values.yaml --config ./updatecli/updatecli.d/docker-builder.yml

=> Tested successfully on my fork: https://github.com/dduportal/pipeline-library/pull/10

Additional Information

Tradeoff

The resources file or yaml could have been updated to fix the file path, but fixing it on the "central" git seemed safer: it removes a constraint on the plugins code for any contributor. WDYT @olblak ?

Potential improvement

The resources file or yaml could be updated to use relative path in complement of the fix.

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal dduportal marked this pull request as ready for review November 13, 2021 09:41
@dduportal dduportal added bug Something isn't working scm-git SCM of kind "Git" target Related to updatecli target labels Nov 13, 2021
@dduportal dduportal added this to the 0.14.1 milestone Nov 13, 2021
@olblak olblak enabled auto-merge (squash) November 13, 2021 11:20
@olblak olblak removed this from the 0.14.1 milestone Nov 13, 2021
@olblak olblak merged commit c9b06f4 into updatecli:main Nov 13, 2021
@dduportal dduportal deleted the fix/file-target-scm branch November 17, 2021 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working scm-git SCM of kind "Git" target Related to updatecli target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[file] error when applying a target with SCM
2 participants