-
Notifications
You must be signed in to change notification settings - Fork 491
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 Github Token to the composer.sh install script #493
Pass Github Token to the composer.sh install script #493
Conversation
…r NULL due to $GITHUB_OAUTH being an alphanumeric hash
I'll try your solution later today, with the changes in my PR I got it working again. Maybe we should merge both solutions? |
Applying only your changes results in this error:
Applying my changes and rerunning it results in the same error. I'll try some different settings to see if I can get it working. |
I'll take a look at it later. You are creating a Github Token for Github account and setting it at |
Yes, I created a token at https://github.com/settings/tokens with repo access. Debugging by adding these lines to composer.sh (Most current version) shows me no token
Looks like the arguments still aren't handled correctly. |
@stefantalen With the current master branch, the token is never getting passed to the script to begin with due to the $1 being "" instead of github_pat
https://github.com/fideloper/Vaprobash/blob/master/Vagrantfile#L323 I'm curious what is getting set for you under /home/vagrant/.composer/auth.json when it's giving you the error below:
With #492 and this change, if I set the github_pat within the Vagrantfile it will set it correctly under /home/vagrant/.composer/auth.json for github.com |
I feel really dumb right now, I commented the original composer installer script and still used my local script. That one never passed the argument 😅 Maybe you can consider adding some echo's within the if statement when the token is being set? That way it'll be more visible what has been set during provision. |
Added an echo with the token output. Should the token be in the output or is the "Setting Github Personal Access Token" sufficient enough?
|
Just noticed this should also correct @Ilyes512 concern at #465 (comment) |
👍 I think adding the token to the output will let you know what token has been set in case something went wrong. |
Though problem might be that if people aren't aware they could be exposing their PAT tokens when showing the output of a I guess you could say the same thing about posting their Vagrantfile, so non-issue, just a thought. |
Maybe concat the token first before displaying it? |
@chrispelzer Does this need any rework? Or is it ready to merge? |
…explicit that it is set correctly" This reverts commit 111beb6.
This is ready to merge. The Github PAT will get passed correctly when set. I reversed the PAT token from being shown, even though it's a development only situation. I just don't like it being exposed and the possibility of someone copying and pasting it. |
…omposer Pass Github Token to the composer.sh install script
thanks! |
* Escape some quotes in environment.sh * Fix incompatibility between Vaprobash 1.4.2 where first parameter for composer.sh is now treated as a Github token Ref: fideloper/Vaprobash#493
* Escape some quotes in environment.sh * Fix incompatibility between Vaprobash 1.4.2 where first parameter for composer.sh is now treated as a Github token Ref: fideloper/Vaprobash#493
* Escape some quotes in environment.sh * Fix incompatibility between Vaprobash 1.4.2 where first parameter for composer.sh is now treated as a Github token Ref: fideloper/Vaprobash#493
The problem with #492 and #491 might be due to the github token not being passed to composer.
This corrects and installs PHPUnit, as if a github_pat was set, it still never ended up getting to composer.sh
I can't say much for if it's an issue for the VMware provider as I only run Virtualbox.