Bug 1356907 - Review Request: rust - The Rust Programming Language
Summary: Review Request: rust - The Rust Programming Language
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Igor Gnatenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 915043 (view as bug list)
Depends On:
Blocks: cargo 1359763 1366555
TreeView+ depends on / blocked
 
Reported: 2016-07-15 09:09 UTC by Josh Stone
Modified: 2016-09-08 08:37 UTC (History)
24 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-08-27 10:33:16 UTC
ignatenko: fedora-review+


Attachments (Terms of Use)

Description Josh Stone 2016-07-15 09:09:53 UTC
Spec URL: http://jistone.fedorapeople.org//rust.spec
SRPM URL: http://jistone.fedorapeople.org//rust-1.10.0-1.fc25.src.rpm

Description:
Rust is a systems programming language that runs blazingly fast, prevents
segfaults, and guarantees thread safety.

This package includes the Rust compiler, standard library, and documentation
generator.

Comment 1 Josh Stone 2016-07-15 09:09:56 UTC
This package built on koji:  http://koji.fedoraproject.org/koji/taskinfo?taskID=14904177

Comment 2 Josh Stone 2016-07-15 17:53:24 UTC
FWIW, here's a scratch build trying an armv7hl bootstrap:
http://koji.fedoraproject.org/koji/taskinfo?taskID=14908548

Even x86 took ~100min, so this might take a while.  Note also that arm is Tier 2 by upstream standards, so it's not as well tested as x86.  It should be build ok, but we'll see whether it can pass the %check...

Comment 3 Josh Stone 2016-07-15 23:27:35 UTC
Nearly 6 hours later, armv7hl failed a few run-pass tests like so:

> error: linking with `cc` failed: exit code: 1
> note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-L" "/builddir/build/BUILD/rustc-1.10.0/armv7-unknown-linux-gnueabihf/stage2/lib/rustlib/armv7-unknown-linux-gnueabihf/lib" "armv7-unknown-linux-gnueabihf/test/run-pass/vec-macro-no-std.0.o" "-o" "armv7-unknown-linux-gnueabihf/test/run-pass/vec-macro-no-std.stage2-armv7-unknown-linux-gnueabihf" "-Wl,--gc-sections" "-pie" "-Wl,-O1" "-nodefaultlibs" "-L" "armv7-unknown-linux-gnueabihf/test/run-pass/" "-L" "armv7-unknown-linux-gnueabihf/test/run-pass/vec-macro-no-std.stage2-armv7-unknown-linux-gnueabihf.run-pass.libaux" "-L" "armv7-unknown-linux-gnueabihf/rt" "-L" "/builddir/build/BUILD/rustc-1.10.0/armv7-unknown-linux-gnueabihf/stage2/lib/rustlib/armv7-unknown-linux-gnueabihf/lib" "-Wl,-Bstatic" "-Wl,-Bdynamic" "-L" "/builddir/build/BUILD/rustc-1.10.0/armv7-unknown-linux-gnueabihf/stage2/lib/rustlib/armv7-unknown-linux-gnueabihf/lib" "-l" "std-e8edd0fd" "-l" "dl" "-l" "pthread" "-l" "gcc_s" "-l" "c" "-l" "m" "-l" "rt" "-l" "util" "-l" "compiler-rt"
> note: /usr/bin/ld: /usr/lib/libc_nonshared.a(elf-init.oS): undefined reference to symbol '__aeabi_unwind_cpp_pr0@@GCC_3.5'
> /lib/libgcc_s.so.1: error adding symbols: DSO missing from command line
> collect2: error: ld returned 1 exit status
> error: aborting due to previous error

I think there's a c/gcc_s interdependency that's not correctly managed here.  At least on x86_64, the default C link line has something like this:

> -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed

i.e. -lgcc_s is positioned twice as-needed, before and after -lc.

I will see if I can reproduce this locally and file an rust issue.


For now, I think we should leave rust.spec as i686/x86_64 only, citing Tier 1 status.

Comment 4 Igor Gnatenko 2016-07-16 09:25:37 UTC
in Exclusive arch replace i686 with %{ix86}

Comment 5 Josh Stone 2016-07-17 02:50:40 UTC
Even if it doesn't really cover *all* x86?  We only have a bootstrap compiler for i686 in particular.  Although i586 is another known target at the moment, upstream only builds std for it, and it's not even listed in the support tiers at all.

Comment 6 Igor Gnatenko 2016-07-26 08:26:02 UTC
* License file FiraSans-LICENSE.txt is not marked as %license
* Package has .a files
* %doc %{_docdir}/%{name}/html/
-> nothing actually owns %{_docdir}/%{name} for -doc subpackage
* %define bootstrap_base https://static.rust-lang.org/dist/%{bootstrap_date}/rustc-%{bootstrap_channel}
use %global
* BuildRequires:  python
-> is it really needed like python? not python2 or python3?
* -doc subpackage must be noarch
* is it possible to move all lib*.so into for example, %{_libdir}/rust/?

Bundled libs, add to License tag and include licenses to %license:
* libbacktrace is BSD
* hoedown is ISC
* I think "or" should be replaced with "and" in License or rust is licensed on one or second license?

Comment 7 Josh Stone 2016-07-26 17:27:29 UTC
(In reply to Igor Gnatenko from comment #6)
> * License file FiraSans-LICENSE.txt is not marked as %license

OK, will change.

> * Package has .a files

Yes, libcompiler-rt.a, and technically all the .rlib files are static archives too.  I could move them all to a rust-static package, and then the base rust package will need to require that anyway.  Is that preferable?

> * %doc %{_docdir}/%{name}/html/
> -> nothing actually owns %{_docdir}/%{name} for -doc subpackage

Ok, it can share ownership of that directory, right?

> * %define bootstrap_base
> https://static.rust-lang.org/dist/%{bootstrap_date}/rustc-
> %{bootstrap_channel}
> use %global

Sure, will change.

> * BuildRequires:  python
> -> is it really needed like python? not python2 or python3?

You're right, it needs python2.  I'll change it.

> * -doc subpackage must be noarch

How strong is that "must"?  The documentation can vary by architecture in small ways.  For instance, `std::os::linux::raw::stat`[1] varies everywhere.  That happens to be deprecated, but it's just one I know off-hand; there may be more.

[1] https://doc.rust-lang.org/stable/std/os/linux/raw/struct.stat.html

> * is it possible to move all lib*.so into for example, %{_libdir}/rust/?

Maybe so, but why?  They have unique hashes in the names, so they won't collide.  Then we'd have to either add rpaths to rustc and rustdoc, or add this path to ld.so.conf.d/ anyway.  So why move them?

> Bundled libs, add to License tag and include licenses to %license:
> * libbacktrace is BSD
> * hoedown is ISC

OK to both.

> * I think "or" should be replaced with "and" in License or rust is licensed
> on one or second license?

Rust's COPYRIGHT explicitly says it is "at your option", so I think "or" is correct.  Then I guess the additions for bundled libraries will be "and", so we need something like this?

  License: ASL 2.0 or MIT, and BSD and ISC

Comment 8 Josh Stone 2016-07-27 05:20:13 UTC
New files:
https://fedorapeople.org/~jistone/review/rust/v2/rust.spec
https://fedorapeople.org/~jistone/review/rust/v2/rust-1.10.0-2.fc26.src.rpm

I updated the things I agreed to above.  I didn't yet do anything about changing the packaging static files, making -doc noarch, or moving libraries to a subdirectory.  Please see my questions about those.

Comment 9 Igor Gnatenko 2016-07-27 06:36:28 UTC
* Fix license tag as we discussed on IRC (add license files if they are there)
* Make docs noarch as discussed on IRC
* Build tests in parallel

Probably we should also consider adding Provides: rustc (with version, %_isa and etc.)

Comment 10 Gwyn Ciesla 2016-07-27 17:58:43 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/rust

Comment 11 Josh Stone 2016-07-27 18:23:42 UTC
(In reply to Jon Ciesla from comment #10)
> Package request has been approved:
> https://admin.fedoraproject.org/pkgdb/package/rpms/rust

Thanks!

Do I need to wait for a bootstrap exception to proceed?
https://fedorahosted.org/fpc/ticket/643

Comment 12 Gwyn Ciesla 2016-07-27 18:28:13 UTC
Yes.  However that should be soon, especially since there's (IIRC) an FPC meeting tomorrow.

Comment 13 Peter Lemenkov 2016-07-28 10:33:05 UTC
*** Bug 915043 has been marked as a duplicate of this bug. ***

Comment 14 Josh Stone 2016-07-29 04:49:45 UTC
It appears FPC did not make a quorum today. :(

In the meantime, here's another update.  I made the changes for license text and a noarch -doc package.  I tried parallel %check, and while that does help them build, many of them execute in parallel threads, so my system was heavily oversubscribed while that was running.  I think it's better to keep that a serial make for now.

https://fedorapeople.org/~jistone/review/rust/v3/rust.spec
https://fedorapeople.org/~jistone/review/rust/v3/rust-1.10.0-3.fc26.src.rpm

Comment 15 Demi Marie Obenour 2016-08-01 02:18:21 UTC
Note that Rust statically links by default for good reason: there is absolutely no binary compatibility, since generic Rust code is always compiled as part of whatever code instantiates the generic.  So I would strongly recommend dynamically linking the compiler (only) and statically linking everything else – any changes to one component will require transitive recompilation of dependencies anyway.

Comment 16 Josh Stone 2016-08-01 15:45:07 UTC
Yes, I agree, Demi.  If you look at the spec %install, you'll see that I remove all of the *.so libraries from the installed rustlib/$target/ path, so folks using this compiler should only be able to link to the static rlibs.

Comment 17 Demi Marie Obenour 2016-08-01 21:28:57 UTC
I think that the dynamic libraries should still be available, for people who want to write code that uses plugins (such as the compiler itself, which is dynamically linked).  But any Rust program in Fedora that does not use plugins should be statically linked.

We should check to see if the Rust parts of Gecko can compile on stable Rust.

Comment 18 Josh Stone 2016-08-01 22:00:55 UTC
The libraries are still there in %{_libdir} for rustc and rustdoc only.  They are explicitly filtered from from requires/provides so we don't "leak" ABI.

Is there any plugin functionality in stable rust?  I thought this still required using feature tags, which implies nightly rust (or cheating the bootstrap key).  I don't think the distro should try to support any unstable features.

Not sure if it's what you mean, but I have confirmed that Firefox 48's mp4parse does compile just fine with stable rustc 1.10.0.

Comment 19 Martin Stransky 2016-08-03 06:12:45 UTC
Folks, it's really good to see this in Fedora, Thanks!

We will use that for Fedora Firefox builds when it's ready. Also Rust is going to be a base Firefox compiler, see https://bugzilla.mozilla.org/show_bug.cgi?id=1284816

Comment 20 Josh Stone 2016-08-03 18:17:50 UTC
Martin, I'm glad you want to use it!  Will it be ok for now if it's only enabled on i686/x86_64?  Hopefully you can enable it conditionally.  I do eventually want to expand to all Fedora archs, but upstream's Tier 1 is just x86.

Comment 21 Tom Tromey 2016-08-04 03:03:46 UTC
I looked briefly at the spec file.

I'd like to reiterate my desire for rust-gdb to "go away".
In an integrated system like Fedora, it isn't necessary.
The core of it looks like:

PYTHONPATH="$PYTHONPATH:$GDB_PYTHON_MODULE_DIRECTORY" gdb \
  -d "$GDB_PYTHON_MODULE_DIRECTORY" \
  -iex "add-auto-load-safe-path $GDB_PYTHON_MODULE_DIRECTORY" \
  "$@"


Here I think each part can be better done through system integration:

* PYTHONPATH is better handled by putting the rust/gdb python code
  into a directory where gdb and/or python already looks
* -d doesn't seem to be needed at all
* -iex isn't needed if the chosen directory is already safe according
  to gdb

Perhaps some minor patch to gdb would be appropriate here.
In this setup the rust-gdb package could still exist (maybe just
to avoid having rust depend on gdb); and even rust-gdb could still
exist, but just as a symlink to gdb.

Comment 22 Martin Stransky 2016-08-04 07:42:12 UTC
(In reply to Josh Stone from comment #20)
> Martin, I'm glad you want to use it!  Will it be ok for now if it's only
> enabled on i686/x86_64?  Hopefully you can enable it conditionally.  I do
> eventually want to expand to all Fedora archs, but upstream's Tier 1 is just
> x86.

Sure, x86 is a good start here, it's not mandatory for Firefox build now.

Comment 23 Josh Stone 2016-08-08 23:23:54 UTC
@tromey There's no existing path that's already in both auto-load safe-path and python sys.path.  I do seem to need "directory" (-d) too, or else it says:

  warning: Missing auto-load script at offset 0 in section .debug_gdb_scripts

I think I figured out how to set all three automatically, by adding a new file like /etc/gdbinit.d/rust.gdb containing:

  directory /usr/share/rust/etc
  add-auto-load-safe-path /usr/share/rust/etc
  python sys.path.append('/usr/share/rust/etc')

Does this look OK to you?


Maybe it would also be better to move this to "/usr/share/gdb/rust", so any future "etc"-like items don't infect gdb's paths.

Comment 24 Tom Tromey 2016-08-09 13:39:49 UTC
(In reply to Josh Stone from comment #23)
> @tromey There's no existing path that's already in both auto-load safe-path
> and python sys.path.  I do seem to need "directory" (-d) too, or else it
> says:
> 
>   warning: Missing auto-load script at offset 0 in section .debug_gdb_scripts

Note that gdb extends sys.path.
But maybe that's not enough.

> I think I figured out how to set all three automatically, by adding a new
> file like /etc/gdbinit.d/rust.gdb containing:
> 
>   directory /usr/share/rust/etc
>   add-auto-load-safe-path /usr/share/rust/etc
>   python sys.path.append('/usr/share/rust/etc')
> 
> Does this look OK to you?

It seems reasonable, though it also seems like gdb could perhaps be a bit
friendlier here somehow.

Comment 25 Josh Stone 2016-08-13 23:18:37 UTC
FPC approved the bootstrap exception, and now the builds are complete!  Rawhide is ready, and I will submit bodhi updates for F24 and F25 soon.

I ended up masking the %check result, because I kept getting spurious failures.  Different builds saw EAGAIN in a few different places, and otherwise failed with different values in tcp-stress.rs (see [1] and [2]).  Odd that %check never had an issue before on copr or koji scratch.  For now, it runs as far as possible with 'make -k' so check result will be logged, just not gating.
 [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=822325
 [2] https://github.com/rust-lang/rust/pull/33640

Then I also had to make -doc have an arch after all, because rpmdiff rejects the whole build if they produce any noarch differences.

I haven't applied the gdb tweaks to the actual package yet, but it's on my TODO list.  You're welcome to file a bug on that, especially if you want to pursue any changes in gdb to help this too.

Comment 26 Fedora Update System 2016-08-13 23:23:50 UTC
rust-1.10.0-4.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-08169a6354

Comment 27 Fedora Update System 2016-08-15 18:27:34 UTC
rust-1.10.0-4.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-08169a6354

Comment 28 Fedora Update System 2016-08-27 10:33:06 UTC
rust-1.10.0-4.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.