Implementation of a split crate scheme.

[afranchuk]

This splits the crates as a demonstration of what that might look like.

cc #255

[eminence]

Thanks for starting work on this!

For stuff like fn split_into_num() and trait FromStrRadix (which is used in both procfs-core and procfs), what did you mean by moving them into a module? I think I'm OK making these previously private stuff be moved into procfs-core and made public.

Github helpfully gives me access to your split-crates branch, so we can collaborate there, if you want; just let me know what you're working on, so I won't step on your toes.

[afranchuk]

For stuff like fn split_into_num() and trait FromStrRadix (which is used in both procfs-core and procfs), what did you mean by moving them into a module? I think I'm OK making these previously private stuff be moved into procfs-core and made public.

I just meant that if you wanted to keep them somewhat private, they could be namespaced under e.g. a "mod procfs-internal". But if you don't mind expanding the API by making them public, that makes it easier. I think most might actually be able to be kept private if the relevant things are moved over into procfs-core.

[eminence]

Ah, I see. For now, let's just make them public in procfs-core. Later we can consider applying #[doc(hidden)] and/or adding some comment like "this function isn't actually part of the public API". Putting them in an internal module would also help communicate that these are internal APIs, but this PR is going to be pretty big anyway, so let's iterate on this topic later

[afranchuk]

@eminence Take a look at what I've got so far. More stuff could be moved to procfs-core, but the current state demonstrates how the code is split (for instance see how process works now, that is in a "finished" state). In particular, see the traits that were added to procfs-core/src/lib.rs and procfs/src/lib.rs, which make a consistent interface for things.

This includes allowing the (new) SystemInfo to be provided explicitly to procfs-core, which procfs will default to the local system. That was originally a struct (the same as ExplicitSystemInfo), but I made it a trait object so that the ProcResult in functions were one-to-one as before (otherwise, e.g., you wouldn't be able to get ticks_per_second if boot_time_secs was unavailable for whatever reason, since an error would be returned).

Types which need SystemInfo to be created get it through a set of parallel traits.

Methods which need SystemInfo get it by returning an impl WithSystemInfo. I chose this approach (rather than creating extension traits for the types which need an alternate method in procfs) because it was more uniform and allowed the original method name to remain the same (otherwise for user convenience you'd want the method names to be different to avoid requiring the user to disambiguate when calling them).

[eminence]

(Just dropping a quick note to say that I haven't forgotten about this PR)

[afranchuk]

No problem, I know it's a lot to take in (and still isn't complete in moving stuff to procfs-core, though it's not far)!

[afranchuk]

Also for such a large changeset I recommend simply looking at the files. The diff view is way more distracting than helpful (I wish they at least supported side-by-side diffs).

[eminence]

I just pushed 2 new commits to your branch: one updates github CI, and the other removes some problematic doc comments (which were trying to link to procfs stuff from procfs_core). Let's see if this is enough to turn CI green

[afranchuk]

Yes, I should have mentioned that I wasn't particularly thorough with updating doc comments (especially because I think there should probably be some more involved rewriting for a few things like module documentation to make the organization clear).

[afranchuk]

It looks like the nightly failure is from the rustix crate?

[eminence]

Yeah, see https://github.com/rust-lang/rust/issues/109614 should be fixed soon

[sunfishcode]

Yes, it looks like the nightly failure is due to https://github.com/rust-lang/rust/issues/109424, which happens to hit rustix.

[afranchuk]

Aside from CI blinking lights, how does the crate organization look? Is this a direction you want to take the project? I only have this as a draft at this point because it would feel more complete to move the few missing things over correctly.

[eminence]

Yes, it definitely is. Since this is a big PR that's hard to review, my strategy/next steps for reviewing is this:

• Use one of the semver tools to see if/how the external API has changed in the procfs crate
• Migrate another tool of mine (which uses the procfs crate) to use this PR, as another way to confirm that the external API hasn't changed very much

Hopefully you've already been able to validate that the new procfs-core crate meets your own requirements for parsing procfs data on non-linux machines

[afranchuk]

Yeah, the procfs-core crate meets my needs. Using a semver tool is a good idea to sanity check the result.

[eminence]

I migrated my procdump project to using this PR, and it went pretty smoothly. I had to make only a few changes:

• The ProcessCgroup struct got renamed to ProcessCGroup
• The cgroups() function now returns a ProcResult<CGroupControllers> instead of a ProcResult<Vec<ProcessCgroup>>
• Things like stat.starttime() now require an additional call to with_system_info().

I also see that some functions like procfs::vmstat are gone, replaced by procfs::VmStat::current(). The introduction of the VmStat struct seems find, but for better API compatbility, it seems like we can still keep procfs::vmstat.

Since this PR is really quite large, my current inclination is to merge this, and then do additional iterations in additional merge requests.

[afranchuk]

It sounds like you hit most/all of the purposeful breaking changes! Those changes were mainly to improve consistency in the API and naming (e.g. ProcessCgroup was renamed because there was ProcessCgroup and CGroupController previously with difference capitalization).

I think it's fine to keep some of those standalone functions. I actually thought I had left a TODO somewhere mentioning that, but I don't see it.

I considered making individual extension traits in procfs to avoid the need for with_system_info(), but I felt that might be somewhat cumbersome in the API (not to mention annoying to write).

[afranchuk]

I ran a tool (cargo-semver-checks) to check compatibility. It seems like the tool has some bug with pub use, as it complained about a few enums and structs being removed when they actually are the same as they used to be (through a pub use of procfs-core). Otherwise with my latest commit there are no other valid complaints aside from that renamed struct (though I guess that exposes the limitations of that tool since it didn't catch the method changes).

[afranchuk]

I just finished moving everything else except the sys module, but there's really not much that would belong in procfs-core there (as it's written now).

[eminence]

Ok, let's get this merged, so other PRs can be rebased. Many thanks @afranchuk for working on this