ros-industrial / industrial_ci

Easy continuous integration repository for ROS repositories
Apache License 2.0
249 stars 129 forks source link

Make built workspace names unique #751

Closed marip8 closed 3 years ago

marip8 commented 3 years ago

I have a use case where I would like to use the docker image produced by ICI to be the base image and UNDERLAY for CI for a different repository. This is problematic because the current names of the workspaces built by ICI are hard-coded to base_ws, upstream_ws, target_ws, and downstream_ws. This means that I need to specify UNDERLAY=/root/target_ws/install, but the source code of the second repository also gets copied into /root/target_ws/src. Coincidentally this works for simple repositories, but (probably) breaks down if an upstream workspace is needed for the second repository build.

The proposed fix to this issue is to prepend the name of the target repository to each workspace name. For example target_ws for repo_a would become repo_a_target_ws. Then repo_b could use UNDERLAY=/root/repo_a_target_ws/install, and its own upstream and target workspaces would be built in /root/repo_b_upstream_ws and /root_repo_b_target_ws, respectively, producing the desired workspace chain

mathias-luedtke commented 3 years ago

I have a use case where I would like to use the docker image produced by ICI to be the base image and UNDERLAY for CI for a different repository.

Interesting idea!

The proposed fix to this issue is to prepend the name of the target repository to each workspace name.

Wouldn't it be enough to set BASEDIR? (Not sure if this would really work with more than 2 layers)

Instead of prepending it like this for all users (might break some setups!), I would be open to adding a WORKSPACE_PREFIX option (or better name), which optionally can be a directory (ending with "/") as well.

marip8 commented 3 years ago

Wouldn't it be enough to set BASEDIR? (Not sure if this would really work with more than 2 layers)

I wasn't aware that this was a settable variable since its not in the documentation (to my knowledge). I just tried it, and it seems to work fine for my use-case. Is there a reason it wouldn't work for more than 2 layers?

It seems like setting BASEDIR=/root/<UNIQUE_NAME> for each repository would allow any number of repositories to be installed in different locations and be chained together. If this is the case, is there a preference between setting BASEDIR and adding a new WORKSPACE_PREFIX variable?

mathias-luedtke commented 3 years ago

wasn't aware that this was a settable variable since its not in the documentation (to my knowledge).

Yes.. Unfortunately, there are many new features, which need documentation

is there a preference between setting BASEDIR and adding a new WORKSPACE_PREFIX variable?

BASEDIR will be bind-mounted, so I am not sure, if it will be persisted in the image properly. WORKSPACE_PREFIX would just change the names inside the BASEDIR.

e.g.

upstream_ws=$BASEDIR/${WORKSPACE_PREFIX}upstream_ws

I just tried it, and it seems to work fine for my use-case.

Did you try it locally? With Gitlab? Please check if you now have that BASEDIR on your Docker host.

marip8 commented 3 years ago

Did you try it locally? With Gitlab?

I tried it with Github Actions on one of my repositories

BASEDIR will be bind-mounted, so I am not sure, if it will be persisted in the image properly.

The final step of my test CI process pushes the built docker image to the Github container registry. I pulled that docker image, and it turns out that the BASEDIR directories were created but were empty.

I'll implement the WORKSPACE_PREFIX option and try that instead

mathias-luedtke commented 3 years ago

and it turns out that the BASEDIR directories were created but were empty.

Yes, I came the same conclusion in my local test. These empty folders are just left-overs from the bind mounting..

I'll implement the WORKSPACE_PREFIX option and try that instead

:+1: We could name it PREFIX as well, so we can use it for all kinds of data, not just workspaces