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.

Sign up for GitHub

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

Jump to bottom

Clarify logic around dev builds and rebuilding #72

Merged
merged 7 commits into from
Sep 4, 2024
Merged

Clarify logic around dev builds and rebuilding #72

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
720fbb9
Disable rebuild of dev docker imgs w --build false
Tristan-Wilson Aug 7, 2024
26ffe7d
Clarify logic around dev builds and rebuilding
Tristan-Wilson Aug 12, 2024
0e22925
No skip_build
Tristan-Wilson Aug 12, 2024
aae904b
Contracts and node images should rebuild on --init
Tristan-Wilson Aug 13, 2024
4732c5d
Declare variable before use for -u flag
Tristan-Wilson Aug 13, 2024
4c489a7
Change contracts to utils, add --force-build-utils
Tristan-Wilson Aug 14, 2024
a7c9f53
Merge branch 'master' into skip-rebuild-opt
Tristan-Wilson Sep 4, 2024
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
22 changes: 17 additions & 5 deletions test-node.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ fi

run=true
force_build=false
skip_build=false
validate=false
detach=false
blockscout=false
Expand Down Expand Up @@ -91,8 +92,19 @@ while [[ $# -gt 0 ]]; do
fi
;;
--build)
force_build=true
shift
if [[ $# -eq 0 || $1 == -* ]]; then
# If no argument after --build, set flag to true
force_build=true
else
while [[ $# -gt 0 && $1 != -* ]]; do
if [[ $1 == "false" ]]; then
force_build=false
skip_build=true
fi
shift
done
fi
;;
--validate)
simple=false
Expand Down Expand Up @@ -177,7 +189,7 @@ while [[ $# -gt 0 ]]; do
echo $0 script [SCRIPT-ARGS]
echo
echo OPTIONS:
echo --build rebuild docker images
echo --build rebuild docker images. --build false disables rebuild
Copy link
Collaborator

Choose a reason for hiding this comment

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

change that to --no-build
not necessarily better, but that's how we currently do all boolean options

Copy link
Collaborator

Choose a reason for hiding this comment

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

(other option is to change no-tokenbridge, no-simple etc..)

Copy link
Member Author

Choose a reason for hiding this comment

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

--no-build is cleaner, I'll change it to that.

echo --dev build nitro and blockscout dockers from source instead of pulling them. Disables simple mode
echo --init remove all data, rebuild, deploy new rollup
echo --pos l1 is a proof-of-stake chain \(using prysm for consensus\)
Expand All @@ -200,17 +212,17 @@ while [[ $# -gt 0 ]]; do
esac
done

if $force_init; then
if ! $skip_build && $force_init; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems there's currently a lot of disarray in force_build, dev_build_X.. in that way skip_build is reasonable but it adds to the mess instead of removing from it.
I can merge this as I don't have a clear better way - but see if you can come up with something.
I would like the solution to parse things in order, so e.g.:
--init --no-build > will not build, but
--init --no-build --build > will build
(it's a good property for scripts run by other scripts, so you can always change behaviour by adding an argument at the end)

I think some of the mess comes from the duality of build being used for building contracts/scripts vs used to build local nitro.. maybe there should be different args for that.

A final note: external users don't generally use --dev so I don't mind introducing changes that will break usage that does build local nitro.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I've come up with something that clarifies it, PTAL.

force_build=true
fi

if $dev_build_nitro; then
if ! $skip_build && $dev_build_nitro; then
if [[ "$(docker images -q nitro-node-dev:latest 2> /dev/null)" == "" ]]; then
force_build=true
fi
fi

if $dev_build_blockscout; then
if ! $skip_build && $dev_build_blockscout; then
if [[ "$(docker images -q blockscout:latest 2> /dev/null)" == "" ]]; then
force_build=true
fi
Expand Down
Loading