let container job handle prepend path correctly.

[TingluoHuang]

No description provided.

[TingluoHuang]

#1969

[davidroberts63]

I've built this locally and tested it out. It does not appear to work as intended. I've compared this (v2.142.1) with my existing agents (v2.141.1) and I am seeing the same PATH value within the container. I'm testing additional items out locally though.

I may be doing something incorrectly with the built agent though. If there are any recommendations I'm happy to try them out.

[davidroberts63]

Okay, after some testing I can confirm code as it is now doesn't prepend the path correctly.

[davidroberts63]

This is where the PATH environment variable is being set. Before the prepend logic would be able to take affect.

[davidroberts63]

I'm willing to offer up a pull request to work with you on this @TingluoHuang. I'm thinking of a few options at the moment:

1. If stdinData.environment['Path'] exists, merge/prepend it's value with the existing process.env['Path']. This could eliminate the prepend path logic completely. Less code.
2. Somehow in here determine if the build is running in a container, if so then skip the stdinData.environment['Path'] item. Letting the prepend logic take over.

[TingluoHuang]

@davidroberts63 thanks for testing this. i haven't have time to test this yet.
can you share a log with system.debug=true that indicate where the PATH is set?

[davidroberts63]

Tried to add it as a file here but there were problems so here's a link to it on gist:

https://gist.github.com/davidroberts63/84c0381d3dc0f37d8b4566256328e1a1

This is where the PATH env var is being set by the Object.keys forEach.
https://gist.github.com/davidroberts63/84c0381d3dc0f37d8b4566256328e1a1#file-worker-log-txt-L2953

After that is where the prepend logic is running:
https://gist.github.com/davidroberts63/84c0381d3dc0f37d8b4566256328e1a1#file-worker-log-txt-L2955

So the prepend is prepending to an already overwritten Path.

[TingluoHuang]

@davidroberts63 you are not on the right commit. :D
https://gist.github.com/davidroberts63/84c0381d3dc0f37d8b4566256328e1a1 shows your are on 7eb9a7495adef78afee24c8590b0917fafc65cfc. i force push the branch 2 days back, so 959abc7115693cb0d887ed0ca262db63ee9c9975 is the write commit.
i tried myself, seems works fine for me. :)

[davidroberts63]

I can confirm that it does work! That's wonderful. Thank you for your patience in pointing out I had pulled an old commit. And again, thank you for such a quick response @TingluoHuang.

[TingluoHuang]

@davidroberts63 thank you for help me test this out. i will merge the PR.