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 c2patool #702

Merged
merged 7 commits into from
Oct 25, 2023
Merged

Add c2patool #702

merged 7 commits into from
Oct 25, 2023

Conversation

leszko
Copy link
Collaborator

@leszko leszko commented Oct 17, 2023

No description provided.

@iameli
Copy link
Contributor

iameli commented Oct 18, 2023

Note that this will get it in the Docker image but not in the tarball archive that will run during a deploy - that'll need to happen via something like 2f584db.

That said, this is making me feel like we should probably just generate that archive off of the Docker image anyway for simplicity

@leszko
Copy link
Collaborator Author

leszko commented Oct 19, 2023

Note that this will get it in the Docker image but not in the tarball archive that will run during a deploy - that'll need to happen via something like 2f584db.

That said, this is making me feel like we should probably just generate that archive off of the Docker image anyway for simplicity

Thanks for the comment. Right, generating the archive from Docker image would be great.

Though, for this PR, I'll just add what you pointed out.

@leszko leszko marked this pull request as ready for review October 19, 2023 12:19
@leszko leszko requested a review from iameli October 19, 2023 12:20
apt update && apt install -y curl build-essential git
curl https://sh.rustup.rs -sSf | sh -s -- -y
source "$HOME/.cargo/env"
cargo install c2patool
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably this to match the other one?

Suggested change
cargo install c2patool
cargo install --version 0.6.2 c2patool

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/marketplace/actions/cargo-install I find dedicated github actions working faster than manual installations. Maybe worth trying out

Copy link
Contributor

@iameli-streams iameli-streams left a comment

Choose a reason for hiding this comment

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

Approved with tiny change

@leszko leszko merged commit 85b5b97 into main Oct 25, 2023
@leszko leszko deleted the rafal/c2pa branch October 25, 2023 07:35
@leszko
Copy link
Collaborator Author

leszko commented Oct 25, 2023

Thanks @pwilczynskiclearcode for the suggestion with the cargo GH Action!

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.

4 participants