Paths

Table of Contentst

lang/rust: Update for ino64
ClosedPublic
Actions

Authored by kib on May 18 2017, 3:14 PM.

Details

Reviewers

riggs
dumbbell
bapt

Group Reviewers

portmgr

Commits

rP441843: Fix lang/rust after ino64 src commit.

Summary

This change adopts the port to ino64 commit, which is explained in
https://lists.freebsd.org/pipermail/freebsd-current/2017-April/065687.html
The src change is nearly ready to commit, and is undergoing a final round
of pre-commit testing. This will likely happen in the next few days,
unless some issue found.

In general, patches for ino64 support will be conditional on FreeBSD
version. It is likely that __FreeBSD_version / kern.osreldate 1200031
will be the value used for the ino64 change, and the port patch may have
a conditional test using that value. Of course, if the final value in
the src commit is different the ports patches will be updated before
commit.

A number of ports require updates after this change is committed to src.
I seek approval of the change to the port in this review, either directly
from the maintainer or portmgr blanket approval of the set of ino64 port
updates. In either case I will commit the ports change shortly after the
src tree commit.

I am also happy if you, as maintainer of the port, wish to handle this
yourself. Please add a comment here with your preference for this port.

Diff Detail

Repository

rP FreeBSD ports repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.May 18 2017, 3:14 PM

Herald added a subscriber: mat. · View Herald TranscriptMay 18 2017, 3:14 PM

The ino64 src change is in review D10439

emaste added a parent revision: D10439: ino64.May 18 2017, 3:24 PM

Hi!

I started to think about this patch and I have some questions.

If I understand correctly, old_fstat.c is used to "patch" the rust-std bootstrap so that it calls the compatibility syscalls. Is that correct?

The content of liblibc is a Git submodule initially, making the libc "crate". That crate is used by devel/cargo so we also need to apply the patch to the copy of libc inside the cargo-registry-$VERSION.tar.xz archive. Newer versions of that crate also have FreeBSD/aarch64 support (i.e. there is a src/liblibc/src/unix/bsd/freebsdlike/freebsd/aarch64.rs): lang/rust 1.17.0 has it and the incoming devel/cargo 0.18.0 will do too. See D10857 and D10867 for patches to enable those two ports on FreeBSD/aarch64.

Did you submit anything upstream yet?

Currently, rustup, the tool to easily install any version of Rust and Cargo in your user home, works out-of-the-box on FreeBSD. It's going to break with this change. Additionally, Rust supports cross-compilation to different targets. With those two things in mind, I'm wondering if a new Rust target should be introduced (e.g. x86_64-unknown-freebsd-ino64). Did you contact Rust authors to discuss it?

In D10799#225176, @dumbbell wrote:

Hi!

I started to think about this patch and I have some questions.

If I understand correctly, old_fstat.c is used to "patch" the rust-std bootstrap so that it calls the compatibility syscalls. Is that correct?

Yes.

The content of liblibc is a Git submodule initially, making the libc "crate". That crate is used by devel/cargo so we also need to apply the patch to the copy of libc inside the cargo-registry-$VERSION.tar.xz archive. Newer versions of that crate also have FreeBSD/aarch64 support (i.e. there is a src/liblibc/src/unix/bsd/freebsdlike/freebsd/aarch64.rs): lang/rust 1.17.0 has it and the incoming devel/cargo 0.18.0 will do too. See D10857 and D10867 for patches to enable those two ports on FreeBSD/aarch64.

Did you submit anything upstream yet?

No, and I have no intend to. I do not think that this is reasonable to submit this stuff to upstream. Did you read the long message explaining the ino64, which is linked in the review description ?

Currently, rustup, the tool to easily install any version of Rust and Cargo in your user home, works out-of-the-box on FreeBSD. It's going to break with this change. Additionally, Rust supports cross-compilation to different targets. With those two things in mind, I'm wondering if a new Rust target should be introduced (e.g. x86_64-unknown-freebsd-ino64). Did you contact Rust authors to discuss it?

There are two issues with the rust and the ino64 branch. First is that the bootstraps from before ino64 cannot generate working binaries on post-ino64 systems. See D10798 for yet another discussion of the same situation, but in context of ghc. The fix there is to regenerate the bootstrap. Then, old_fstat.c becomes unneeded. Or, put it another way, old_fstat.c saves somebody from doing cross-build, which looks to be quite painful for rustc.

