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

libdeltachat: init at 1.54.0 #121151

Merged
merged 3 commits into from May 6, 2021
Merged

Conversation

dotlambda
Copy link
Member

@dotlambda dotlambda commented Apr 29, 2021

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

I could not try KDeltaChat because the interface was unusable for me. Might be that I need to run KDE to test it.

@link2xt
Copy link
Contributor

link2xt commented Apr 29, 2021

I could not try KDeltaChat because the interface was unusable for me. Might be that I need to run KDE to test it.

What does it mean? It does not start or you did not figure out how to use it? There is a menu for account manager where you can create an account, KDE frameworks other than kirigami are not needed.

@dotlambda
Copy link
Member Author

What does it mean? It does not start or you did not figure out how to use it? There is a menu for account manager where you can create an account, KDE frameworks other than kirigami are not needed.

Never mind. I just didn't know how to set up an account. What's not working is seeing messages sent from KDeltaChat on the phone, but I guess you haven't implemented that yet?

@dotlambda
Copy link
Member Author

Also, hitting Ctrl+Q results in

src/scheduler.rs:120: shutting down inbox loop
src/scheduler.rs:242: shutting down simple loop
src/scheduler.rs:242: shutting down simple loop
src/scheduler.rs:292: shutting down smtp loop
NO MORE EVENTS!
[1]    241522 segmentation fault (core dumped)  ./result/bin/kdeltachat

@dotlambda dotlambda marked this pull request as ready for review April 29, 2021 16:54
@link2xt
Copy link
Contributor

link2xt commented Apr 30, 2021

What's not working is seeing messages sent from KDeltaChat on the phone, but I guess you haven't implemented that yet?

Yes, there is no BCC-self setting on the Settings page yet. But overall, KDeltaChat is pre-alpha, there are many things missing and it sometimes segfaults on exit probably due to deltachat/deltachat-core-rust#2280

@dotlambda dotlambda force-pushed the libdeltachat-init branch 2 times, most recently from 369f3bc to fcc23c4 Compare April 30, 2021 08:46
homepage = "https://github.com/deltachat/deltachat-core-rust/";
changelog = "https://github.com/deltachat/deltachat-core-rust/blob/${version}/CHANGELOG.md";
license = licenses.mpl20;
maintainers = with maintainers; [ dotlambda ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
maintainers = with maintainers; [ dotlambda ];
maintainers = with maintainers; [ dotlambda ];
# darwin fails with:
# error: failed to add native library /private/tmp/nix-build-libdeltachat-1.52.0.drv-0/source/target/release/build/libsqlite3-sys-e85b24d5407a705c/out/libsqlite3.a: file too small to be an archive
broken = stdenv.isDarwin;

feel free to short the error message.

Copy link
Member Author

Choose a reason for hiding this comment

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

@SuperSandro2000 Can you try again?

Copy link
Member

Choose a reason for hiding this comment

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

I didn't build it myself. Ofborg did it for me. My builder is currently very flaky and I don't know why so lets wait a bit for ofborg.

Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up setting platforms = platforms.linux. If someone wants to use it on macOS, feel free to contribute a patch.

@dotlambda
Copy link
Member Author

I have no clue how to test whether it's actually using the system libraries.

@dotlambda
Copy link
Member Author

@ofborg build python3.pkgs.deltachat python39.pkgs.deltachat

@link2xt
Copy link
Contributor

link2xt commented May 2, 2021

I think you are still using vendored OpenSSL.

Patch deltachat-ffi/Cargo.toml to remove default vendored feature: https://github.com/deltachat/deltachat-core-rust/blob/6971bfc3d4570697fa6b29678e501464cc9deddf/deltachat-ffi/Cargo.toml#L29

This will be fixed in the next version, CMakeLists.txt will disable vendoring for OpenSSL and SQLite.

@dotlambda
Copy link
Member Author

I can confirm that system-wide OpenSSL is used now:

$ readelf -d result/lib/libdeltachat.so

Dynamic section at offset 0xf92408 contains 35 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libssl.so.1.1]
 0x0000000000000001 (NEEDED)             Shared library: [libcrypto.so.1.1]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x000000000000001d (RUNPATH)            Library runpath: [/nix/store/5bg9dy337rp8r1mcc4cdrjwhw1kwmppj-openssl-1.1.1k/lib:/nix/store/v8q6nxyppy1myi3rxni2080bv8s9jxiy-glibc-2.32-40/lib]
