This would render the DockerFile non-functional outside of Travis. I would highly recommend not doing this. Hardcoding paths in CI can create nightmares for future comers... I work off forks and make package changes without a problem - I'd like to hear more about the issue you are having. That's the point of these lines to look if its a PR then it will grab your code to work with over master. If you're PRing those fetch lines will grab your changes regardless of original repo or fork.

Other options

• make docker container ARGs for username that defaults to ros-planning but then allows someone running locally to change to theirs on run time. The branch to use is already an ARG and that's how it builds correctly today
• use travis environmental variables like TRAVIS_BRANCH and TRAVIS_BUILD_DIR which already clones the relevant branch into the container.

The docker container should be standalone if at all possible

@crdelsey - here is the run with a failure introduced: https://travis-ci.org/Nickolaim/navigation2/builds/461388945

@SteveMacenski - I am confused. The old version had the hardcoded staff and was not working for the local builds. The new version will work for local builds, with whatever changes are happening locally.

This is the first time in 10+ years I work with CI I see a case when CI pipeline is NOT building/testing the current changes. This is very unusual and unintuitive.

It does work with local builds on master, but your proposed changes wont work on local changes at all unless you have this docker file in a particular directory - making it not standalone. I can't run docker build without it failing as an independent file. If you wanted to make it run locally on an arbitrary branch, I'd recommend my first bullet with run time ARGs that default to CI logic to allow you to clone the base repo of your chosing (instead of ros-planning, be $ARG_USER where ARG_USER=ros-planning` at the start to make it default to current settings.
ie (before cloning)

ARG USER='ros-planning'

and when running locally set the docker build variable with the user name you want to override. PULLREQ will default false if you dont set so then it'll just run with your flavor. That's the best case I think.

The old version will clone ros-planning/master and check if its running as a pull request - if so, it fetches your branch that the PR is based off of and checks it out.

git fetch origin pull/$PULLREQ/head:pr_branch; git checkout pr_branch;

I think there are several scenarios here:

1. Running docker in Travis CI in ros-panning/navigation2 on master. This shows health of the of the the master branch and it is absolutely necessary
2. Running docker in Travis CI for PRs. This shows quality of the PR and also absolutlely necessary, otherwise 1 will be affected
3. Running docker in Travis CI for forked repos. I think it is necessary, because this shows quality of the change. If the this run is red, PR should not happen
4. Running docker on a local machine with changes from a specific repo/branch. Note that it is not going to use local source code, and I find this is very confusing
5. Running docker on a local machine with changes on the local machine

We can discuss the scenarios and importance. To me 1,2,3 and 5 are important. I understand how to make this code work for 1-4, but I am not sure that's the right approach.

I agree with @SteveMacenski - I think this should be handled by adding another ARG to the Dockerfile and checking the variable before deciding whether to clone or use the local copy. The default should be to clone master, which will make the Dockerfile able to be run from any location.

@mkhansen-intel - OK.

What about Travis CI runs in the forked repo? Should they run code from the forked repo or they should run code from ros-planning/navigation2 on master? The other option is to break hard, with an error message saying that one has to do PR in order to see the actual results of the changes.

@Nickolaim let me walk through a few of these so you can see what I'm talking about

1 Run locally
If you set the ARG USER='ros-planning' when you do the docker buil, you can set this USER to nickolaim and therefore you'll build your docker file locally to test. When you don't set or running in CI on Travis, then it will default to ros-planning and not effect any of the current work.

2 Run CI
ARG USER='ros-planning' will use the default unless someone changes the .travis.yml file and not effect CI in any way.

3 Run CI test locally not on a branch
That's not really supported in this configuration I suppose, but if you're running changes locally, you can just change the line you need to copy over. Alternatively (and what I believe you should do) is commit those changes to a branch. It's always good to commit early and often. It's easy to squash commits but its exceedingly challenging to rip one apart later.

4 Deploy docker
If I want to deploy a stable tag release or edge in order to test on a robot in my hardware CI, I just need to use the dockerfile as is without any additional changes. If I want to use a tag, I can just send it through the existing param like travis does. If I was edge, then I just run it as normal and creates a perfect docker image

5 personal fork on Travis server
Please please don't do this, you can run locally as in the above examples and it will give you the same results as if you ran on Travis. Additionally, you can just run on travis as part of a PR once you're ready for that. You can always make a docker image and compile / test you code in the container so you don't pollute your outside environment. Docker (as opposed to snaps) let you get in and run commands and compile code. Some software developers singularly develop inside of clean docker containers.
Also when you do this, as the nature of travis builds since its not a pull request, we all get emails about your builds (success or failure). I typically ignore them and don't care, but you can just run locally with the ARG.

Now if we do what you propose with changing it to copy the local environment, it might be slightly more convenient for your testing, but it is overall much harder to do the majority of the items above. Keep in mind, we all have our own separate developer tools. If you want a docker environment to build code in you can just make a docker for you to work in. I have my own set, I'm sure other folks here do as well.

For example, if you take the current docker file and cut it off here what you end up with is the correct environment to do all your development and you can just go into the container, put whatever code you like into it with ordinary bash tools and develop in that isolated sandbox.

phew

Does that make sense? If not, we can discuss more, I'd be open to a phone call, especially if I'm misunderstanding.

Sorry for the long discussion. My experience is very different from what this project is required. Hope #393 will address the issues raised in this PR.