Fix for wrapper scripts not being applied in non-interactive environments #85
Conversation
| @@ -0,0 +1,6 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
These all (apart from dotnet + nuget) could be auto-generated, since they are identical except in naming.
abdurriq
force-pushed
the
azure-artifacts-shims-fix
branch
from
d59af58 to
2e02c21
Compare
| [[ ${RESOLVE_SHIMS_IMPORTED} == "true" ]] && return | ||
| RESOLVE_SHIMS_IMPORTED=true |
There was a problem hiding this comment.
Useful to avoid the function being redefined according to Copilot. Probably unnecessary.
…ents These environments do not respect the BASH_ENV variable so installing these alias hooks does not work
abdurriq
force-pushed
the
azure-artifacts-shims-fix
branch
from
2e02c21 to
c298b99
Compare
|
This is similar to #83 Can we think about compatibilty for all the people using I still also wonder how reliable PATH is and whether since this is a devcontainer if there was anything else we could do to help the shims be first in PATH? |
|
@microsoft-github-policy-service agree company="Microsoft" |
Ah good point; I expect we need to maintain backward compatibility for these then?
Yeah, this is a bit tricky, which is why I preferred to delegate it to the consumer (by providing a suitable |
|
I guess |
My thought is that we leave the # File created for backwards compatibility only
# use version 2 if the functionality is needed
# "features": {
# "ghcr.io/microsoft/codespace-features/artifacts-helper:2": {}
# }There are some people that |
I do like your approach. I was just kind of thinking for documentation purposes if there were any other suggestions we could make about things the customer could do with their It would be good to make a few test Codespaces using the universal, dotnet and node images and just compare the PATH to see if there is a consistent location we could use. What folder does |
|
I did a code search and only found one repo using cc: @scaryrawr |
Oh nice, thank you very much! In that case, I'll drop that option + implementation, and keep only the new implementation. I will check the |
|
#55 discusses some of the reasons people were trying BTW, could we put some checks and a wait/retry in the shim script? When a Codespace first starts there is a delay until the ADO extension activates. If, as an example, the C# extension runs I have used a script like this in my #!/bin/bash
set -e
echo "::step::Waiting for AzDO Authentication Helper..."
# Wait up to 3 minutes for the ado-auth-helper to be installed
MAX_WAIT=180
ELAPSED=0
while [ $ELAPSED -lt $MAX_WAIT ]; do
if [ -f "${HOME}/ado-auth-helper" ]; then
echo "::step::Running dotnet restore..."
dotnet restore
echo "::step::✓ Restore completed successfully"
exit 0
fi
sleep 2
ELAPSED=$((ELAPSED + 2))
# Progress indicator every 20 seconds
if [ $((ELAPSED % 20)) -eq 0 ]; then
echo " Still waiting... (${ELAPSED}s elapsed)"
fi
done
# Timeout reached
echo "::error::AzDO Authentication Helper not found after ${MAX_WAIT} seconds"
echo "Expected location: ${HOME}/ado-auth-helper"
echo "Restore cannot proceed without authentication"
exit 1And we do something similar in the We ought to be able to build that in here as well so that when the shim is invoked it checks to see if the environment is ready yet before moving on. I would guess that could be added to your |
This is the same motivation for me!
Got it, will add this too. |
There doesn't seem to be any consistency between them unfortunately.
|
|
That is what I was afraid of. I think we also have to consider what PATH does VS Code extensions have and also before we use an existing folder we need to make sure that the tools themselves do not install in thos folders. What if we made something like I would also suggest we just create our own folder in |
|
I assume you do not have any way to test this? I propose you add a new feature folder called We can keep |
I was going to try to setup a Docker container and some how mock calls to verify that the correct binaries are called (e.g. have a mocked If not, the alternative you mentioned would be more straightforward. |
|
Well by test I meant actually use it and work out what is needed for it to solve the problems we want to solve. But for the automated tests, I think a valid test would be to use a base image that has dotnet and/or node installed and just successfully run |
My understanding regarding this is that the Docker ENV should be respected by all tools, including VS Code (not quite sure how Docker bakes it into the container, though). Assuming that is the case, as long as we can prepend our shims path late enough that nothing else interferes, it should be OK.
Yeah, this is the tricky bit though as I've not figured out which file is used to determine PATH for all shells, including non-interactive. I thought it was
I think the location should be flexible, as long as we can correctly modify PATH. |
Ah, right. I may be able to do this by building a custom image and then using it in my Codespace which is currently broken (aliases don't work in VS Code, so C# tests fail).
Yup, that makes sense! Some edge-cases (e.g. ensuring the exact base binary is the one called, not another further down in PATH, etc.) could be added, but probably overcomplicated for our purposes. |
Ran into a hurdle where the change seems to work (with the feature bundled in the repo's |
3 participants
Should fix #55. This change does away with the aliases approach and instead installs shims which call the
ado-auth-helperand pass the result to the actual executable. To hijack calls to the actual executables, they need to be installed further up in PATH (as high as possible), to ensure that they have the highest priority when the command is run. This should be a more robust approach than aliases, which are tricky to forcibly source in non-interactive shells.The
resolve-shim.shhelper script finds the actual executable by usingwhichto enumerate all executables, and then using the 2nd (not 1st, because 1st is the shim script).TODO: