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

git.latest: add auth to merge/reset calls when LFS used with SSH auth #51319

Merged
merged 4 commits into from Jan 28, 2019

Conversation

@terminalmage
Copy link
Contributor

commented Jan 24, 2019

In these cases, if the commands are not auth'ed then the git LFS subcommands don't know which identity file to use and will fall back to the one from ~/.ssh/config. By ensuring that the command is auth'ed, the LFS subcommands are able to download files when needed.

Normally we don't need to auth to the remote repo for this, since we have made sure that we already have the needed objects downloaded, and that is why these commands were not auth'ed before.

Also, this fixes a bunch of external links that were broken. When I initially added those external links they were correct, but included a URL-encoded space (i.e. %20). In the intervening weeks/months/etc. the URL-encoded space seems to have been replaced by an underscore, so the links in our docs were out-of-date.

Resolves #51255.

In these cases, if the commands are not auth'ed then the git LFS
subcommands don't know which identity file to use and will fall back to
the one from `~/.ssh/config`. By ensuring that the command is auth'ed,
the LFS subcommands are able to download files when needed.

Normally we don't need to auth to the remote repo for this, since we
have made sure that we already have the needed objects downloaded, and
that is why these commands were not auth'ed before.
@s0undt3ch

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

Test failures seem related to the changes.

If the repo has not yet been cloned then this will result in an error.
And since we are checking the global gitconfig here, there's no need to
run it from the target dir.
@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2019

Should be fixed now. I had added a check in the global gitconfig for the LFS filter settings that get added when you initialize the LFS plugin. But I told it to run the command from the local path where the repo has been cloned. If the repo has never been cloned before (as in the case of the integration tests), that directory does not yet exist.

Since the config check only needs the global config, the cwd is actually not important.

@s0undt3ch

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

One last test failure.

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

OK, that last one should now be fixed.

@s0undt3ch s0undt3ch merged commit ab60782 into saltstack:2018.3 Jan 28, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/jenkins/pr-merge This commit is being built
Details
WIP Ready for review
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.