samuela / rustybox

A free-range, non-GMO fork of busybox in 100% Rust 🦀
811 stars 33 forks source link

Rustybox won't compile with latest nightly toolchain #35

Open axonasif opened 3 years ago

axonasif commented 3 years ago

So I was trying to compile rustybox and obviously, I have the latest nightly toolchain. But I'm afraid that the build failed viciously :sweat_smile: I wonder, would it require the nightly toolchain which was present back at the time of the last commit made on May 31, 2020 ?

I've attached the log below. log.txt

samuela commented 3 years ago

It looks like all the errors are of the form

error[E0599]: no method named `wrapping_offset_from` found for raw pointer `*mut i8` in the current scope
   --> util_linux/mount.rs:950:42
    |
950 |     O_len = strchrnul(O_opt, ',' as i32).wrapping_offset_from(O_opt) as libc::c_long as libc::c_int;
    |                                          ^^^^^^^^^^^^^^^^^^^^ help: there is an associated function with a similar name: `wrapping_offset`
    |
    = note: try using `<*const T>::as_ref()` to get a reference to the type behind the pointer: https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref
    = note: using `<*const T>::as_ref()` on a pointer which is unaligned or points to invalid or uninitialized memory is undefined behavior

I'm not exactly sure what we're supposed to be using in place of wrapping_offset_from these days. Does anyone know what the appropriate refactor is? It looks like https://github.com/rust-lang/rust/commit/467415d50cdf8a0d15ec19dc63251443b35d4cee is the commit that deprecated it.

We could also try recompiling some of this stuff with the latest c2rust which ostensibly has been updated to address this.

skyne98 commented 2 years ago

@samuela, I might have figured it out from the changes in between old and new documentation. It seems like a pattern of x.wrapping_offset(y.wrapping_offset_from(x)) should be replaced with x.wrapping_offset((y as isize) - (x as isize)).

I tried to write an automatic refactoring script using JS + Deno + Regex, but it ultimately is too simple to proper replacements in some complicated cases, though it does work on most simpler ones.

Script:

import { walk } from "https://deno.land/std/fs/mod.ts";

const iter = walk('./');
for await (const e of iter)
    if (e.isFile && e.path.includes('.rs')) {
        let fileText = new TextDecoder().decode(await Deno.readFile(e.path));
        console.log('Looking at file ' + e.path);

        let regex = /(\(.+\)|[A-z]+)[\n ]*.wrapping_offset_from\((.+)\)/gi;
        console.log('Does the file contain the issue? ' + regex.test(fileText));
        let fixedText = fileText.replace(regex, '$1.wrapping_offset((($2) as isize) - (($1) as isize))');

        // Write the fixed text back to the file
        await Deno.writeFile(e.path, new TextEncoder().encode(fixedText));
    }

To run: deno run --allow-read --allow-write fix.js.

I hope you find this information helpful to succeed where I failed 😼 Let rustybox live!

skyne98 commented 2 years ago

I am also hoping to make a small distro using it soon, so I hope this can be resolved! There is no other project like this, it seems!

samuela commented 2 years ago

Hi @skyne98, that's awesome! Super excited to hear that you're interested in building a distro around it. That was one of my initial motivations when starting the project, but I haven't gotten around to it.

The dependency on nightly features has led to this unfortunate bit rot. I think that the best path forward for rustybox may actually be to re-run c2rust on a more recent version of c2rust and busybox. Automating that process may be the best way to stay up-to-date with changes in nightly features. Granted, getting this working the first time was a significant PITA.

Another option is to start rewriting things in rust. A lot of the basics can already be imported from https://github.com/uutils/coreutils.

skyne98 commented 2 years ago

It would be nice to keep things up to date with the most recent BusyBox and c2rust automatically, however, one problem arises in my mind: how do we make sure to only covert the parts which weren't rewritten?

Also, I am not familiar with the workflow of c2rust, so sorry I am currently no help on this front 😐

samuela commented 2 years ago

It would be nice to keep things up to date with the most recent BusyBox and c2rust automatically, however, one problem arises in my mind: how do we make sure to only covert the parts which weren't rewritten?

Yes, this is the fundamental question. I think the key here is to do it on a per-utility basis, eg. transpile "mkdir" but not "ls", etc.

Don't worry, c2rust is surprisingly approachable! I won't go so far as to say that it's straightforward, but it was easier than I expected.

skyne98 commented 2 years ago

In that case, there should be some simple build system in place. The whole binary should be clearly split-up and somehow marked as rewritten (or not). The build system will rely on this information to make decisions about if to transpile. I guess it can be as simple as having some mark file near the source code file, or some specific comment at the first line of each source code file.

But again, no idea how easy or hard it is to do granular transpilations with c2rust, you are the expert here! 😄

samuela commented 2 years ago

yeah I think something like that makes sense. IIRC busybox has build flags to enable/disable each possible sub utility. That's prob the way to go in terms of configuring which ones to transpile. Based on my previous experience I'm guessing that transpilation is likely not stable enough yet to just be another part of the build process; in my experience it required some manual fixes after transpilation to get things to compile.

skyne98 commented 2 years ago

Probably will have some time to look into how c2rust works on the weekend, but if it's possible to transpile a certain file, then we can set up a cargo make or something similar with commands to regenerate a specific command. Then we can also make some helpers to regenerate a set or all of them. We might also store a CSV (or JSON, or TOML) file of all converted commands, which this helper can easily parse and exclude from the list of things to transpile.

If I get some time, I could have and would have preferred to do it using Deno + TS, but I guess it would be too much of an unnecessary dependency, what do you think? Although, people will rarely need to transpile, so I guess it can be fine.

samuela commented 2 years ago

I think best to start with something simple. First things first let's see if it's even possible to automate the whole c2rust transformation. And if so we can move from there to figure out the best way to flag utilities on and off. I'm confident we can get the configuration story figured out one way or another

skyne98 commented 2 years ago

Yep, we can go Some build system -> Check the state file -> c2rust -> Apply patch fix manual fixes and try to do it for a single utility with a simple bash file for the first run. This way, we can just update a patch file whenever the manual changes required change.

Whenever I get to it, I will test out some options. So, would you mind using something like Deno as a build dependency (or more of a generation dependency)? Also, it would be cool if you could show an example of transpiling one util

samuela commented 2 years ago

I'd like to keep the core build system -- eg cargo build -- pure rust/cargo as much as possible. But I'm open to having some nix-shell scripts that semi-automate the transpilation process as well. I'm not picky about how those scripts are written as long as they work with nix-shell!

skyne98 commented 2 years ago

Hah, I see! I kind of got my hands dirty with nix recently, replicating its derivation system as the package management system for the distro project I am working on, so it might be fun diving even deeper to understand it better! 😄