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

hy-util: support missing outvars in hy_split_nevra and add hy_parse_module_spec #1206

Conversation

jlebon
Copy link
Contributor

@jlebon jlebon commented Apr 23, 2021

commit f814dda7b87201800e0ffa8abd4fdd4c9f021697
Date:   Fri Apr 23 12:08:34 2021 -0400

    hy-util: support missing outvars in hy_split_nevra

    Often, the client-side is only interested in e.g. extracting the package
    name, or the NVR, from a NEVRA. Let's make that easier by not requiring
    them to specify output variables for the components they're not
    interested in.

    = changelog =
    msg: Allow omitting some output variables when calling hy_split_nevra()
    type: enhancement
commit 2dd667f1ba0ffccc3c14f8c7fbe7aee27ad9e1b9
Date:   Fri Apr 23 12:10:24 2021 -0400

    hy-util: add hy_parse_module_spec

    This is a simple wrapper around libdnf::Nsvcap's `parse()` function.
    Support missing output variables, just like `hy_split_nevra`.

    = changelog =
    msg: Add new API hy_parse_module_spec()
    type: enhancement

Often, the client-side is only interested in e.g. extracting the package
name, or the NVR, from a NEVRA. Let's make that easier by not requiring
them to specify output variables for the components they're not
interested in.

= changelog =
msg: Allow omitting some output variables when calling hy_split_nevra()
type: enhancement
This is a simple wrapper around libdnf::Nsvcap's `parse()` function.
Support missing output variables, just like `hy_split_nevra`.

= changelog =
msg: Add new API hy_parse_module_spec()
type: enhancement
@@ -36,6 +36,8 @@ int hy_detect_arch(char **arch);

int hy_split_nevra(const char *nevra, char **name, int *epoch,
char **version, char **release, char **arch);
int hy_parse_module_spec(const char *spec, char **name, char **stream,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't sure if to add a docstring here. I can do so, but was keying off the other functions in this file not having one.

return 0;
}
return DNF_ERROR_INTERNAL_ERROR;
}

int
hy_parse_module_spec(const char *spec, char **name, char **stream,
Copy link
Member

Choose a reason for hiding this comment

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

This should be named hy_split_nsvcap with the first parameter being named *nsvcap. That makes it consistent with the other one that does the same thing for packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hy_split_nevra takes a NEVRA and splits it into its components. hy_parse_module_spec takes a module spec and tries it on various forms until it gets a match before splitting into its components. Hence why they're named differently. One could imagine a hy_split_nsvcap which assumes an NSVCAP as its first argument, analogous to hy_split_nevra.

Copy link
Member

Choose a reason for hiding this comment

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

But this does effectively assume that you're passing NSVCAP to it, because it does the same splitting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it would parse e.g. foobar:1.2 just fine. Unspecified components are returned as empty strings. Open to change that to just leave them NULL instead.

@jlebon
Copy link
Contributor Author

jlebon commented Jun 9, 2021

With the final rework in #1207, we no longer need this. The first commit still makes sense I think, but meh... not worth the churn.

@jlebon jlebon closed this Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants