Investigate to use thiserror in our crates

[utam0k]

Introduction

Now we are using anyhow everywhere. But It would be better to provide some codes as crates(e.g. libcontainer, libcgroup...). Their crates in their current state are not very user-friendly of error handling.

Goal

Investigate and implement to use of thiserror instead of anyhow in crates

Tracking

Let's do the following order because of the number of codes.

• liboci-cli We don't need use anyhow in it
• libcgroups Adopt thiserror for libcgroups #1872
• libcontainer
  • Implement thiserror for libcontainer - Part 1 #1876
  • Implemented more thiserror for libcontainer (Part 2) #1881
  • Implement thiserror for libcontainer - Part 3 #1895
  • Implemented thiserror for libcontainer - Part 4 #1912
  • implemented thiserror for containers - Part 5 #1930
• remove anyhow Finally, remove anyhow from the libcontainer dependency. #1937

[utam0k]

@chermehdi I'm happy if you have interested in this issue. But this issue is not easy and take a lof of time to achieve better than other good first issues.

[chermehdi]

Sounds good, let us do them in that order, the first one should give us a pretty good idea about how will the others look like, and how much effort is required as well.

[utam0k]

@chermehdi That's right, libcontainer is going to be quite large task, so if there is a way to make it divisible that would be nice too. May I assign you? There is no deadline, etc. Of course, I can help you whenever.

[chermehdi]

@utam0k yeah sounds good, just assing me.

[chermehdi]

@utam0k liboci-cli only contains arg definitions for the cli implementation. I don't see any behavior that requires error handling, I am not sure why it was included as part of this issue. Can you clarify?

[utam0k]

@utam0k liboci-cli only contains arg definitions for the cli implementation. I don't see any behavior that requires error handling, I am not sure why it was included as part of this issue. Can you clarify?

Ops, sorry. You don't need to take it. Let's look into libcgroups.

[chermehdi]

@utam0k Where do we want to use thiserror? I suggest to minimize the changes we only introduce thiserror usage right at the edge of the library and keep error handling internally using anyhow/context. this will minimize the number of changes we have to make.

Another thing that I want us to think about is what is our API boundary, and how we want this to be used as a library.

• Do we want to provide libcgroups as it's own crate? or will we provide Youki as a library as a whole with the API like createFromSpec, start, stop?

[utam0k]

@chermehdi Sorry for the late reply.

💯

I suggest to minimize the changes we only introduce thiserror usage right at the edge of the library and keep error handling internally using anyhow/context. t

Ideally, yes. Once this is possible, it can be spun out and offered like containers/oci-spec-rs.

Do we want to provide libcgroups as it's own crate?

[squili]

Has there been any progress on this? If not, I'd be happy to help!

[utam0k]

👋 Hi, @squili. Unfortunately, we don't have any progress. I would like to introduce thiserror with libcgroup and libcontainer. Are you interested?

[squili]

Sure, I'd love to help with that!

[utam0k]

Which crate would you like to choose?

[squili]

Looks like libcgroup needs to be first, since it's a dependency of libcontainer

[utam0k]

@squili
I have given you an issue. If possible, could you please generate a plan and create a list of tasks to be done? This can be made later. Thank you.

[utam0k]

@chermehdi If you have made any progress, please let me know. I know you are probably very busy.

[squili]

Here's the current plan:

• Add thiserror
• Add thiserror errors
  • Controllers
    • libcgroups::systemd
    • libcgroups::v1
    • libcgroups::v2
  • Stats providers
    • libcgroups::v1
    • libcgroups::v2
  • Managers
    • libcgroups::systemd
    • libcgroups::v1
    • libcgroups::v2
  • Other non-test
• Remove anyhow

[utam0k]

@squili Do you want to create a feature branch?

[squili]

I can make a branch on a fork and PR

[squili]

Finished work for libcgroups, ready to review! It still uses anyhow in tests because it's useful for tests, but it's been removed as a dependency for everything else.

[yihuaf]

@squili Sorry I did not catch this thread. I made #1876 and hope I did not step on your toes.

[utam0k]

@yihuaf May I ask you to create TODOs list and share them with @squili ?

[yihuaf]

Here we go.

• container
• process (partly done)
• rootfs
• seccomp
• workload
• apparmor
• capabilities
• config
• hook
• notify_socket
• signal
• tty
• utils

At the top level, we should have a concrete error type in lib.rs to make libcontainer a stand alone library.

Finally

• remove anyhow

@squili Let's just leave a note in this thread to coordinate :)

[squili]

@yihuaf ill make sure to leave a note for the things i work on! though im going through a few things right now so it may be a bit

[yihuaf]

I plan on to work on these today:

apparmor
capabilities
config
hook
notify_socket
signal
tty

#1881

[YJDoc2]

@yihuaf and @squili , thanks for your efforts! May I ask you to add the related PR links (existing and future) in the issue description itself? I think it will help in keeping track of which all PRs were part of this change.