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

Adding ly_ctx_add_parsed_module API #2187

Closed

Conversation

steweg
Copy link
Contributor

@steweg steweg commented Feb 26, 2024

This patch adds possiblity for user to add manually created parsed module into context.

This patch adds possiblity for user to add manually created parsed
module into context.
@michalvasko
Copy link
Member

What is the purpose of the new function? Why not simply use ly_ctx_load_module() or lys_set_implemented()?

@steweg
Copy link
Contributor Author

steweg commented Feb 26, 2024

Background:

  • I need to be able to create YANG module on-the-fly and use it for many things within my project
  • This YANG module dynamically combine multiple existing YANG modules into one big based on certain logic (just the parsed part).
  • To do that outside of libyang it means:
    • to perform complete YANG parsing logic on existing YANG files and put it in some common structure
    • creating new module with modified structures
    • exporting it as YANG file (or alternatively libyang supported format)
    • import it back into libyang as regular module
  • So almost everything is already available in libyang, just injecting of parsed module is missing

Coming back to your question: So ly_ctx_load_module() is not ok for me as it expects that the input will be in YANG format, which I cannot really easily produce outside of libyang.

And lys_set_implement() is also not ok, as I believe it requires parsed module to be already part of context.

I need something in the middle. Do you agree or see some other option how to do that?

@michalvasko
Copy link
Member

Sorry, I have misunderstood the functionality and also did not expect the test actually demonstrates common usage. So, you have a really specific, perhaps unique, use-case and I am not sure it should be supported this way. Even manipulating parsed node structures is not expected, the modules are always returned as const (maybe not always for convenience, but they should definitely not be modified by hand, only by other libyang functions).

TLDR; This PR is the only way you can support your use-case? Even constructing the module in YANG first and then parsing it seems better to me, modifying an existing context by manually relinking parsed nodes is a dangerous thing to do.

@steweg
Copy link
Contributor Author

steweg commented Feb 26, 2024

To support my use-case, I need 2 things in total:

  1. Is this PR to allow adding custom parsed modules
  2. For modules imports crafting, it is needed to somehow allow to use LY_ARRAY_NEW_RET & LY_ARRAY_FREE macros by libyang-python. Macros are not directly callable in python, so I need to wrap it in some C function. It can be either in libyang or within libyang-python. I have patch for libyang-python like this:
LY_ERR
lypy_array_new(struct ly_ctx *ctx, size_t item_size, void **array, void **new_item)
{
    char (*item)[item_size];
    char (*array2)[item_size] = (char (*)[item_size])(*array);
    LY_ARRAY_NEW_RET(ctx, array2, item, LY_EMEM);
    if (new_item) {
        *new_item = item;
    }

    *array = (void *)array2;
    return LY_SUCCESS;
}

void
lypy_array_free(void *array)
{
    LY_ARRAY_FREE(array);
}

So if you would prefer, I can shift this code into libyang directly. Just let me know

I fully understood and agree that playing with nodes, which are part of some existing modules, is very risky thing, so I am doing just minimal steps to achieve the needed results as:

  1. creating of new fake parsed schema tree module structure
  2. creating new top level lysp_node_grp in each real module i need to use (grouping doesn't really have compiled version so no problem with compilation nor data)
  3. adding the needed real module lysp_nodes into this grouping as childs (but not change parent on them)
  4. creating lysp_import of real module inside fake module
  5. add uses nodes into fake module schema tree where needed
  6. import, implement and compile fake module

Assumption is that none of the xpath nodes (when, must, path..) in those modules will use absolute paths.

So effectively my fake module will after compilation reuse as much as possible from existing modules and "compiled" tree of my fake module, is kind of totally independent.

Alternative would be to copy lysp_node* tree from one module to another, which is much bigger job, as it is needed to also manage dependencies of imports/submodules etc... Another alternative is to do this kind of changes totally outside of libyang, which even more complicated due to additional already mentioned parsing & printing steps.

@michalvasko
Copy link
Member

michalvasko commented Feb 26, 2024

The cause of the requirement in 2. is intentional meaning those macros are not available on purpose. You are trying to perform operations that are supposed to be internal only. We have kept almost all the structures public so that even more invasive use-cases are supported but not encouraged or officially supported (meaning there are no dedicated API functions). That is why I am against adding ly_ctx_add_parsed_module() or making those macros public, it would just enlarge the overall provided functionality which is already extensive and difficult to maintain.

So, I am sorry, but unless you can come up with some smaller changes that could be beneficial in general, not just for your use-case, you will have to implement all of this outside libyang. You can even use internal headers and it should not really take that much more effort but it will remain up to you to maintain it.

@jktjkt
Copy link
Contributor

jktjkt commented Feb 26, 2024

Once you have a lys_module, you can use libyang to print it out into a YANG-formatted text, and then load that new YANG module into another ly_ctx as usual. Granted, you'll "waste" one roundtrip of printing and parsing, but hopefully your hot path doesn't involve producing YANG modules. Would that work for you, or am I missing something?

@steweg
Copy link
Contributor Author

steweg commented Feb 27, 2024

Ok, thanks both for the suggestion, I have adjust the code on my side based on your suggestions and it seems working. Cancelling this PR

@steweg steweg closed this Feb 27, 2024
@steweg steweg deleted the feature/ly_ctx_add_parsed_module_devel branch February 27, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants