-
Notifications
You must be signed in to change notification settings - Fork 994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Fish support for environments #15503
Changes from all commits
5f19714
bf5d21e
b249c6d
e16bf22
d42b388
269b4c8
e703b07
88a48b4
c60eb5a
1ab3a8d
70c34ae
37eab05
e32b420
8cdfd29
966c631
dd517c6
325fe06
2d202ed
66ca6a2
f57f32f
4535b74
32125de
cd94ce3
35d682e
d55eff5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -166,6 +166,7 @@ def deactivates(filenames): | |
bats = [] | ||
shs = [] | ||
ps1s = [] | ||
fishs = [] | ||
for env_script in env_scripts: | ||
path = os.path.join(conanfile.generators_folder, env_script) | ||
# Only the .bat and .ps1 are made relative to current script | ||
|
@@ -174,6 +175,9 @@ def deactivates(filenames): | |
bats.append("%~dp0/"+path) | ||
elif env_script.endswith(".sh"): | ||
shs.append(subsystem_path(subsystem, path)) | ||
elif env_script.endswith(".fish"): | ||
path = os.path.abspath(path) | ||
fishs.append(path) | ||
elif env_script.endswith(".ps1"): | ||
path = os.path.relpath(path, conanfile.generators_folder) | ||
# This $PSScriptRoot uses the current script directory | ||
|
@@ -186,6 +190,14 @@ def sh_content(files): | |
save(os.path.join(conanfile.generators_folder, filename), sh_content(shs)) | ||
save(os.path.join(conanfile.generators_folder, "deactivate_{}".format(filename)), | ||
sh_content(deactivates(shs))) | ||
if fishs: | ||
def fish_content(files): | ||
return ". " + " && . ".join('"{}"'.format(s) for s in files) | ||
fishs = [it for it in fishs if os.path.exists(it)] | ||
if fishs: | ||
filename = "conan{}.fish".format(group) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here it seems it is missing the deactivation function (not script)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually the deactivation is not an script, but a function exported by Fish in the environment. So we can call the deactivate_conanbuild, but there is no file associated. The function is created on-the-fly: https://github.com/conan-io/conan/pull/15503/files#diff-56b36c23589a66b0bd803c545703684c74400cbb7bd8538b8949ed84613e3fc7R550 |
||
generated.append(filename) | ||
save(os.path.join(conanfile.generators_folder, filename), fish_content(fishs)) | ||
if bats: | ||
def bat_content(files): | ||
return "\r\n".join(["@echo off"] + ['call "{}"'.format(b) for b in files]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the function_name is this one? Why not just the
group
with the random chars? Is there risk of collision ofdeactivate_xxxx
function name?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It follow the common name used by other virtualenv like:
conanbuild.sh
->deactivate_conanbuild.sh
. So you can actually can call from your terminal directly, like:In Fish, functions are exported as a command, so you can run directly.
About the random to avoid collision: In case we have multiple dependencies add bin folder to PATH, we will need to undo the configuration after deactivating it. Usually it would be store in a deactivate script, but here we are using a variable. Actually, the random is just an idea, but could be the build folder name too, because is kind unique name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, having something deterministic would make more sense, I probably still didn't understand the issue.