...

But it doesn't look like SQLite is.

@link2xt
Copy link
Contributor

link2xt commented May 2, 2021

@dotlambda I have just fixed vendoring on the master branch: deltachat/deltachat-core-rust#2410

Could you try commit deltachat/deltachat-core-rust@8e9d8ae without any patches?

If this works, I think it's easier to wait until the next release, it will be made after deltachat/deltachat-core-rust#2384 is merged.

@dotlambda
Copy link
Member Author

dotlambda commented May 2, 2021

@dotlambda I have just fixed vendoring on the master branch: deltachat/deltachat-core-rust#2410

Could you try commit deltachat/deltachat-core-rust@8e9d8ae without any patches?

Nice, it works perfectly:

Dynamic section at offset 0xdda4b8 contains 36 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libssl.so.1.1]
 0x0000000000000001 (NEEDED)             Shared library: [libcrypto.so.1.1]
 0x0000000000000001 (NEEDED)             Shared library: [libsqlite3.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
 0x000000000000001d (RUNPATH)            Library runpath: [/nix/store/5bg9dy337rp8r1mcc4cdrjwhw1kwmppj-openssl-1.1.1k/lib:/nix/store/dbxrckq9l6lr1s53db6kn4f2rajj766s-sqlite-3.35.2/lib:/nix/store/v8q6nxyppy1myi3rxni2080bv8s9jxiy-glibc-2.32-40/lib]

Thank you so much for looking into this!

If this works, I think it's easier to wait until the next release, it will be made after deltachat/deltachat-core-rust#2384 is merged.

That's what I'll do.

@dotlambda dotlambda marked this pull request as draft May 2, 2021 15:06
@link2xt
Copy link
Contributor

link2xt commented May 3, 2021

@dotlambda The lastest version tagged 1.54.0 is released: https://github.com/deltachat/deltachat-core-rust/tree/1.54.0

@dotlambda dotlambda changed the title libdeltachat: init at 1.52.0 libdeltachat: init at 1.54.0 May 3, 2021
@dotlambda dotlambda marked this pull request as ready for review May 3, 2021 09:48
@link2xt
Copy link
Contributor

link2xt commented May 3, 2021

@dotlambda

What's not working is seeing messages sent from KDeltaChat on the phone, but I guess you haven't implemented that yet?

I pushed a commit that adds setting that you need to enable: https://git.sr.ht/~link2xt/kdeltachat/commit/6989af9c746a054dabe24af1210736f464099793

Comment on lines +23 to +27
cargoDeps = rustPlatform.fetchCargoTarball {
inherit src;
name = "${pname}-${version}";
sha256 = "1p5yrhczp9nfijbvkmkmx1rabk5k3c1ni4k1vc0mw4jgl26lslcm";
};
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious as to why rustPlatform.buildRustPackage wasn't used since, it looks like you're running most of the logic.

And maybe just comment as to why buildRustPackage isn't suitable if there is a good reason :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Cause I'm using CMake. Using buildRustPackage leads to an empty output though I guess I could do make and make install in postBuild and installPhase. I don't see any advantage of that though.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jonringer Do you know of an easier way?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed that. No, the way that you did it makes sense then :)

@dotlambda dotlambda merged commit 30c3036 into NixOS:master May 6, 2021
@dotlambda dotlambda deleted the libdeltachat-init branch May 6, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants