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.

Sign up for GitHub

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

Jump to bottom

bin/xbps-repodb: add new tool to manage multiple repositories at once #286

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

bin/xbps-repodb: add new tool to manage multiple repositories at once #286

wants to merge 3 commits into from

Conversation

Duncaen
Copy link
Member

@Duncaen Duncaen commented May 12, 2020

This currently only supports -p/--purge to remove obsolete binary
package files from repositories with the awareness that different
noarch package version can co-exist within a repository in
different architectures repository indexes.

@Duncaen
bin/xbps-repodb: add new tool to manage multiple repositories at once
0aa996b 
This currently only supports -p/--purge to remove obsolete binary
package files from repositories with the awareness that different
noarch package version can co-exist within a repository in
different architectures repository indexes.
@lgtm-com
Copy link

lgtm-com bot commented May 12, 2020

This pull request introduces 1 alert when merging 0aa996b into 1594129 - view on LGTM.com

new alerts:

  • 1 for Missing header guard

pullmoll
pullmoll reviewed May 12, 2020
View reviewed changes
bin/xbps-repodb/purge.c
xbps_error_printf("invalid repodata: %s\n", name);
exit(1);
}
strncpy(arch->arch, name, d-name);
Copy link
Member

@pullmoll pullmoll May 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

strncpy(3) should use the size of the destination for n, i.e. strncpy(arch->arch, name, sizeof(arch->arch));, not a size depending on the source. I see you made sure it fits into arch->arch before but gcc or clang will probably complain?

Copy link
Member

Choose a reason for hiding this comment

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

Hm... no, they don't complain. Probably gcc is smart enough to see that this is ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is used to copy only the architecture part from from XXX-repodata, n is used here as how many bytes to copy and not as limit, for the limit use case I would have used xbps_strlcpy.

@pullmoll
Copy link
Member

I have another use case. I accidentially just added *.mips-musl.xbps packages to my x86_64-repodata, because I forgot to prepend XBPS_ARCH=mips-musl to the command. DUH!
Could we make this tool remove (optionally?) wrong architecture packages from a repodata file?

@Duncaen
Copy link
Member Author

Duncaen commented May 12, 2020

I think its better to not allow xbps-rindex -a to add different architectures packages anymore, I can't think of any usecase where this would be desirable and this would make this mode not really required.

@Duncaen
Copy link
Member Author

Duncaen commented May 12, 2020

Hmm this shouldn't have been possible, xbps-rindex -a already checks the architecture:

xbps/bin/xbps-rindex/index-add.c

Lines 271 to 276 in 1594129

if (!xbps_pkg_arch_match(xhp, arch, NULL)) {
fprintf(stderr, "index: ignoring %s, unmatched arch (%s)\n", pkgver, arch);
xbps_object_release(binpkgd);
free(pkgver);
continue;
}

@pullmoll
Copy link
Member

My bad. I didn't see the error message and for *.noarch.xbps it succeeded, which made me think it succeeded for all packages. Inspecting index.plist there is no <string>mips-musl</string> so everything is ok.

@Duncaen
Copy link
Member Author

Duncaen commented May 12, 2020

My plan for xbps-repodb is to eventually move all the staging stuff to it to allow it to maintain the stage over multiple repositories and make xbps-rindex a tool for "simple" local repositories.

  • xbps-rindex loses some of its functionality.
    • signing of packages moves to xbps-repodb (this is a task for published repositories).
    • --clean could stay to allow simple cleaning of stagedata.
    • --remove-orphans may stay for simple usecases, but xbps-repodb --purge is prefered.
  • xbps-repodb becomes the publishing tool for repositories.
    • it checks the stagedata and selectively moves sets of packages from each stagedata into the corresponding repodata when all dependencies and shared libraries can be satisfied.
    • sign repodata.
    • sign packages.
    • maybe maintain a new filesdb in the future?
    • maybe also maintain a man page dump?

xbps-src would not be changed it would continue to just use xbps-rindex to add new packages to *-stagedata.
xbps-repodb would be "privileged" ans has access to secret keys so it can sign repodata and packages.
It should be "fast" and non-disruptive to run it in a cronjob to have the secret keys owned by a user separate from xbps-src.

@Chocimier
Copy link
Member

Great that you found time to start working on this!

Chocimier
Chocimier reviewed May 12, 2020
View reviewed changes
bin/xbps-repodb/main.c Outdated Show resolved Hide resolved
@Chocimier
Copy link
Member

Is it assured that for each stagedata there is repodata at same time?

st3r4g
st3r4g reviewed Jun 6, 2020
View reviewed changes
bin/xbps-repodb/main.c
struct xbps_handle xh;
const struct option longopts[] = {
{ "debug", no_argument, NULL, 'd' },
{ "dry-run", no_argument, NULL, 'n' },
Copy link

Choose a reason for hiding this comment

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

missing "purge" long option here

ericonr
ericonr reviewed Jun 17, 2020
View reviewed changes
bin/xbps-repodb/main.c
main(int argc, char **argv)
{
int c = 0, rv = 0;
struct xbps_handle xh;
Copy link
Member

Choose a reason for hiding this comment

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

Can't you do struct xbps_handle xh = { }; instead of a memset below? It should initialize the whole struct to zero and is easier to follow.

Copy link
Member

Choose a reason for hiding this comment

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

Well, this isn't ISO C. struct xbps_handle xh = { 0 }; should work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericonr ericonr ericonr left review comments

@st3r4g st3r4g st3r4g left review comments

@Chocimier Chocimier Chocimier left review comments

@pullmoll pullmoll pullmoll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Duncaen @pullmoll @Chocimier @st3r4g @ericonr