OCI/Conda mapping #3310
OCI/Conda mapping #3310
Conversation
Hind-M
added
the
release::enhancements
| @@ -71,7 +71,7 @@ namespace mamba::download | |||
| * OCIMirror implementation * | |||
| ****************************/ | |||
|
|
|||
| namespace | |||
| namespace utils | |||
There was a problem hiding this comment.
Why turning it into a non anonymous namespace? Did you have conflict with functions defined in another namespace?
There was a problem hiding this comment.
To have access in the tests.
There was a problem hiding this comment.
In that case I think it would be worth having a preprocessor token defining the namespace to util if we build the test, and emtpy if not. Otherwise this symbol (and the other ones included in this namespace) will be exported and visible from other libraries, which is something we wanted to avoid with the anonymous namespace.
@Klaim any thought on this?
There was a problem hiding this comment.
You are correct that naming the scope makes the symbols visible, anonymous namespaces are essentially similar to marking everything static (and more). So this is not a small change.
For testing internal functions there are various usual strategies:
- making the internal functions visible (like here);
- tests link with a "utility library" version of the library, which is essentially like a static library except it's not removing unused functions at linking;
- real proper "unit tests" as in test source file which is specific to that component and runs as a single executable, testing only that sub-component;
- the tests "include" the cpp they test (which can lead to ODR violations, depends on how both are coded);
From the current mamba organisation, looks like the first solution is the easiest to setup at the moment. Alternating the definition of the namespace might not work properly because this is an internal translation unit of libmamba, so you would have to build libmamba twice, once with the visible symbols, once with them hidden.
There was a problem hiding this comment.
Ok then let's keep it as is for now, if it clashes with other symbols when linking, we will simply change its name.
|
Did you do an end-to-end test yet? Because I was pretty confused that this ever worked before (since |
Please review #3307 first, as this PR is based on it. |
I added a check on mamba list output after creating the test env, and it's using |
|
@wolfv (When names are "reserved" it implies that the "implementation" is allowed to use them, they are reserved to avoid conflicts mainly, so I suppose EDIT: oh wait, you're talking in the url, I thought it was about the actual mutex, nevermind XD |
Remove dry-run and check result with umamba_list
Map packages names starting with
_to match OCI registries rules (and CEP)