Skip to content

add p5p archive publisher command #1

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

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

add p5p archive publisher command #1

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91ed850
add p5p archive publisher command
rcgoodfellow Feb 27, 2022
a852819
review feedback updates
rcgoodfellow Mar 17, 2022
a8e09e0
Merge branch 'main' into p5p
rcgoodfellow Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,27 @@ fn run_steps(ib: &mut ImageBuilder) -> Result<()> {
&a.publisher,
])?;
}
"pkg_set_p5p_publisher" => {
#[derive(Deserialize)]
struct PkgSetP5PPublisherArgs {
archive: String,
publisher: String,
}

let a: PkgSetP5PPublisherArgs = step.args()?;
let mp = ib.root()?;

pkg(log, &["-R", &mp.to_str().unwrap(),
"set-publisher", "-p", &a.archive, "--search-first",
])?;
Copy link
Member

@jclulow jclulow Mar 16, 2022

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

@rcgoodfellow rcgoodfellow Mar 17, 2022

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.

pkg(log, &["-R", &mp.to_str().unwrap(), "set-publisher",
"--no-refresh",
"--non-sticky",
&a.publisher,
])?;
Copy link
Member

@jclulow jclulow Mar 16, 2022

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.

pkg(log, &["-R", &mp.to_str().unwrap(), "refresh", "--full"])?;
pkg(log, &["-R", &mp.to_str().unwrap(), "update"])?;
Copy link
Member

@jclulow jclulow Mar 16, 2022

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

@richlowe richlowe Mar 19, 2022

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!)

}
"pkg_approve_ca_cert" => {
#[derive(Deserialize)]
struct PkgApproveCaCertArgs {
Expand Down