Replace tar+scp with rsync (#22) #35
Conversation
| @@ -13,16 +13,6 @@ PROJECT_DIR_NAME="$( basename "$PROJECT_DIR")" | |||
|
|
|||
| # Read config variables from local.properties. | |||
| REMOTE_BUILD_MACHINE=$(awk -F "=" '/remote_build.machine/ {print $2}' "$PROJECT_DIR/local.properties") | |||
| LOCAL_GZIP_LEVEL=$(awk -F "=" '/remote_build.local_gzip_level/ {print $2}' "$PROJECT_DIR/local.properties") | |||
There was a problem hiding this comment.
rsync actually supports compression (from 1 to 9) via --compress-level=
| --exclude='artifacts' \ | ||
| --exclude='captures' \ | ||
| --exclude='build' \ |
There was a problem hiding this comment.
Why not exclude local build files?
There was a problem hiding this comment.
If I understand correctly it is excluded a little bit below.
|
|
||
| # Build project on a remote machine and then archive it. | ||
| # Build project on a remote machine |
There was a problem hiding this comment.
Dot at the end please :)
|
|
||
| # Copy build results from remote machine to local. | ||
| scp "$REMOTE_BUILD_MACHINE":~/"$PROJECT_DIR_NAME"/remotely_built_project.tar "$PROJECT_DIR/build/" | ||
| cd ~/mainframer/ && \ |
There was a problem hiding this comment.
$PROJECT_DIR_NAME?
|
Updated with requested changes |
| @@ -162,8 +162,8 @@ You can tune compression levels as you wish. Default compression levels are `1`. | |||
| Configurable via `local.properties`: | |||
|
|
|||
| ``` | |||
| remote_build.local_gzip_level=1 | |||
There was a problem hiding this comment.
This is breaking change, let's leave properties name as they are for now :)
There was a problem hiding this comment.
Actually rsync itself is kind of breaking, but we can plan this for 1.1.
There was a problem hiding this comment.
I don't see why, it is presented in Mac OS and most Linux distros, should work for most of users of out the box
There was a problem hiding this comment.
It is still a behaviour change. I’m running it locally at this point without any issues, but who knows.
There was a problem hiding this comment.
Well, it is, but at this point of time it should be very compatible for most users, so I would prefer to update only minor version
| LOCAL_ARCHIVE_COMMAND="tar \ | ||
| -c \ | ||
| --exclude='build/project_for_remote_build.tar' \ | ||
| --exclude='local.properties' \ |
There was a problem hiding this comment.
You must exclude local.properties, otherwise remote build may fail if it'll see your local.properties with path to android sdk that does not make sense for remote build machine!
There was a problem hiding this comment.
Even worse, it can rewrite or remove the local one on back sync.
There was a problem hiding this comment.
Omg, totally needs to be fixed then
There was a problem hiding this comment.
Looks and works great, some nits + @artem-zinnatullin comments.
| pushd "$PROJECT_DIR" | ||
| eval "$LOCAL_UNARCHIVE_COMMAND" | ||
| # Sync project back to local machine. | ||
| rsync -a --delete --compress-level=$REMOTE_COMPRESS_LEVEL \ |
There was a problem hiding this comment.
Don’t know about @artem-zinnatullin, but I would prefer full parameter names, i. e. --archive instead of -a.
| eval $LOCAL_ARCHIVE_COMMAND | ||
| popd | ||
| --exclude='**/build' \ | ||
| -e "ssh" ./ "$REMOTE_BUILD_MACHINE:~/$PROJECT_DIR_NAME" |
| @@ -162,8 +162,8 @@ You can tune compression levels as you wish. Default compression levels are `1`. | |||
| Configurable via `local.properties`: | |||
|
|
|||
| ``` | |||
| remote_build.local_gzip_level=1 | |||
There was a problem hiding this comment.
Well, it is, but at this point of time it should be very compatible for most users, so I would prefer to update only minor version
…rguments in rsync commands
|
Cheers for the feedback, updated with requested changes |
|
Was able to save about 30 seconds (working from remote location today), thank you @steventannz! |
|
@steventannz thank you for PR! |
Replaces tar and scp with rsync as part of #22