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

Change the UI devapp server to use the standard build scripts #18

Merged
merged 1 commit into from
Aug 7, 2021

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Aug 7, 2021

I originally wanted to put all the dependencies into the image instead of
waiting until the container was set up. But different branches could have
different dependencies and may not want to upgrade/downgrade. And putting this
into the container allows us to use the standard build scripts and not have
this bitrot.

This file had
ENV NODE_VERSION 9.4.0
RUN npm install -g npm@6.0.0

We are currently at

export NODE_VERSION=14.7.0
export NPM_VERSION=6.14.8

using the standard scripts, which are regularly tested via CI, allows us to
ensure that this dockerfile stays relevant across the many configurations that
we currently support.

This fixes e-mission/e-mission-docs#657

I originally wanted to put all the dependencies into the image instead of
waiting until the container was set up. But different branches could have
different dependencies and may not want to upgrade/downgrade. And putting this
into the container allows us to use the standard build scripts and not have
this bitrot.

This file had
ENV NODE_VERSION 9.4.0
RUN npm install -g npm@6.0.0

We are currently at

export NODE_VERSION=14.7.0
export NPM_VERSION=6.14.8

using the standard scripts, which are regularly tested via CI, allows us to
ensure that this dockerfile stays relevant across the many configurations that
we currently support.

This fixes e-mission/e-mission-docs#657
@shankari
Copy link
Contributor Author

shankari commented Aug 7, 2021

Tested using the following settings in the docker-compose

diff --git a/examples/em-phone-devapp-serve/docker-compose.livereload.yml b/examples/em-phone-devapp-serve/docker-compose.livereload.yml
index 815d09d..23177bd 100644
--- a/examples/em-phone-devapp-serve/docker-compose.livereload.yml
+++ b/examples/em-phone-devapp-serve/docker-compose.livereload.yml
@@ -1,20 +1,20 @@
 version: "3"
 services:
   devapp-server:
-    image: emission/e-mission-phone.dev.ui-only:latest
+    image: emission/e-mission-phone.dev.ui-only:3.0.0
     environment:
       - PHONE_REPO=https://github.com/e-mission/e-mission-phone.git
       - PHONE_BRANCH=master
       # CHANGEME: enable this for autoreloading
-      # - CHOKIDAR_USEPOLLING=true
+      - CHOKIDAR_USEPOLLING=true
     ports:
       - "3000:3000"
-    # volumes:
+    volumes:
       # specify the host directory where the source code should live
       # If this is ~/e-mission-phone-docker, then you can edit the files at
       # ~/e-mission-phone-docker/e-mission-phone/www/...
       # - ~/e-mission-phone-docker:/src/
       # - CHANGEME:/src/
-      # - /tmp/e-mission-phone-docker:/src/
+      - /tmp/e-mission-phone-docker:/src/

Note that I had to mkdir -p /tmp/e-mission-phone-docker
However, the file permissions seemed fine

$ ls -al /tmp/e-mission-phone-docker
total 0
drwxr-xr-x    3 kshankar  wheel    96 Aug  5 21:35 .
drwxrwxrwt  135 root      wheel  4320 Aug  6 22:33 ..
drwxr-xr-x   32 kshankar  wheel  1024 Aug  5 23:02 e-mission-phone

@shankari
Copy link
Contributor Author

shankari commented Aug 7, 2021

Live reload works, including hitting breakpoints in the code.

live-reload demo small mov

@shankari shankari merged commit c999b9b into e-mission:master Aug 7, 2021
@asiripanich
Copy link
Member

asiripanich commented Aug 7, 2021

Would this work if I run the emulator but edit code on a code editor that is not Android Studio? Would be good if you can explain how you run live reload.

@asiripanich
Copy link
Member

Ok, so the auto reload works fine on the master branch.. Let me try again on rciti1.

@shankari
Copy link
Contributor Author

shankari commented Aug 7, 2021

You probably saw this, but you can edit the files with any editor and it will work. For the example above, I edited the files using vim. The large screen on the right is chrome, which is connected to the app and being used for debugging.

shankari added a commit to shankari/e-mission-phone that referenced this pull request Aug 9, 2021
Without this change, we get the following error while running in a docker container

```
Updating bower

bower ESUDO         Cannot be run with sudo

Additional error details:
Since bower is a user command, there is no need to execute it with superuser permissions.
If you're having permission errors when using bower without sudo, please spend a few minutes learning more about how your system should work and make any necessary repairs.

You can however run a command with sudo using "--allow-root" option
```

Since the default user in the container is `root`, this is actually an expected
use case for us.

I tried to avoid this by calling `bower update --allow-root` in the docker
setup script e-mission/e-mission-docker#18

But that ran into multiple inconsistencies (
e-mission/e-mission-docs#657 (comment)
e-mission/e-mission-docs#657 (comment)
)

In order to avoid more bitrotted containers, just changing the code in the
setup script so it works in the container as well

This fixes e-mission/e-mission-docs#657
asiripanich pushed a commit to rciti/e-mission-phone that referenced this pull request Aug 9, 2021
Without this change, we get the following error while running in a docker container

```
Updating bower

bower ESUDO         Cannot be run with sudo

Additional error details:
Since bower is a user command, there is no need to execute it with superuser permissions.
If you're having permission errors when using bower without sudo, please spend a few minutes learning more about how your system should work and make any necessary repairs.

You can however run a command with sudo using "--allow-root" option
```

Since the default user in the container is `root`, this is actually an expected
use case for us.

I tried to avoid this by calling `bower update --allow-root` in the docker
setup script e-mission/e-mission-docker#18

But that ran into multiple inconsistencies (
e-mission/e-mission-docs#657 (comment)
e-mission/e-mission-docs#657 (comment)
)

In order to avoid more bitrotted containers, just changing the code in the
setup script so it works in the container as well

This fixes e-mission/e-mission-docs#657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants