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

New Environment model for recipes and profiles #8630

Merged
merged 35 commits into from Mar 17, 2021
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
75ff512
proposal
memsharded Feb 21, 2021
3a585e5
build-host contexts poc
memsharded Feb 23, 2021
5f323b1
added CMakeGen integration
memsharded Feb 24, 2021
16863a1
fixing Macos tests
memsharded Feb 24, 2021
3fd4633
review
memsharded Feb 24, 2021
de6787e
make sure profile compose and include() correctly
memsharded Feb 24, 2021
7f86beb
Merge branch 'develop' into feature/environment_propagate
memsharded Feb 24, 2021
a96287c
minor interfaces
memsharded Feb 25, 2021
004ab8b
accesing Node
memsharded Feb 26, 2021
3d318e3
fixing tests
memsharded Feb 26, 2021
247f98c
fix py27 test
memsharded Feb 26, 2021
997936b
trying with shell only, not bash
memsharded Mar 1, 2021
fa0ca3d
not necessary to escape quotes
memsharded Mar 1, 2021
e63890e
Merge branch 'develop' into feature/environment_propagate
memsharded Mar 1, 2021
1a0abdf
merged develop
memsharded Mar 1, 2021
1f66a6c
poc for self.run() env definition
memsharded Mar 3, 2021
f78169a
fix test
memsharded Mar 3, 2021
b6dcb07
renaming files
memsharded Mar 3, 2021
e3a3cf0
change default to opt-out
memsharded Mar 4, 2021
1482923
minor changes
memsharded Mar 4, 2021
1d42450
Merge branch 'develop' into feature/environment_propagate
memsharded Mar 5, 2021
935b3d0
more complete real test
memsharded Mar 6, 2021
15dbf07
fix test
memsharded Mar 7, 2021
4221d32
fix v2_cmake template for shared libs in OSX with run_environment=True
memsharded Mar 8, 2021
3798c78
Merge branch 'develop' into feature/environment_propagate
memsharded Mar 8, 2021
4dbe0c0
Merge branch 'develop' into feature/environment_propagate
memsharded Mar 9, 2021
e1bf328
improved v2_cmake new template with environment
memsharded Mar 9, 2021
c43ed63
cpp_info.exes do not exist yet
memsharded Mar 9, 2021
3198d92
fix tests
memsharded Mar 9, 2021
b84196a
cleaning
memsharded Mar 11, 2021
e3a4aa9
Merge branch 'develop' into feature/environment
memsharded Mar 11, 2021
2fc9228
just the data model
memsharded Mar 11, 2021
392a788
fixing test
memsharded Mar 11, 2021
1530a0f
review
memsharded Mar 11, 2021
da85931
removing unnecessary dict access
memsharded Mar 11, 2021
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
1 change: 0 additions & 1 deletion .gitignore
Expand Up @@ -7,7 +7,6 @@ __pycache__/

# Distribution / packaging
.Python
env/
build/
develop-eggs/
dist/
Expand Down
1 change: 1 addition & 0 deletions conan/tools/env/__init__.py
@@ -0,0 +1 @@
from conan.tools.env.environment import Environment
288 changes: 288 additions & 0 deletions conan/tools/env/environment.py
@@ -0,0 +1,288 @@
import fnmatch
import os
import textwrap
from collections import OrderedDict

from conans.errors import ConanException
from conans.util.files import save


class _EnvVarPlaceHolder:
pass


class _Sep(str):
pass


class _PathSep:
pass


def environment_wrap_command(filename, cmd, cwd=None):
assert filename
filenames = [filename] if not isinstance(filename, list) else filename
Copy link
Contributor

@SSE4 SSE4 Mar 11, 2021

Choose a reason for hiding this comment

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

Suggested change
filenames = [filename] if not isinstance(filename, list) else filename
try:
from collections.abc import Iterable
except ImportError:
from collections import Iterable
filenames = [filename] if not isinstance(filename, Iterable) else filename

Copy link
Contributor

Choose a reason for hiding this comment

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

We have conans.utils.misc.make_tuple that will work here (pay attention, a string is iterable)

Copy link
Member Author

Choose a reason for hiding this comment

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

See comment below, filename must be either a string value or a list. Other input types as sets, tuples, dicts, etc are not supported, and it will be documented as this.

bats, shs = [], []
for f in filenames:
full_path = os.path.join(cwd, f) if cwd else f
if os.path.isfile("{}.bat".format(full_path)):
bats.append("{}.bat".format(f))
elif os.path.isfile("{}.sh".format(full_path)):
shs.append("{}.sh".format(f))
Copy link
Contributor

Choose a reason for hiding this comment

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

should we raise for else here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is injecting the environment file if it is found, but the file might not always be there, for example if there are no environment variables to define, instead of generating a blank file, the file is not being generated. That is what allows to inject previously unknown build_requires into a recipe that will create such conanbuildenv.sh env file, that will be used if present, but it will not generate a blank conanbuildenv.sh file that will always be there for absolutely all packages, even if they are not using build_requires at all.

if bats and shs:
raise ConanException("Cannot wrap command with different envs, {} - {}".format(bats, shs))

if bats:
command = " && ".join(bats)
return "{} && {}".format(command, cmd)
elif shs:
command = " && ".join(". ./{}".format(f) for f in shs)
return "{} && {}".format(command, cmd)
else:
return cmd


class Environment:
def __init__(self):
# TODO: Maybe we need to pass conanfile to get the [conf]
# It being ordered allows for Windows case-insensitive composition
self._values = OrderedDict() # {var_name: [] of values, including separators}

def __bool__(self):
return bool(self._values)

__nonzero__ = __bool__

def __repr__(self):
return repr(self._values)

def vars(self):
return list(self._values.keys())

def value(self, name, placeholder="{name}", pathsep=os.pathsep):
return self._format_value(name, self._values[name], placeholder, pathsep)

@staticmethod
def _format_value(name, varvalues, placeholder, pathsep):
values = []
for v in varvalues:

if v is _EnvVarPlaceHolder:
values.append(placeholder.format(name=name))
elif v is _PathSep:
values.append(pathsep)
else:
values.append(v)
return "".join(values)

@staticmethod
def _list_value(value, separator):
if isinstance(value, list):
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to accept several different types of inputs, it should be either a value or a list. Passing an iterable, like a set, that is not ordered could produce unexpected results. Lets keep the interface as narrow as possible, there is always time to extend if we want.

result = []
for v in value[:-1]:
result.append(v)
result.append(separator)
result.extend(value[-1:])
return result
else:
return [value]

def define(self, name, value, separator=" "):
value = self._list_value(value, _Sep(separator))
self._values[name] = value

def define_path(self, name, value):
value = self._list_value(value, _PathSep)
self._values[name] = value

def unset(self, name):
"""
clears the variable, equivalent to a unset or set XXX=
"""
self._values[name] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that the environment variable is assigned an empty value or that it doesn't exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

The value exists in the Environment, but an empty list represents the need to unset it, actually removing it from the system environment with a unset VAR or set VAR= in Windows. It is a list, because an Environment.unset still can compose with a Environment.define or Environment.append, and it is more uniform than having a None value that would need to be handled differently.


def append(self, name, value, separator=" "):
value = self._list_value(value, _Sep(separator))
self._values[name] = [_EnvVarPlaceHolder] + [_Sep(separator)] + value

def append_path(self, name, value):
value = self._list_value(value, _PathSep)
self._values[name] = [_EnvVarPlaceHolder] + [_PathSep] + value

def prepend(self, name, value, separator=" "):
value = self._list_value(value, _Sep(separator))
self._values[name] = value + [_Sep(separator)] + [_EnvVarPlaceHolder]

def prepend_path(self, name, value):
value = self._list_value(value, _PathSep)
self._values[name] = value + [_PathSep] + [_EnvVarPlaceHolder]

def save_bat(self, filename, generate_deactivate=False, pathsep=os.pathsep):
deactivate = textwrap.dedent("""\
echo Capturing current environment in deactivate_{filename}
setlocal
echo @echo off > "deactivate_{filename}"
echo echo Restoring environment >> "deactivate_{filename}"
for %%v in ({vars}) do (
set foundenvvar=
for /f "delims== tokens=1,2" %%a in ('set') do (
if "%%a" == "%%v" (
echo set %%a=%%b>> "deactivate_{filename}"
set foundenvvar=1
)
)
if not defined foundenvvar (
echo set %%v=>> "deactivate_{filename}"
)
)
endlocal

""").format(filename=filename, vars=" ".join(self._values.keys()))
capture = textwrap.dedent("""\
@echo off
{deactivate}
echo Configuring environment variables
""").format(deactivate=deactivate if generate_deactivate else "")
result = [capture]
for varname, varvalues in self._values.items():
value = self._format_value(varname, varvalues, "%{name}%", pathsep)
result.append('set {}={}'.format(varname, value))

content = "\n".join(result)
save(filename, content)

def save_ps1(self, filename, generate_deactivate=False, pathsep=os.pathsep):
# FIXME: This is broken and doesnt work
Copy link
Contributor

Choose a reason for hiding this comment

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

what exactly is broken here? also it doesn't seem to be used anywhere at all

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole powershell implementation is not working neither tested. I need a PS1 expert to contribute it, maybe you or @solvingj can do something, but that can wait, it is not central to this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a TODO: ... and/or an exception is more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll be happy to work on the PS1 implementation when ready.

deactivate = ""
capture = textwrap.dedent("""\
{deactivate}
""").format(deactivate=deactivate if generate_deactivate else "")
result = [capture]
for varname, varvalues in self._values.items():
value = self._format_value(varname, varvalues, "$env:{name}", pathsep)
result.append('$env:{}={}'.format(varname, value))

content = "\n".join(result)
save(filename, content)

def save_sh(self, filename, generate_deactivate=False, pathsep=os.pathsep):
deactivate = textwrap.dedent("""\
echo Capturing current environment in deactivate_{filename}
echo echo Restoring variables >> deactivate_{filename}
for v in {vars}
do
value=$(printenv $v)
if [ -n "$value" ]
then
echo export "$v=$value" >> deactivate_{filename}
else
echo unset $v >> deactivate_{filename}
fi
done
echo Configuring environment variables
""".format(filename=filename, vars=" ".join(self._values.keys())))
capture = textwrap.dedent("""\
{deactivate}
echo Configuring environment variables
""").format(deactivate=deactivate if generate_deactivate else "")
result = [capture]
for varname, varvalues in self._values.items():
value = self._format_value(varname, varvalues, "${name}", pathsep)
if value:
result.append('export {}="{}"'.format(varname, value))
else:
result.append('unset {}'.format(varname))

content = "\n".join(result)
save(filename, content)

def compose(self, other):
"""
:type other: Environment
"""
for k, v in other._values.items():
existing = self._values.get(k)
if existing is None:
self._values[k] = v
else:
try:
index = v.index(_EnvVarPlaceHolder)
except ValueError: # The other doesn't have placeholder, overwrites
self._values[k] = v
else:
new_value = v[:] # do a copy
new_value[index:index + 1] = existing # replace the placeholder
# Trim front and back separators
val = new_value[0]
if isinstance(val, _Sep) or val is _PathSep:
new_value = new_value[1:]
val = new_value[-1]
if isinstance(val, _Sep) or val is _PathSep:
new_value = new_value[:-1]
self._values[k] = new_value
return self


class ProfileEnvironment:
def __init__(self):
self._environments = OrderedDict()

def __repr__(self):
return repr(self._environments)

def get_env(self, ref):
""" computes package-specific Environment
it is only called when conanfile.buildenv is called
"""
result = Environment()
for pattern, env in self._environments.items():
if pattern is None or fnmatch.fnmatch(str(ref), pattern):
env = self._environments[pattern]
memsharded marked this conversation as resolved.
Show resolved Hide resolved
result = result.compose(env)
return result

def compose(self, other):
"""
:type other: ProfileEnvironment
"""
for pattern, environment in other._environments.items():
existing = self._environments.get(pattern)
if existing is not None:
self._environments[pattern] = existing.compose(environment)
else:
self._environments[pattern] = environment

def loads(self, text):
for line in text.splitlines():
line = line.strip()
if not line or line.startswith("#"):
continue
for op, method in (("+=", "append"), ("=+", "prepend"),
("=!", "unset"), ("=", "define")):
tokens = line.split(op, 1)
if len(tokens) != 2:
continue
pattern_name, value = tokens
pattern_name = pattern_name.split(":", 1)
if len(pattern_name) == 2:
pattern, name = pattern_name
else:
pattern, name = None, pattern_name[0]

env = Environment()
if method == "unset":
env.unset(name)
else:
if value.startswith("(path)"):
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this magic keyword, but I understand it is convenient to inform Conan it is a path... have we considered any other alternative?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried a few alternatives, like different operators, but this was the most readable and intuitive and easy to parse at the same time, I think it is difficult to beat this, but suggestions welcome:

# define
MyPath1=(path)/my/path1
#  append
MyPath2+=(path)/my/path2

Copy link
Contributor

Choose a reason for hiding this comment

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

My concern is that (path) is valid for a path name... and if something can happen, it will happen 😄

⇒  pwd
/Users/jgsogo/dev/conan/tmp/path(weird/(path)porqueyolovalgo
[py38]jgsogo@Javierg:~/dev/conan/tmp/path(weird/(path)porqueyolovalgo|

maybe we need to rely on comments?

# define
MyPath1=/my/path1  # path, type:path
#  append
MyPath2+=/my/path2   # path, type:path,...

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that there might be env-vars that need to contain a # too, so lines cannot contain comments either.

Strictly speaking, the only character that cannot be an env-var name is =. Even the += or =! could be colliding with values or variable names containing those special characters.
But in practice, POSIX shells only allow alphanumeric identifiers, so it is relatively extended practice.

As the environment variable values can contain any arbitrary value, then there is by definition impossible to have any VAR OPERATION VALUE definition that implements the different logic operations and completely avoid all possible collisions. We would need a structured format, json, xml, or equivalent, but I doubt we want to go that way.

If we are concerned about (path) colliding, the best action could be to make the token more specific, something like var+=(conanpath!)/path/to/something

value = value[6:]
method = method + "_path"
getattr(env, method)(name, value)

existing = self._environments.get(pattern)
if existing is None:
self._environments[pattern] = env
else:
self._environments[pattern] = existing.compose(env)
break
else:
raise ConanException("Bad env defintion: {}".format(line))
2 changes: 1 addition & 1 deletion conans/assets/templates/new_v2_cmake.py
Expand Up @@ -84,7 +84,7 @@ def test(self):
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_MINSIZEREL ${{CMAKE_RUNTIME_OUTPUT_DIRECTORY}})
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${{CMAKE_RUNTIME_OUTPUT_DIRECTORY}})

