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

Support 'from source' packaging, and using crates.io as a source #30

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

Conversation

nick42d
Copy link

@nick42d nick42d commented Aug 17, 2024

Added new flags --no-bin and --source.

Existing behaviour is --source project.

New options

--source crates-io --no-bin

Download crate from crates.io, get it's checksum, and write a PKGBUILD that downloads and builds from crates.io. No tarball is produced.

Example PKGBUILD

https://gist.github.com/nick42d/6ce7809a0b24444aaccf9655811cb750

--source crates-io

Download crate from crates.io, and create a binary release tarball in target/cargo-aur. Write a PKGBUILD that assumes user will upload binary release tarball to github/gitlab

Example PKGBUILD

https://gist.github.com/nick42d/dfe9e652473370671fecae9bce2d84fd

--source project --no-bin

Package the source code of the project to a source tarball in target/cargo-aur. Write a PKGBUILD that assumes user will upload source tarball to github/gitlab

Example PKGBUILD

https://gist.github.com/nick42d/e441adb325c01cbbee68c835bfe58e3b

This change should be non-breaking as --source defaults to project so I put this as a minor version bump. However, you could consider a major version bump due to the new functionality.

Testing note: the version number in Cargo.toml is used to download the .crate. If you try and run --source crates.io with version = "1.7.2" this will fail since 1.7.2 isn't uploaded to crates.io yet. Change version to 1.7.1 to test the new flags.

@nick42d
Copy link
Author

nick42d commented Aug 18, 2024

Added fix for failing test

@fosskers
Copy link
Owner

Thank you, I'll be reviewing this tomorrow.

src/crates.rs Outdated Show resolved Hide resolved
src/crates.rs Show resolved Hide resolved
src/crates.rs Show resolved Hide resolved
src/crates.rs Outdated Show resolved Hide resolved
src/crates.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
writeln!(file, "check() {{")?;
writeln!(file, " cd $pkgname-$pkgver")?;
writeln!(file, " export RUSTUP_TOOLCHAIN=stable")?;
writeln!(file, " cargo test --frozen --all-features")?;
Copy link
Owner

Choose a reason for hiding this comment

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

For tests I agree that --all-features is probably what they want.

Note that for the builder, this will cause all dependencies to be built a second time, but if this is irritating they can build with --nocheck as supplied by makepkg and other tools.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
@fosskers fosskers changed the title Support 'from source' packaging, and using crates.io as a source. Closes #28 Support 'from source' packaging, and using crates.io as a source Aug 24, 2024
src/crates.rs Show resolved Hide resolved
src/error.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@fosskers
Copy link
Owner

fosskers commented Aug 24, 2024

Great job so far. A few more things and we can merge this in. We'll need to come to a decision about the approach to the flag names.

@fosskers
Copy link
Owner

fosskers commented Sep 1, 2024

Thanks for your patience on this. I'm going to put this in soon.

Comment on lines +379 to +381
fn source_tarball(cargo_target: &Path, output: &Path, config: &Config) -> Result<(), Error> {
let args = ["publish", "--dry-run", "--allow-dirty"];
let status = Command::new("cargo").args(args).status()?;
Copy link
Owner

Choose a reason for hiding this comment

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

What if, instead of building the tarball ourselves, we pull one automatically based on a release tag? That's the strategy I use for Aura's main package on the AUR.

Copy link
Author

Choose a reason for hiding this comment

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

That should be OK, this will require author to publish to github using the exact tag format we are expecting so that we can then pull it to generate the checksums. Are you OK with that? I was worried that it was kind of an implicit behaviour that would need extra documentation, particularly since it deviates from the default. Dare I say it but I feel like this would have been a use case for source=url

Copy link
Owner

Choose a reason for hiding this comment

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

the exact tag format we are expecting

Isn't the tag format uniform, at least with Github? Always something like v1.2.3?

Copy link
Author

Choose a reason for hiding this comment

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

Github tags are free text entry - here's an example of one I made up on a spare repository.
https://github.com/nick42d/ratatuirepro/releases/tag/version-1.501.22

Comment on lines +136 to +142
(true, false) => {
let crate_file = crates::CrateFile::download_new(&config)?;
let built_crate_file = crate_file.build(args.musl)?;
let license = built_crate_file.tarball(&cargo_target)?;
let sha256 = sha256sum(&config.package, &output)?;
(sha256, license)
}
Copy link
Owner

Choose a reason for hiding this comment

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

This variation is unfortunately failing for me with the message:

:: Stripping binary...
:: Packing tarball...
:: Error: No such file or directory (os error 2)

It also seems to download a tarball to target/ and not target/cargo-aur/.

Copy link
Author

@nick42d nick42d Sep 17, 2024

Choose a reason for hiding this comment

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

Please try change 'version' in Cargo.toml to 1.7.1 if you haven't already. 1.7.2 will fail as it's not on crates.io yet.

Comment on lines +143 to +150
(false, true) => {
source_tarball(&cargo_target, &output, &config)?;
let license = alert_if_must_copy_license(&config.package.license)
.then(|| license_file(None))
.transpose()?;
let sha256 = sha256sum(&config.package, &output)?;
(sha256, license)
}
Copy link
Owner

Choose a reason for hiding this comment

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

This variant seems to produce a PKGBUILD that refers to a source with a different filename than the one produced by the build/packaging process. See my other comment about a potential shift in strategy for this variant.

Copy link
Author

Choose a reason for hiding this comment

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

I have resolved this now, apologies.

Comment on lines +151 to +160
(false, false) => {
release_build(args.musl)?;
let license = alert_if_must_copy_license(&config.package.license)
.then(|| license_file(None))
.transpose()?;
tarball(args.musl, &cargo_target, &output, license.as_ref(), &config)?;
let sha256 = sha256sum(&config.package, &output)?;
(sha256, license)
}
};
Copy link
Owner

Choose a reason for hiding this comment

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

The default - appears to work without issue.

@fosskers
Copy link
Owner

fosskers commented Oct 7, 2024

Hi there, please forgive the delay, I'm currently out of town for a wedding.

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.

Feature request: Support non-binary AUR package using crates.io as source
2 participants