Second problem is that rust provides rust definitions for the system structures, and that structures changed. Ports patch fixes them, but I am not sure about some aspects of it, in particular, WRT rust lib interfaces stability guarantees. At least, patched port generates working binaries, I checked it with re-building cargo.

If you do not like either of approaches and have something more idiomatic and appropriate, sure go your way, I do not insist on this patch at all. JFYI, ino64 will be committed really soon, i.e. in hours.

In D10799#225180, @kib wrote:

The content of liblibc is a Git submodule initially, making the libc "crate". That crate is used by devel/cargo so we also need to apply the patch to the copy of libc inside the cargo-registry-$VERSION.tar.xz archive. Newer versions of that crate also have FreeBSD/aarch64 support (i.e. there is a src/liblibc/src/unix/bsd/freebsdlike/freebsd/aarch64.rs): lang/rust 1.17.0 has it and the incoming devel/cargo 0.18.0 will do too. See D10857 and D10867 for patches to enable those two ports on FreeBSD/aarch64.

Did you submit anything upstream yet?

No, and I have no intend to. I do not think that this is reasonable to submit this stuff to upstream. Did you read the long message explaining the ino64, which is linked in the review description?

Yes, I read it but perhaps I was unclear. I was talking about the part of your patch which modifies the Rust copy of the system types and structs in files under src/liblibc/src/unix/bsd/freebsdlike and src/vendor/libc/src/unix/bsd/freebsdlike. Do you say that such a change shouldn't be submitted upstream?

There are two issues with the rust and the ino64 branch. First is that the bootstraps from before ino64 cannot generate working binaries on post-ino64 systems. See D10798 for yet another discussion of the same situation, but in context of ghc. The fix there is to regenerate the bootstrap. Then, old_fstat.c becomes unneeded. Or, put it another way, old_fstat.c saves somebody from doing cross-build, which looks to be quite painful for rustc.

Thanks, the dicussion in the GHC patch review helped. I understand that the executables (like rustc) and dynamic libraries are fine. However, the static libraries are not, thus the old_fstat.c addition in the bootstrap libstd.rlib. I created a bootstrap compiler + rust-std for aarch64, so I can probably rebuild one for amd64 and i386.

Second problem is that rust provides rust definitions for the system structures, and that structures changed. Ports patch fixes them, but I am not sure about some aspects of it, in particular, WRT rust lib interfaces stability guarantees. At least, patched port generates working binaries, I checked it with re-building cargo.

If you do not like either of approaches and have something more idiomatic and appropriate, sure go your way, I do not insist on this patch at all. JFYI, ino64 will be committed really soon, i.e. in hours.

Your patch is good, and thank you very much for doing all the research and work on this!

I accept the patch, though I can't commit it.

This revision is now accepted and ready to land.May 23 2017, 9:54 PM

In D10799#225393, @dumbbell wrote:

In D10799#225180, @kib wrote:

The content of liblibc is a Git submodule initially, making the libc "crate". That crate is used by devel/cargo so we also need to apply the patch to the copy of libc inside the cargo-registry-$VERSION.tar.xz archive. Newer versions of that crate also have FreeBSD/aarch64 support (i.e. there is a src/liblibc/src/unix/bsd/freebsdlike/freebsd/aarch64.rs): lang/rust 1.17.0 has it and the incoming devel/cargo 0.18.0 will do too. See D10857 and D10867 for patches to enable those two ports on FreeBSD/aarch64.

Did you submit anything upstream yet?

No, and I have no intend to. I do not think that this is reasonable to submit this stuff to upstream. Did you read the long message explaining the ino64, which is linked in the review description?

Yes, I read it but perhaps I was unclear. I was talking about the part of your patch which modifies the Rust copy of the system types and structs in files under src/liblibc/src/unix/bsd/freebsdlike and src/vendor/libc/src/unix/bsd/freebsdlike. Do you say that such a change shouldn't be submitted upstream?

I only mean that I personally do not intend to submit it upstream. That said, I indeed do not think that the patch (its lib source part) is suitable for inclusion into the upstream source as is, because it breaks structure definitions for pre-ino64 systems, in particular, for stable/10 and 11. It should be conditionalized on the __FreeBSD_version value in rust sources, somehow. I do not know how to do it, and did not investigated. Ports patch conditionally applies the change, checking the version in the ports Makefile.

There are two issues with the rust and the ino64 branch. First is that the bootstraps from before ino64 cannot generate working binaries on post-ino64 systems. See D10798 for yet another discussion of the same situation, but in context of ghc. The fix there is to regenerate the bootstrap. Then, old_fstat.c becomes unneeded. Or, put it another way, old_fstat.c saves somebody from doing cross-build, which looks to be quite painful for rustc.

Thanks, the dicussion in the GHC patch review helped. I understand that the executables (like rustc) and dynamic libraries are fine. However, the static libraries are not, thus the old_fstat.c addition in the bootstrap libstd.rlib. I created a bootstrap compiler + rust-std for aarch64, so I can probably rebuild one for amd64 and i386.

Second problem is that rust provides rust definitions for the system structures, and that structures changed. Ports patch fixes them, but I am not sure about some aspects of it, in particular, WRT rust lib interfaces stability guarantees. At least, patched port generates working binaries, I checked it with re-building cargo.

If you do not like either of approaches and have something more idiomatic and appropriate, sure go your way, I do not insist on this patch at all. JFYI, ino64 will be committed really soon, i.e. in hours.

Your patch is good, and thank you very much for doing all the research and work on this!

In D10799#225533, @kib wrote:

I only mean that I personally do not intend to submit it upstream. That said, I indeed do not think that the patch (its lib source part) is suitable for inclusion into the upstream source as is, because it breaks structure definitions for pre-ino64 systems, in particular, for stable/10 and 11. It should be conditionalized on the __FreeBSD_version value in rust sources, somehow. I do not know how to do it, and did not investigated. Ports patch conditionally applies the change, checking the version in the ports Makefile.

Ok. I agree as well: that patch is ok for the Ports tree but not upstream. I wasn't saying we should push it as-is, that's why I initially asked if you already discussed this with upstream, but no. That's fine, I can bring this topic upstream. On top of my head, perhaps we'll need to introduce a new Rust target, but I don't know the Rust compiler enough. Perhaps there are better ways.

Thanks!

I just got a failure with this patch on 12.0-CURRENT r318776 amd64.

Running `/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/bootstrap/debug/rustc --crate-name std src/libstd/lib.rs --crate-type dylib --crate-type rlib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 --cfg 'feature="panic_unwind"' --cfg 'feature="jemalloc"' --cfg 'feature="backtrace"' --cfg 'feature="panic-unwind"' --cfg 'feature="alloc_jemalloc"' -C metadata=4ae50384caff498d -C extra-filename=-4ae50384caff498d --out-dir /wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps --target x86_64-unknown-freebsd -L dependency=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps -L dependency=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/release/deps --extern std_unicode=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libstd_unicode-ed54ef6082fbcc46.rlib --extern compiler_builtins=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcompiler_builtins-c13672c4c73fa75a.rlib --extern collections=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcollections-e6817aadad5ee360.rlib --extern alloc_jemalloc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc_jemalloc-184adff2e07c5eb6.rlib --extern rand=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/librand-60273cf2198b0f17.rlib --extern panic_abort=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libpanic_abort-74e4981a648f2f59.rlib --extern libc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liblibc-ed559831ecc47b27.rlib --extern panic_unwind=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libpanic_unwind-6adbfc72a11a4ec1.rlib --extern core=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcore-5b967cb2e98e0679.rlib --extern alloc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc-b1ace0fca798fd7d.rlib --extern unwind=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libunwind-62846e64973eca80.rlib --extern alloc_system=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc_system-00fe1df0fa7b0d5d.rlib -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/libbacktrace/.libs -l static=backtrace -l execinfo -l pthread -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/compiler-rt/. -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/jemalloc/lib`

error[E0463]: can't find crate for std_unicode which core_collections depends on

--> src/libstd/lib.rs:330:1
 |

330 | extern crate collections as core_collections;

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

fatal runtime error: failed to initiate panic, error 5
error: Could not compile std.

Caused by:

