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

Pass Gitlab auth_token to packages listed in Remotes #266

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

overmar
Copy link
Contributor

@overmar overmar commented Dec 4, 2018

When install_gitlab has to install packages from the remotes line, it loses its auth_token, this allows the auth_token to persist in the ... and be recalled when necessary

@overmar overmar changed the title Patch 1 Pass Gitlab auth_token to packages listed in Remotes Dec 6, 2018
@bestdan
Copy link

bestdan commented Feb 15, 2019

I think this might solve this issue for github too:
#295

Any reason to not make both changes in this PR?

@antoine-sachet
Copy link
Contributor

antoine-sachet commented Apr 10, 2019

This may have unintended consequences, just like PR #145 introduced #337.

This works when the dependency needs the same host, user, password, auth_token, etc. But what if it does not? Then you've just broken the installation by passing the wrong host. Even if you just pass the credentials, you may pass the wrong credentials: imagine a gitlab package with a dependency on a github package, it will use the wrong auth_token.

You can easily pass credentials to the dependencies using environment variables (and it won't pass the wrong credentials), so there is no need for this in the first place. A better solution would be a useful message if private dependencies are detected/suspected.

By the way, credentials are already passed to the dependencies (since #145, see below).

combine_deps(
    package_deps(deps, repos = repos, type = type),
    remote_deps(pkg, ...)) # credentials passed to remote deps here

package_deps does need access to the repo to check the version but in any case the result is discarded since combine_deps will keep the remote from remote_deps... so does package_deps really need the sha of the remote commit?

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.

3 participants