Skip to content
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

Open
wants to merge 22 commits into
base: develop2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 50 additions & 1 deletion conan/tools/env/environment.py
Expand Up @@ -508,12 +508,52 @@ def save_sh(self, file_location, generate_deactivate=True):
content = f'script_folder="{os.path.abspath(filepath)}"\n' + content
save(file_location, content)

def save_fish(self, file_location, generate_deactivate=True):
filepath, filename = os.path.split(file_location)
deactivate_file = os.path.join(filepath, "deactivate_{}".format(filename))
values = self._values.keys()
if len(values) == 0:
# Empty environment, nothing to restore (Easier to handle in Fish)
deactivate = ("""echo "echo Nothing to restore" > {deactivate_file}"""
.format(deactivate_file=deactivate_file))
else:
deactivate = textwrap.dedent("""\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For new one, I'd try to do directly an in-memory or in-tmp-folder deactivate script rather than a local generated one.

echo "echo Restoring environment" > "{deactivate_file}"
for v in {vars}
set is_defined "true"
set value (printenv $v); or set is_defined ""
if test -n "$value" -o -n "$is_defined"
echo set -gx "$v" "$value" >> "{deactivate_file}"
else
echo set -ge "$v" >> "{deactivate_file}"
end
end
""".format(deactivate_file=deactivate_file, vars=" ".join(self._values.keys())))
capture = textwrap.dedent("""\
{deactivate}
""").format(deactivate=deactivate if generate_deactivate else "")
result = [capture]
for varname, varvalues in self._values.items():
value = varvalues.get_str("${name}", self._subsystem, pathsep=self._pathsep)
value = value.replace('"', '\\"')
if value:
result.append('set -gx {} "{}"'.format(varname, value))
else:
result.append('set -ge {}'.format(varname))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it could be simplified by using set -p (prepend), instead of expanding all values.

https://fishshell.com/docs/current/cmds/set.html


content = "\n".join(result)
content = relativize_generated_file(content, self._conanfile, "$script_folder")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to not use this and relativize paths as the other environment do?

content = f'set script_folder "{os.path.abspath(filepath)}"\n' + content
save(file_location, content)

def save_script(self, filename):
"""
Saves a script file (bat, sh, ps1) with a launcher to set the environment.
If the conf "tools.env.virtualenv:powershell" is set to True it will generate powershell
launchers if Windows.

If the conf "tools.env.virtualenv:fish" is set to True it will generate fish launchers.

:param filename: Name of the file to generate. If the extension is provided, it will generate
the launcher script for that extension, otherwise the format will be deduced
checking if we are running inside Windows (checking also the subsystem) or not.
Expand All @@ -522,18 +562,27 @@ def save_script(self, filename):
if ext:
is_bat = ext == ".bat"
is_ps1 = ext == ".ps1"
is_fish = ext == ".fish"
else: # Need to deduce it automatically
is_bat = self._subsystem == WINDOWS
is_ps1 = self._conanfile.conf.get("tools.env.virtualenv:powershell", check_type=bool)
if is_ps1:
is_fish = self._conanfile.conf.get("tools.env.virtualenv:fish", check_type=bool)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no wrappers for .fish files, are they? If there are no wrappers, this will break when set and used to build any dependency.
This should most likely apply only to end-consumer conanfile, not cache dependencies, and only if not used for conan build or anything like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, please, take a look in the test test_transitive_tool_requires. It generates a tool requirement, then a second recipe consumes it and runs an executable that prints an environment variable. All using Fish.

if is_fish:
filename = filename + ".fish"
is_bat = False
is_ps1 = False
elif is_ps1:
filename = filename + ".ps1"
is_bat = False
is_fish = False
else:
filename = filename + (".bat" if is_bat else ".sh")

path = os.path.join(self._conanfile.generators_folder, filename)
if is_bat:
self.save_bat(path)
elif is_fish:
self.save_fish(path)
elif is_ps1:
self.save_ps1(path)
else:
Expand Down
1 change: 1 addition & 0 deletions conans/model/conf.py
Expand Up @@ -104,6 +104,7 @@
"tools.apple:enable_arc": "(boolean) Enable/Disable ARC Apple Clang flags",
"tools.apple:enable_visibility": "(boolean) Enable/Disable Visibility Apple Clang flags",
"tools.env.virtualenv:powershell": "If it is set to True it will generate powershell launchers if os=Windows",
"tools.env.virtualenv:fish": "If it is set to True it will generate fish launchers",
# Compilers/Flags configurations
"tools.build:compiler_executables": "Defines a Python dict-like with the compilers path to be used. Allowed keys {'c', 'cpp', 'cuda', 'objc', 'objcxx', 'rc', 'fortran', 'asm', 'hip', 'ispc'}",
"tools.build:cxxflags": "List of extra CXX flags used by different toolchains like CMakeToolchain, AutotoolsToolchain and MesonToolchain",
Expand Down