find_package({name})
find_package({name} CONFIG REQUIRED)
memsharded marked this conversation as resolved.
Show resolved Hide resolved

add_executable(example example.cpp)
target_link_libraries(example {name}::{name})
Expand Down
4 changes: 2 additions & 2 deletions conans/client/generators/__init__.py
Expand Up @@ -89,7 +89,7 @@ def _new_generator(self, generator_name, output):
if generator_name == "CMakeToolchain":
from conan.tools.cmake import CMakeToolchain
return CMakeToolchain
if generator_name == "CMakeDeps":
elif generator_name == "CMakeDeps":
from conan.tools.cmake import CMakeDeps
return CMakeDeps
elif generator_name == "MakeToolchain":
Expand Down Expand Up @@ -196,4 +196,4 @@ def write_toolchain(conanfile, path, output):
with conanfile_exception_formatter(str(conanfile), "generate"):
conanfile.generate()

# TODO: Lets discuss what to do with the environment
# TODO: Lets discuss what to do with the environment
6 changes: 3 additions & 3 deletions conans/client/loader.py
Expand Up @@ -193,7 +193,7 @@ def _initialize_conanfile(conanfile, profile):
if pkg_settings:
tmp_settings.update_values(pkg_settings)

conanfile.initialize(tmp_settings, profile.env_values)
conanfile.initialize(tmp_settings, profile.env_values, profile.buildenv)
conanfile.conf = profile.conf.get_conanfile_conf(ref_str)

def load_consumer(self, conanfile_path, profile_host, name=None, version=None, user=None,
Expand Down Expand Up @@ -262,7 +262,7 @@ def load_conanfile_txt(self, conan_txt_path, profile_host, ref=None):

def _parse_conan_txt(self, contents, path, display_name, profile):
conanfile = ConanFile(self._output, self._runner, display_name)
conanfile.initialize(Settings(), profile.env_values)
conanfile.initialize(Settings(), profile.env_values, profile.buildenv)
conanfile.conf = profile.conf.get_conanfile_conf(None)
# It is necessary to copy the settings, because the above is only a constraint of
# conanfile settings, and a txt doesn't define settings. Necessary for generators,
Expand Down Expand Up @@ -302,7 +302,7 @@ def load_virtual(self, references, profile_host, scope_options=True,
# for the reference (keep compatibility)
conanfile = ConanFile(self._output, self._runner, display_name="virtual")
conanfile.initialize(profile_host.processed_settings.copy(),
profile_host.env_values)
profile_host.env_values, profile_host.buildenv)
conanfile.conf = profile_host.conf.get_conanfile_conf(None)
conanfile.settings = profile_host.processed_settings.copy_values()

Expand Down
9 changes: 8 additions & 1 deletion conans/client/profile_loader.py
@@ -1,6 +1,7 @@
import os
from collections import OrderedDict, defaultdict

from conan.tools.env.environment import ProfileEnvironment
from conans.errors import ConanException, ConanV2Exception
from conans.model.conf import ConfDefinition
from conans.model.env_info import EnvValues, unquote
Expand Down Expand Up @@ -149,7 +150,8 @@ def _load_profile(text, profile_path, default_folder):

# Current profile before update with parents (but parent variables already applied)
doc = ConfigParser(profile_parser.profile_text,
allowed_fields=["build_requires", "settings", "env", "options", "conf"])
allowed_fields=["build_requires", "settings", "env", "options", "conf",
"buildenv"])

# Merge the inherited profile with the readed from current profile
_apply_inner_profile(doc, inherited_profile)
Expand Down Expand Up @@ -224,6 +226,11 @@ def get_package_name_value(item):
new_prof.loads(doc.conf, profile=True)
base_profile.conf.update_conf_definition(new_prof)

if doc.buildenv:
buildenv = ProfileEnvironment()
buildenv.loads(doc.buildenv)
base_profile.buildenv.compose(buildenv)


def profile_from_args(profiles, settings, options, env, cwd, cache):
""" Return a Profile object, as the result of merging a potentially existing Profile
Expand Down