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

script: double change #89

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions build-dakkar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set -e
## set defaults

rom_fp="$(date +%y%m%d)"
curpath=$PWD

myname="$(basename "$0")"
if [[ $(uname -s) = "Darwin" ]];then
Expand All @@ -13,7 +14,7 @@ elif [[ $(uname -s) = "Linux" ]];then
fi

## handle command line arguments
read -p "Do you want to sync? " choice
read -p "[Phhusson magic-wand] Do you want to sync the whole jazz? " choice

function help() {
cat <<EOF
Expand Down Expand Up @@ -100,14 +101,14 @@ function get_rom_type() {
mainrepo="https://github.com/PixelExperience/manifest"
mainbranch="oreo-mr1"
localManifestBranch="android-8.1"
treble_generate="pixel"
treble_generate=""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that it really matters, but I guess they don't need extra_make_options either

extra_make_options="WITHOUT_CHECK_API=true"
;;
crdroid)
mainrepo="https://github.com/crdroidandroid/android"
mainbranch="8.1"
localManifestBranch="android-8.1"
treble_generate="crdroid"
treble_generate="lineage"
extra_make_options="WITHOUT_CHECK_API=true"
;;
mokee)
Expand Down Expand Up @@ -300,6 +301,25 @@ function jack_env() {
fi
}

function save_the_dev() {
if [ -d device/phh/treble ];then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please invert this to reduce indentation and for better clarity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invert?
What do you mean ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like
[ -d device/phh/treble] && return

cd device/phh/treble
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a subshell, instead of cd xxx; .... ; cd $curpath which is error prone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a function doesn't automatically assume to execute stuff in subshell?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if that's the case, you can just drop the cd $oldpwd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stuff is, the execution of this script is in the root of sync, if I drop the cd it will stash on the whole rom files and not only on DT

if git stash -u | grep -q 'No local changes to save';then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

broken indentation

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to if ! git stash -u;then instead

stashstatus=blank
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"blank" when you're later checking for "empty" is weird -_-'

fi
cd $curpath

fi
}

function restore_the_dev() {
if [ -d device/phh/trble ] && [ -z "$stashstatus" ];then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, trble => treble

cd device/phh/treble
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a subshell, instead of cd xxx; .... ; cd $curpath which is error prone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as said earlier, should be already in subshell since this is a function

git stash apply
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this might/will fail, some error handling should be done here

Copy link
Contributor Author

@Dil3mm4 Dil3mm4 Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this fails due to set -e should automatically stop the whole script notifying the user about the failed stash, so I dunno what we should handle further.

cd $curpath
fi
}

parse_options "$@"
get_rom_type "$@"
get_variants "$@"
Expand All @@ -314,7 +334,9 @@ init_release
init_main_repo
init_local_manifest
init_patches
save_the_dev
sync_repo
restore_the_dev
fi
patch_things
jack_env
Expand Down