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

Update according to comment on AUR Web #1

Merged
merged 5 commits into from Apr 16, 2022
Merged

Update according to comment on AUR Web #1

merged 5 commits into from Apr 16, 2022

Conversation

S7evinK
Copy link
Owner

@S7evinK S7evinK commented Sep 3, 2021

No description provided.

@chris-morgan
Copy link

OK, I finally got back to this (funny how time flies).

  • The conduit-example.toml patch needs to be modified: there’s a new line database_backend = "rocksdb" immediately below the database_path line.

  • Having thought more about it, I prefer /etc/matrix-conduit/conduit.toml to /etc/matrix-conduit.toml: that way if more /etc files get added later, no change is required. Not a big deal by any means.

  • I’m in favour of renaming the package from matrix-conduit to conduit. The word “matrix” in the name is basically noise; if there were already a package named conduit I could understand it, but when there isn’t I don’t think it’s necessary.

  • I have now successfully built and installed it, and it’s running. Most functionality doesn’t seem to be working, but I’ll file issues on Conduit about that because I believe the problems are with it rather than with this package.

@S7evinK
Copy link
Owner Author

S7evinK commented Feb 26, 2022

Thanks for the feedback (again), and sorry for my delay.
I've talked to Conduits maintainer and he'd like to keep the word "matrix" in the package name.

Just pushed a small update, if you'd like to verify that the package is OK now. :)

Copy link

@Xiretza Xiretza left a comment

Choose a reason for hiding this comment

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

See the Rust package guidelines. --all-features had to be removed from the cargo invocations because the conduit repo contains broken code gated by some non-default features (heed and sled at time of writing).

prepare() {
cd "$_pkgname"
patch --forward --strip=1 --input="${srcdir}/0001-update-service-dynamicuser_paths.patch"
patch --forward --strip=1 --input="${srcdir}/0002-example-info.patch"
Copy link

Choose a reason for hiding this comment

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

  cargo fetch --locked --target "$CARCH-unknown-linux-gnu"

PKGBUILD Outdated
@@ -28,18 +40,13 @@ pkgver() {

build(){
cd "$_pkgname"
env CARGO_INCREMENTAL=1 cargo build --release --locked
env CARGO_INCREMENTAL=0 cargo build --release --locked
Copy link

Choose a reason for hiding this comment

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

Suggested change
env CARGO_INCREMENTAL=0 cargo build --release --locked
export RUSTUP_TOOLCHAIN=stable
export CARGO_TARGET_DIR=target
cargo build --frozen --release

@@ -28,18 +40,13 @@ pkgver() {

build(){
cd "$_pkgname"
env CARGO_INCREMENTAL=1 cargo build --release --locked
env CARGO_INCREMENTAL=0 cargo build --release --locked
}

Copy link

Choose a reason for hiding this comment

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

check() {
  cd "$_pkgname"

  export RUSTUP_TOOLCHAIN=stable
  cargo test --frozen
}

@S7evinK S7evinK merged commit a6ffd83 into master Apr 16, 2022
@S7evinK S7evinK deleted the beta-update branch April 16, 2022 14:14
@S7evinK S7evinK restored the beta-update branch April 16, 2022 14:58
@S7evinK S7evinK deleted the beta-update branch April 16, 2022 15:06
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.

None yet

3 participants