Feature/git fork #628
Feature/git fork #628
Conversation
bin/git-fork
Outdated
|
|
||
| if [[ -z "$token" ]]; then | ||
| echo "Enter github password" | ||
| read password |
There was a problem hiding this comment.
I prefer let curl ask for my password instead of passing it in the option.
There was a problem hiding this comment.
Understandable. Although one is already being asked for the MFA code. Does that make sense to be asked for the MFA code before the password?
There was a problem hiding this comment.
@hickey
I think the order is not critical. Just because let curl ask for your password is simpler.
bin/git-fork
Outdated
| @@ -55,6 +90,12 @@ if [ "$origin" = true ]; then | |||
| else | |||
| # clone forked repo into current dir | |||
| git clone "${remote_prefix}${user}/${project}.git" "$project" | |||
| until [[ -d "$project" ]]; do | |||
| echo "Github is being slow today. Waiting 10 secs to attempt clone." | |||
There was a problem hiding this comment.
There are various causes that should be responsible for the failure of git clone.
Fortunately, the error message from git clone is clear enough.
Better shorten the message and remove our guess.
There was a problem hiding this comment.
The issue I see there is that git clone is very verbose and just adds a lot of cruf to the terminal that does not need to be there (see below). Although I just realized that I should be sending stderr to /dev/null to avoid the excess cruf.
Since the fork is asynchronous there is no way to determine when the fork is complete other than trying it. If I execute the fork command against github.com, the repo exists by the time that the clone is attempted. But if I try the fork command against the internal Github Enterprise instance, I end up waiting at least once for the fork to complete. (probably the fast LAN vs 200 ms across the net).
The more that I think about it, the attempt to clone should probably only be attempted for about a minute or so. After that point the command should exit with an error code and a message that the repo could not be cloned locally.
0:128 ᐅ git clone [email protected]:hickey/fioo.git foo
Cloning into 'foo'...
ERROR: Repository not found.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
bin/git-fork
Outdated
| @@ -55,6 +90,12 @@ if [ "$origin" = true ]; then | |||
| else | |||
| # clone forked repo into current dir | |||
| git clone "${remote_prefix}${user}/${project}.git" "$project" | |||
There was a problem hiding this comment.
Maybe we could check if $project exists before cloning into it?
There was a problem hiding this comment.
Git clone will not allow one to clone over an already existing repo directory.
0:0 ᐅ git clone [email protected]:hickey/git-extras.git git-extras
fatal: destination path 'git-extras' already exists and is not an empty directory.
There was a problem hiding this comment.
@hickey
Ah ha, this idea is inspired by your -d $project check.
If there is $project already, we can directly exit the script, skip git clone, cd, and so on.
bin/git-fork
Outdated
| @@ -55,6 +90,12 @@ if [ "$origin" = true ]; then | |||
| else | |||
| # clone forked repo into current dir | |||
| git clone "${remote_prefix}${user}/${project}.git" "$project" | |||
There was a problem hiding this comment.
@hickey
Ah ha, this idea is inspired by your -d $project check.
If there is $project already, we can directly exit the script, skip git clone, cd, and so on.
hickey
force-pushed
the
feature/git-fork
branch
from
f8d04bc to
7867f02
Compare
|
Added better detection when the fork fails (which is really a comms error with talking to Github). If directory already exists, clone will silently fail and the upstream references will be created. If they already exist, then a warning message is printed that it exists. |
|
If you would prefer, I can squash all the commits and resubmit the PR. |
| cd "$project" | ||
| git remote add upstream "${remote_prefix}${owner}/${project}.git" | ||
| git fetch upstream | ||
| git clone "${remote_prefix}${user}/${project}.git" "$project" 2>/dev/null |
There was a problem hiding this comment.
I agree that the output of git clone is cumbersome, but still think we should not hide the error message. They are like low level log, which may be useful sometimes.
There was a problem hiding this comment.
Still insist the need for the output of git clone.
| until [[ (( "$total_wait" > 60 )) || -d "$project" ]]; do | ||
| echo "Github is being slow today. Waiting $timeout secs to attempt clone." | ||
| sleep ${timeout[0]} | ||
| total_wait=$(expr $total_wait + ${timeout[0]}) |
There was a problem hiding this comment.
Better to use total_wait=$((total_wait + timeout[0])), which is more modern.
There was a problem hiding this comment.
I thought about that, but some of the older shells don't understand the double parenthesis. So I opted for the more traditional form.
| # Check to make sure we cloned the repo. Backoff exponetially and try again | ||
| timeout=( 2 2 2 6 6 6 20 20 ) | ||
| total_wait=0 | ||
| until [[ (( "$total_wait" > 60 )) || -d "$project" ]]; do |
There was a problem hiding this comment.
Actually, it will wait for 64(44 plus the last 20) seconds.
man/git-fork.1
Outdated
| @@ -40,7 +40,71 @@ adds the forked repo as a remote called \fBorigin\fR | |||
| .P | |||
| Remotes will use ssh if you have it configured with GitHub, if not, https will be used\. | |||
| . | |||
| .P | |||
| Your Github settings can not be saved as git config values instead of specifying them each time\. To enable this you need to execute a few git config commands like the following\. | |||
There was a problem hiding this comment.
I believe you meant now rather than "not be saved" here.
man/git-fork.html
Outdated
| @@ -99,8 +99,52 @@ <h2 id="DESCRIPTION">DESCRIPTION</h2> | |||
|
|
|||
| <p> Remotes will use ssh if you have it configured with GitHub, if not, https will be used.</p> | |||
|
|
|||
| <p> Your Github settings can not be saved as git config values instead of | |||
There was a problem hiding this comment.
And here as well: s/not be saved/now be saved/
man/git-fork.md
Outdated
| @@ -21,8 +21,49 @@ git-fork(1) -- Fork a repo on github | |||
|
|
|||
| Remotes will use ssh if you have it configured with GitHub, if not, https will be used. | |||
|
|
|||
| Your Github settings can not be saved as git config values instead of | |||
There was a problem hiding this comment.
And here as well: s/not be saved/now be saved/
|
Sorry, I have been busy and this PR fell off the list. Updated wording in the documentation and surprised that I missed that typo. Thanks. |
|
Let me know if you would like this PR squashed. |
| # set_config_value "key" "value" "global|system" (global or system not required) | ||
| set_config_value() { | ||
| $(git config ${3+--$3} git-extras.$1 $2) | ||
| echo $? |
There was a problem hiding this comment.
IMHO, I prefer to use git config ${3+--$3} git-extras.$1 $2 directly.
When writing shell script, people are more relied on $? that the return value to check if the previous command succeeded.
| cd "$project" | ||
| git remote add upstream "${remote_prefix}${owner}/${project}.git" | ||
| git fetch upstream | ||
| git clone "${remote_prefix}${user}/${project}.git" "$project" 2>/dev/null |
There was a problem hiding this comment.
Still insist the need for the output of git clone.
|
You may want to use the curl's |
|
We should close this — it's out-of-date and 4 years old, and has no votes. |
Added support for forking on instances of Github other than github.com.
In addition added ability to pull stored settings from git config so that one is not prompted each time for things like username and such.
Also introduced a self documenting way of including helper scripts into the generated scripts. See the Makefile and helper/config-value for details. Also helps keeping one from forgetting to update the needs_* files.