HaxeFoundation / hxcpp

Runtime files for c++ backend for haxe
Other
295 stars 184 forks source link

Use cwd when running `haxelib path` #1107

Closed tobil4sk closed 4 months ago

tobil4sk commented 4 months ago

If a local haxelib repo is being used, then moving out of the cwd will break library resolution.

This was presumably done in case haxelib is not in PATH, but in that situation HAXEPATH is unlikely to have been set either.

Fixes: https://github.com/HaxeFoundation/haxelib/issues/618

Simn commented 4 months ago

Shouldn't this use Sys.getEnv ("HAXEPATH") ?? "" instead? It might be unlikely that HAXEPATH is set, but it's still possible.

tobil4sk commented 4 months ago

It might be unlikely that HAXEPATH is set, but it's still possible.

The problem is that we don't want it to change directories, otherwise if there is a local repo in the project directory it will change directory and so the local repo won't get used, like in https://github.com/HaxeFoundation/haxelib/issues/618.

I think originally it changed directory to HAXEPATH in case haxelib isn't found in PATH. But what I was saying is that if haxelib is not found in PATH, then something probably went wrong in the installation process, so in that case HAXEPATH probably won't have been set anyway. So changing CWD to HAXEPATH doesn't seem to solve the problem it was trying to address, so it seem safe to change it to "" which will just keep it in the project directory. This way it respects the project's local repo.

Simn commented 4 months ago

Thanks for the explanation, makes sense!