process didn't exit successfully: `/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/bootstrap/debug/rustc --crate-name std src/libstd/lib.rs --crate-type dylib --crate-type rlib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 --cfg feature="panic_unwind" --cfg feature="jemalloc" --cfg feature="backtrace" --cfg feature="panic-unwind" --cfg feature="alloc_jemalloc" -C metadata=4ae50384caff498d -C extra-filename=-4ae50384caff498d --out-dir /wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps --target x86_64-unknown-freebsd -L dependency=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps -L dependency=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/release/deps --extern std_unicode=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libstd_unicode-ed54ef6082fbcc46.rlib --extern compiler_builtins=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcompiler_builtins-c13672c4c73fa75a.rlib --extern collections=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcollections-e6817aadad5ee360.rlib --extern alloc_jemalloc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc_jemalloc-184adff2e07c5eb6.rlib --extern rand=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/librand-60273cf2198b0f17.rlib --extern panic_abort=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libpanic_abort-74e4981a648f2f59.rlib --extern libc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liblibc-ed559831ecc47b27.rlib --extern panic_unwind=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libpanic_unwind-6adbfc72a11a4ec1.rlib --extern core=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libcore-5b967cb2e98e0679.rlib --extern alloc=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc-b1ace0fca798fd7d.rlib --extern unwind=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/libunwind-62846e64973eca80.rlib --extern alloc_system=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage1-std/x86_64-unknown-freebsd/release/deps/liballoc_system-00fe1df0fa7b0d5d.rlib -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/libbacktrace/.libs -l static=backtrace -l execinfo -l pthread -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/compiler-rt/. -L native=/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/native/jemalloc/lib` (signal: 6, SIGABRT: process abort signal)

command did not execute successfully: "/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/build/x86_64-unknown-freebsd/stage0/bin/cargo" "build" "-j" "8" "--target" "x86_64-unknown-freebsd" "-v" "--release" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/wrkdirs/usr/ports/lang/rust/work/rustc-1.17.0-src/src/libstd/Cargo.toml"
expected success, got: exit code: 101

Build completed unsuccessfully in 1:33:27
gmake[1]: *** [Makefile:24: all] Error 1

FTR, someone from OpenBSD started a discussion about ABI breakage in various OSes and how to support that in Rust:
https://internals.rust-lang.org/t/pre-rfc-target-extension-dealing-with-breaking-changes-at-os-level/5289

jilles added a subscriber: jilles.May 24 2017, 9:20 PM

jilles added inline comments.

lang/rust/files/extra-patch-ino64
24–25	This needs to be changed to cope with the late increase of `d_namlen` to 16 bits.

Update patch to take into account 16bit d_namlen.

Builds for me.

This revision now requires review to proceed.May 25 2017, 5:38 PM

dumbbell added a child revision: D10857: lang/rust: Enable on FreeBSD/aarch64 + various fixes.May 26 2017, 8:41 PM

I tested the patch successfully on pre-ino64 and post-ino64 systems:

lang/rust and devel/cargo build fine
A simple Rust program which reads file stats is returning meaningful data.

This revision is now accepted and ready to land.May 27 2017, 10:30 AM

bapt accepted this revision.May 27 2017, 11:51 AM

Closed by commit rP441843: Fix lang/rust after ino64 src commit. (authored by kib). · Explain WhyMay 27 2017, 12:07 PM

This revision was automatically updated to reflect the committed changes.

This seems to fail on pre-ino64 systems (maybe the removal of ${LN} -sf ${DISTDIR}/${DIST_SUBDIR}/${RUST_STD_BOOTSTRAP} ${WRKSRC}/build/cache/${RUST_BOOTSTRAP_DIR} ???)

See http://package23.nyi.freebsd.org/data/110i386-default-PR218976/2017-05-27_18h52m33s/logs/errors/rust-1.17.0.log

In D10799#226936, @antoine wrote:

This seems to fail on pre-ino64 systems (maybe the removal of ${LN} -sf ${DISTDIR}/${DIST_SUBDIR}/${RUST_STD_BOOTSTRAP} ${WRKSRC}/build/cache/${RUST_BOOTSTRAP_DIR} ???)

See http://package23.nyi.freebsd.org/data/110i386-default-PR218976/2017-05-27_18h52m33s/logs/errors/rust-1.17.0.log

Indeed, thank you for the report. D10973.

Also, both ghc and rust i386 post-ino64 are not handled. Patches should be similar, I plan to do it early next week.

jbeich added a subscriber: jbeich.Jun 27 2017, 12:46 PM

jbeich added inline comments.