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

add p5p archive publisher command #1

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

Conversation

rcgoodfellow
Copy link

This allows custom p5p archives to be included in the image build process.

src/main.rs Outdated
Comment on lines 2535 to 2539
pkg(log, &["-R", &mp.to_str().unwrap(), "set-publisher",
"--no-refresh",
"--non-sticky",
&a.publisher,
])?;
Copy link
Member

Choose a reason for hiding this comment

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

I suspect it would be better to adjust the existing pkg_set_publisher step to do this. You could make the uri argument to that step optional, and add a new optional sticky boolean that if provided would include an appropriate --sticky or --non-sticky argument.

src/main.rs Outdated
Comment on lines 2540 to 2541
pkg(log, &["-R", &mp.to_str().unwrap(), "refresh", "--full"])?;
pkg(log, &["-R", &mp.to_str().unwrap(), "update"])?;
Copy link
Member

Choose a reason for hiding this comment

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

We should turn these into separate steps, I think. A pkg_refresh step with an optional full boolean, and a pkg_update step.

Copy link
Member

Choose a reason for hiding this comment

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

I've been talking to @jclulow about similar things. and feel the need to add that if you just did refresh --full, you probably want to pass --no-refresh to everything else, to avoid doing checks again and again. Here, especially, that seems easy. It might be less easy for every pkg operation (but it would be good!)

src/main.rs Outdated
Comment on lines 2532 to 2534
pkg(log, &["-R", &mp.to_str().unwrap(),
"set-publisher", "-p", &a.archive, "--search-first",
])?;
Copy link
Member

Choose a reason for hiding this comment

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

This form of set-publisher does a bit of magic when you don't specify a publisher argument, as far as I can tell. Depending on what publishers are in the archive it might end up adding more than one, or updating existing publishers. Is that what you intend?

Also, it seems like the image will contain publisher information that might not make sense once it's installed, unless you ship the archive at the exact same location inside the final image.

Did you try a version of this that used the temporary archive functionality; i.e., something like pkg update -g $ARCHIVE_PATH ? You would still need to have set the existing publisher(s) non-sticky I believe.

Copy link
Author

Choose a reason for hiding this comment

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

The intention is to add the packages from the archive as the packages with the highest priority.

In the builds that I have that use this machinery, yes, I am shipping the p5p archive at the exact location it's used from here. The intent is to ship the archive to its expected/installed location.

I did try the pkg install -g $ARCHIVE_PATH * route. The packages I'm using this for are ONU builds, and this avenue did not go well.

@rcgoodfellow
Copy link
Author

Thanks for the feedback @jclulow. I've broken up the pkg_set_p5p_publisher command as you suggested and provided some more context on the last question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants