osrf / gzweb

Web client for Gazebo classic simulation
http://gazebosim.org/gzweb
Other
63 stars 60 forks source link

[deploy.sh] fix node-gyp failure on rebuild #191

Closed moriarty closed 4 years ago

moriarty commented 4 years ago

node-gyp configure will fail if build directory already exists.

One reason this might already exists is if new models have been added to the model path, and npm run deploy --- -m local is run again.

This change enables building a generic docker image for gzweb, and later loading in the models at launch time via a volume mount that were not known when the original gzweb image was built.

moriarty commented 4 years ago

@iche033 & @chapulina I must admit I'm no expert in nodejs so this might be a hack.

There are some deprecated and old tags for gzweb here: https://hub.docker.com/r/osrf/gazebo/tags

And there are no images or tags of gzweb in the new official location here: https://hub.docker.com/_/gazebo/tags

The images here: https://github.com/osrf/docker_images are out of date https://github.com/osrf/docker_images/blob/master/gazebo/9/ubuntu/bionic/gzweb9/Dockerfile and haven't been updated since the bitbucket transition https://github.com/osrf/gazebo_tutorials/pull/118 but they also needed a few more changes.

In general, the old gzweb docker_image in https://github.com/osrf/docker_images seems more like an sample of how to put gzweb into docker, it starts a gzserver (without many plugins available or the mechanisms to load custom ones in place) and gzweb which is probably not what it would do in. @mikaelarguedas might have some insight here as I've opened issues to docker_images in the past and to use them in practice usually you end up writing your own.

In practice, I believe it's possible to create a generic gzweb docker image, which is not aware of all the required models ahead of time, but when brought up with kubernetes or docker-compose, unknown models could be volume mounted to the gzweb container, and added to the GAZEBO_MODEL_PATH environment variable, and the command run by gzweb would first re-run deploy -m and then run start...

Not actual code, just an example of how this change would enable updating the official gzweb images for generic use:

version: "3"
  gz-server:
    ...
    volumes:
      - gz-server-models:/path/to/the/actual/models

  gz-web:
     image: could.be.official.registry/gzweb:latest
     depends_on:
       - gz-server
     environment:
       - GAZEBO_MASTER_URI=gz-server:11345
       - GAZEBO_MODEL_PATH=/path/to/mount/models
     volumes:
       - gz-server-models:/path/to/mount/models:ro
     command: >
       bash -c "npm run deploy --- -m local
       && npm start"

volumes:
  gz-server-models:
moriarty commented 4 years ago

GitHub actions were not triggered on this PR see PR #192

moriarty commented 4 years ago

Ignore all the docker reasoning I mentioned... I think this is simply a bug. https://github.com/nodejs/node-gyp/blame/9bf112b44c98be39bf8770a1341fa4ddf23556d4/README.md#L167-170

moriarty commented 4 years ago

Bump @iche033 and @chapulina I'll copy from the nodejs/node-gyp readme link I included above her for quicker reference

Command Description
help Shows the help dialog
build Invokes make/msbuild.exe and builds the native addon
clean Removes the build directory if it exists
configure Generates project build files for the current platform
rebuild Runs clean, configure and build all in a row
install Installs Node.js header files for the given version
list Lists the currently installed Node.js header versions
remove Removes the Node.js header files for the given version

This PR replaces:

with just:

Because build will fail when the build directory isn't clean.