Fix dependency and `subdir` in repoquery `whoneeds` #3743

Hind-M · 2025-01-14T10:10:49Z

Fix subdir in repoquery
Fix repoquery whoneeds no longer lists package version dependency #3717

Hind-M · 2025-01-15T15:33:36Z

libmamba/src/core/query.cpp

@@ -599,7 +592,7 @@ namespace mamba
        }

        std::vector<mamba::printers::FormattedString> headers;
-        std::vector<std::string_view> cmds, args;
+        std::vector<std::string> cmds, args;


This is the bug as cmds and args were refering to sfmt after it was out of scope, and this was only appearing in osx and windows...

Hind-M · 2025-01-15T15:35:54Z

libmamba/src/core/query.cpp

@@ -670,7 +663,7 @@ namespace mamba
                }
                else if (cmd == "Subdir")
                {
-                    row.emplace_back(get_subdir(pkg.channel));
+                    row.emplace_back(pkg.platform);


This was not tested and was returning the channel conda-forge instead of the subdir.

Hind-M · 2025-01-15T15:38:24Z

micromamba/tests/test_repoquery.py

+    res = helpers.umamba_repoquery("whoneeds", "-c", "conda-forge", spec, "--platform", "osx-64")
+    assert "Name Version Build Depends Channel Subdir" in helpers.remove_whitespaces(res)
+    if spec == "xtensor=0.24.5":  # `Depends` column is empty here (bug?)
+        assert "cascade 0.1.1 py38h5ce3968_0 conda-forge osx-64" in helpers.remove_whitespaces(res)


Not sure if the current behavior in this case (specifying the spec version) is the appropriate one (need to look into it more with maybe more context).

The first builds of cascade (i.e. the ones of 0.1.1) depend on xtensor, but not on xtensor==0.24.5, here is one of their index.json:

{ "arch": "x86_64", "build": "py310ha6c322b_0", "build_number": 0, "depends": [ "boost-cpp >=1.78.0,<1.79.0a0", "fmt >=9.1.0,<10.0a0", "hdf5 >=1.12.2,<1.12.3.0a0", "heyoka >=0.21.0", "heyoka >=0.21.0,<0.22.0a0", "heyoka.py >=0.21.0", "libcxx >=14.0.6", "numpy", "pybind11-abi 4", "python >=3.10,<3.11.0a0", "python_abi 3.10.* *_cp310", "spdlog >=1.11.0,<1.12.0a0", "tbb >=2021.8.0", "xtensor", "xtensor-blas" ], "license": "MPL-2.0", "name": "cascade", "platform": "osx", "subdir": "osx-64", "timestamp": 1677367283346, "version": "0.1.1" }

Probably those builds of cascade have been done against xtensor==0.24.5.

I think xtensor should be reported in this case, as it is the case with:

micromamba repoquery whoneeds xtensor -c conda-forge --platform osx-64 | grep cascade

cascade 0.1.1 py38h5ce3968_0 xtensor conda-forge conda-forge cascade 0.1.1 py310ha6c322b_0 xtensor conda-forge conda-forge cascade 0.1.1 py39hb1d9d96_0 xtensor conda-forge conda-forge

What do you think?

micromamba/tests/test_repoquery.py

jjerphan · 2025-01-17T09:38:31Z

libmamba/src/core/query.cpp

@@ -621,7 +614,7 @@ namespace mamba
            else if (col.find_first_of(":") == col.npos)
            {
                headers.emplace_back(col);


Could the current code have problem as well? How about?

Suggested change

headers.emplace_back(col);

headers.emplace_back(std::string(col));

Converting col to std::string in cmds.push_back(std::string(col)); is due to changing cmds to std::vector<std::string> (instead of std::vector<std::string_view>).
But it's not needed in headers.emplace_back(col); as headers(std::vector<mamba::printers::FormattedString>) already have a constructor accepting std::string_view.

Sorry, my remark was not clear.

I was wondering whether we could also have this problem for all vectors referencing std::string_views directly or indirectly, i.e. could we have the problem with headers again here if the original std::string of std::string_view are freed?

Since headers is a vector of a struct (FormattedString) containing a string and not a string_view, it should be okay.
The only string_views pointing to a local string being out of scope later (sfmt) were cmds and args which were modified to string now.
So seems ok so far.

jjerphan · 2025-01-17T09:50:54Z

micromamba/tests/test_repoquery.py

+    res = helpers.umamba_repoquery("whoneeds", "-c", "conda-forge", spec, "--platform", "osx-64")
+    assert "Name Version Build Depends Channel Subdir" in helpers.remove_whitespaces(res)
+    if spec == "xtensor=0.24.5":  # `Depends` column is empty here (bug?)
+        assert "cascade 0.1.1 py38h5ce3968_0 conda-forge osx-64" in helpers.remove_whitespaces(res)


The first builds of cascade (i.e. the ones of 0.1.1) depend on xtensor, but not on xtensor==0.24.5, here is one of their index.json:

{ "arch": "x86_64", "build": "py310ha6c322b_0", "build_number": 0, "depends": [ "boost-cpp >=1.78.0,<1.79.0a0", "fmt >=9.1.0,<10.0a0", "hdf5 >=1.12.2,<1.12.3.0a0", "heyoka >=0.21.0", "heyoka >=0.21.0,<0.22.0a0", "heyoka.py >=0.21.0", "libcxx >=14.0.6", "numpy", "pybind11-abi 4", "python >=3.10,<3.11.0a0", "python_abi 3.10.* *_cp310", "spdlog >=1.11.0,<1.12.0a0", "tbb >=2021.8.0", "xtensor", "xtensor-blas" ], "license": "MPL-2.0", "name": "cascade", "platform": "osx", "subdir": "osx-64", "timestamp": 1677367283346, "version": "0.1.1" }

Probably those builds of cascade have been done against xtensor==0.24.5.

I think xtensor should be reported in this case, as it is the case with:

micromamba repoquery whoneeds xtensor -c conda-forge --platform osx-64 | grep cascade

cascade 0.1.1 py38h5ce3968_0 xtensor conda-forge conda-forge cascade 0.1.1 py310ha6c322b_0 xtensor conda-forge conda-forge cascade 0.1.1 py39hb1d9d96_0 xtensor conda-forge conda-forge

What do you think?

Fix subdir in repoquery

bb9bcd9

Hind-M added the release::bug_fixes For PRs fixing bugs label Jan 14, 2025

Use string instead of string_view

49c124f

Hind-M force-pushed the whoneeds_output branch from 6d218d8 to 49c124f Compare January 15, 2025 15:26

Hind-M changed the title ~~Fix subdir in repoquery~~ Fix dependency and subdir in repoquery whoneeds Jan 15, 2025

Hind-M commented Jan 15, 2025

View reviewed changes

Hind-M marked this pull request as ready for review January 16, 2025 10:34

jjerphan reviewed Jan 17, 2025

View reviewed changes

Code review: use var to store res transformation

04e78bd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dependency and `subdir` in repoquery `whoneeds` #3743

Fix dependency and `subdir` in repoquery `whoneeds` #3743

Hind-M commented Jan 14, 2025 •

edited

Loading

Hind-M Jan 15, 2025

Hind-M Jan 15, 2025

Hind-M Jan 15, 2025

jjerphan Jan 17, 2025 •

edited

Loading

jjerphan Jan 17, 2025

Hind-M Jan 17, 2025

jjerphan Jan 17, 2025

Hind-M Jan 17, 2025

jjerphan Jan 17, 2025 •

edited

Loading

	headers.emplace_back(col);
	headers.emplace_back(std::string(col));

Fix dependency and subdir in repoquery whoneeds #3743

Are you sure you want to change the base?

Fix dependency and subdir in repoquery whoneeds #3743

Conversation

Hind-M commented Jan 14, 2025 • edited Loading

Hind-M Jan 15, 2025

Choose a reason for hiding this comment

Hind-M Jan 15, 2025

Choose a reason for hiding this comment

Hind-M Jan 15, 2025

Choose a reason for hiding this comment

jjerphan Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

jjerphan Jan 17, 2025

Choose a reason for hiding this comment

Hind-M Jan 17, 2025

Choose a reason for hiding this comment

jjerphan Jan 17, 2025

Choose a reason for hiding this comment

Hind-M Jan 17, 2025

Choose a reason for hiding this comment

jjerphan Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Fix dependency and `subdir` in repoquery `whoneeds` #3743

Fix dependency and `subdir` in repoquery `whoneeds` #3743

Hind-M commented Jan 14, 2025 •

edited

Loading

jjerphan Jan 17, 2025 •

edited

Loading

jjerphan Jan 17, 2025 •

edited

Loading