diff --git a/dev/environment-dev.yml b/dev/environment-dev.yml index 7a6eaf17a2..a579eb7991 100644 --- a/dev/environment-dev.yml +++ b/dev/environment-dev.yml @@ -31,6 +31,7 @@ dependencies: - pytest-asyncio - pytest-timeout - pytest-xprocess + - memory_profiler - requests - sel(win): pywin32 - sel(win): menuinst diff --git a/libmamba/include/mamba/util/string.hpp b/libmamba/include/mamba/util/string.hpp index 17e94cc992..66ead50dd9 100644 --- a/libmamba/include/mamba/util/string.hpp +++ b/libmamba/include/mamba/util/string.hpp @@ -197,6 +197,11 @@ namespace mamba::util [[nodiscard]] auto strip(std::string_view input) -> std::string_view; [[nodiscard]] auto strip(std::wstring_view input) -> std::wstring_view; + /** + * Dedicated implementation for inplace stripping of `std::string` to avoid copies + */ + void inplace_strip(std::string& input); + [[nodiscard]] auto strip_parts(std::string_view input, char c) -> std::array; [[nodiscard]] auto strip_parts(std::wstring_view input, wchar_t c) -> std::array; diff --git a/libmamba/src/api/install.cpp b/libmamba/src/api/install.cpp index d6ebca5033..8844c14bb3 100644 --- a/libmamba/src/api/install.cpp +++ b/libmamba/src/api/install.cpp @@ -888,6 +888,8 @@ namespace mamba { throw std::runtime_error(util::concat("Got an empty file: ", file)); } + + // Inferring potential explicit environment specification for (std::size_t i = 0; i < file_contents.size(); ++i) { auto& line = file_contents[i]; @@ -930,26 +932,23 @@ namespace mamba } } - std::vector f_specs; - for (auto& line : file_contents) - { - if (line[0] != '#' && line[0] != '@') - { - f_specs.push_back(line); - } - } - + // If we reach here, we have a file with no explicit env, and the content of the + // file just lists MatchSpecs. if (specs.cli_configured()) { auto current_specs = specs.cli_value>(); - current_specs.insert(current_specs.end(), f_specs.cbegin(), f_specs.cend()); + current_specs.insert( + current_specs.end(), + file_contents.cbegin(), + file_contents.cend() + ); specs.set_cli_value(current_specs); } else { - if (!f_specs.empty()) + if (!file_contents.empty()) { - specs.set_cli_value(f_specs); + specs.set_cli_value(file_contents); } } } diff --git a/libmamba/src/core/util.cpp b/libmamba/src/core/util.cpp index 2f8c7c5197..23f6af6906 100644 --- a/libmamba/src/core/util.cpp +++ b/libmamba/src/core/util.cpp @@ -301,6 +301,34 @@ namespace mamba line.pop_back(); } + // Remove leading and trailing whitespace in place not to create a new string. + util::inplace_strip(line); + + // Skipping empty lines + if (line.empty()) + { + continue; + } + + // Skipping comment lines starting with # + if (util::starts_with(line, "#")) + { + continue; + } + + // Skipping comment lines starting with @ BUT headers of explicit environment specs + if (util::starts_with(line, "@")) + { + auto is_explicit_header = util::starts_with(line, "@EXPLICIT"); + + if (is_explicit_header) + { + output.push_back(line); + } + continue; + } + + // By default, add the line to the output (MatchSpecs, etc.) output.push_back(line); } file_stream.close(); diff --git a/libmamba/src/util/string.cpp b/libmamba/src/util/string.cpp index d6d78ab920..28c9cf14c2 100644 --- a/libmamba/src/util/string.cpp +++ b/libmamba/src/util/string.cpp @@ -447,6 +447,26 @@ namespace mamba::util return strip_if(input, [](Char c) { return !is_graphic(c); }); } + void inplace_strip(std::string& input) + { + if (input.empty()) + { + return; + } + + const auto start = input.find_first_not_of(" \t\n\v\f\r"); + + if (start == std::string::npos) + { + input.clear(); + return; + } + + const auto end = input.find_last_not_of(" \t\n\v\f\r"); + + input = input.substr(start, end - start + 1); + } + /********************************************* * Implementation of strip_parts functions * *********************************************/ diff --git a/micromamba/tests/test_create.py b/micromamba/tests/test_create.py index 76d698ce82..18b92c3b1b 100644 --- a/micromamba/tests/test_create.py +++ b/micromamba/tests/test_create.py @@ -9,6 +9,8 @@ from . import helpers +from memory_profiler import memory_usage + __this_dir__ = Path(__file__).parent.resolve() env_file_requires_pip_install_path = __this_dir__ / "env-requires-pip-install.yaml" @@ -1441,3 +1443,55 @@ def test_create_empty_or_absent_dependencies(tmp_path): "-p", env_prefix, "-f", tmp_path / "env_spec_absent_dependencies.yaml", "--json" ) assert res["success"] + + +env_spec_empty_lines_and_comments = """ +# The line below are empty (various number of spaces) +""" + +env_spec_empty_lines_and_comments += "\n" +env_spec_empty_lines_and_comments += " \n" +env_spec_empty_lines_and_comments += " \n" +env_spec_empty_lines_and_comments += " \n" +env_spec_empty_lines_and_comments += "# One comment \n" +env_spec_empty_lines_and_comments += " @ yet another one with a prefixed by a tab\n" +env_spec_empty_lines_and_comments += "wheel\n" + +env_repro_1 = """ +wheel + +setuptools +""" + +env_repro_2 = """ +wheel +setuptools + +# comment +""" + + +@pytest.mark.parametrize("env_spec", [env_spec_empty_lines_and_comments, env_repro_1, env_repro_2]) +def test_create_with_empty_lines_and_comments(tmp_path, env_spec): + # Non-regression test for: + # - https://github.com/mamba-org/mamba/issues/3289 + # - https://github.com/mamba-org/mamba/issues/3659 + memory_limit = 100 # in MB + + def memory_intensive_operation(): + env_prefix = tmp_path / "env-one_empty_line" + + env_spec_file = tmp_path / "env_spec.txt" + + with open(env_spec_file, "w") as f: + f.write(env_spec) + + res = helpers.create("-p", env_prefix, "-f", env_spec_file, "--json") + assert res["success"] + + max_memory = max(memory_usage(proc=memory_intensive_operation)) + + if max_memory > memory_limit: + pytest.fail( + f"test_create_with_empty_lines_and_comments exceeded memory limit of {memory_limit} MB (used {max_memory:.2f} MB)" + )