Odin icon indicating copy to clipboard operation
Odin copied to clipboard

`core:os` tracking issue

Open laytan opened this issue 11 months ago • 16 comments

We are nearing the replacement of core:os, this issue outlines todos and nice to haves.

Todos:

  • [x] #3279
  • [x] #4712
  • [x] @gingerBill needs to look at the env API, env_posix.odin has a bunch of NOTE(laytan): comments outlining potential improvements, same thing on the directory iterator, these comments are mostly about surfacing errors instead of ignoring them or returning just a boolean (which core:os famously does a lot and we should avoid) (https://github.com/odin-lang/Odin/commit/b3bbb00f1ac9cc3db242c3af9690f6beedf046e8 & https://github.com/odin-lang/Odin/commit/79944056b9c24a0262a1038667f9a473a3ce2cd6)
  • [x] the IMPORTANT NOTE saying the package is a mockup needs to be removed from doc.odin
  • [x] write on linux needs to loop until everything is written, just like windows and posix implementations (#4730)
  • [x] linux does not do the MAX_RW thing other implementations do but afaik it does have the same restriction (#4730)
  • [x] remove on posix needs to handle directories, just like the other implementations (it already did, whoops)
  • [ ] unused File_Flag cases should be removed, .Sparse, .Unbuffered_IO
  • [x] core:path needs to be looked over and possibly removed/merged into os2 (#4954)
  • [ ] Flysand: create() does not allow to set the permission bits or specify the behavior when the file exists (.Trunc or .Excl). I would like to see truncate parameter to that procedure that is false by default. If it's not specified, .Excl is the behavior on existing files, otherwise existing file is truncated.
  • [ ] atomic bulk operations
  • [ ] bulk operations errors with more info on what actually went wrong and where, so instead of Permission_Denied, get a path back or something too
  • [ ] Errors are not detailed enough, often getting an .Unknown, this is because it is built on io.Error

Targets supported in core:os but not yet in core:os/os2:

  • [x] wasi (#4716)
  • [ ] js - just os.write is implemented, maybe it should just be dropped entirely for os2
  • [ ] haiku - should be pretty easy to use the posix implementation at first, can go "native" later

Non blocking but can have api consequences:

  • [ ] buffered file mode/flag (needs testing to see if it's beneficial)
  • [ ] Jeroen: Virtual File System - file possibly backed by not actual files

Packages that use core:os and need to be updated: (Started in https://github.com/odin-lang/Odin/pull/5858)

  • [x] core:compress/gzip
  • [x] core:crypto
  • [x] core:encoding/csv
  • [x] core:encoding/hxa
  • [x] core:encoding/ini
  • [x] core:encoding/xml
  • [x] core:flags
  • [x] core:fmt
  • [x] core:image/bmp
  • [x] core:image/jpeg
  • [x] core:image/netpbm
  • [x] core:image/png
  • [x] core:image/qoi
  • [x] core:image/tga
  • [x] core:image
  • [x] core:log
  • [x] core:math/big
  • [x] core:mem/virtual
  • [x] core:net
  • [x] core:odin/parser
  • [x] core:path/filepath (todo: only rel and dir to look at adopting or not in core:os/os2)
  • [x] core:prof/spall
  • [x] core:terminal
  • [x] core:testing
  • [x] core:text/i18n
  • [x] core:text/regex
  • [x] core:text/table
  • [x] core:time/timezone
  • [x] core:unicode/tools
  • [x] tests/core/encoding/hxa
  • [x] tests/core/flags
  • [x] tests/core/io
  • [x] tests/core/os
  • [x] tests/documentation
  • [x] vendor:fontstash
  • [x] vendor:libc
  • [x] vendor:opengl
  • [X] examples/all (for all of the odin checks at the moment, later also for documentation)
  • [ ] odin-lang/examples repo needs to be updated

Nice to have, but not blocking the replacement of core:os:

  • [ ] JS impl of core:fmt uses a 1k stack buffer to format into, add a way to grow if that's insufficient? Someone who uses wasm may want to look at this.
  • [ ] more tests
  • [x] docs additions, dir.odin, file.odin, etc. don't have any docs
  • [ ] docs unification, doc style differs between files and procs
  • [ ] linux process_info should query /proc/pid/exe instead of the way it's done now to get executable paths
  • [ ] netbsd, openbsd, freebsd targets implementation for _process_info_by_pid, _process_list, _process_open, _process_handle_still_valid, _process_state_update_times for full process API coverage (decided to make non blocking because it is a new API)
  • [x] current_executable_path(allocator: runtime.Allocator) -> (string, Error) returning the full absolute executable path of the program, useful for loading things relative to this path. You can already do this in a round about way with: current_process_info({.Executable_Path}, context.allocator) but a more direct way with the native APIs for each target would be nice (#4733)
  • [ ] wrapper api over the process API that handles the details of processes and just allows easy communication, similar to this idea: https://gist.github.com/laytan/68be38614d9274663a48fd3d710fefc2
  • [ ] a heap allocator that does not rely on any system libraries, ideally this goes into base: though, so it can be the default allocator in general (#4749)
  • [ ] core:testing to replace os2.stdout and os2.stderr with an implementation that synchronises between tests (currently writing to stdout or stderr in tests doesn't work well because the terminal renders progress and clears it out)
  • [ ] Flysand: There is no procedure to free a file handle without closing it (which can be used in tandem with new_file, if i.e. a library returned a handle, and that library is responsible for closing the file).
  • [ ] Flysand: It would be nice to have unlink :: remove alias.
  • [ ] process_kill is often too much (SIGKILL), would be nice to have another proc that does SIGTERM

After replacement the following can be closed:

  • #3325
  • #4264
  • #3154
  • #3186

laytan avatar Jan 17 '25 18:01 laytan

Virtual File System, working similarly to allocators, in that you call it with a Mode, instead of having to have vtable with all of stat , open , close , read ,write, etc.

Kelimion avatar Jan 17 '25 18:01 Kelimion

doc style differs between files and procs

Pretty sure I messed up files a little bit. I (or someone else) will have to rewrite the documentation at some point for files. process.odin should be the most recent and the most correct style, but still not ideal.

flysand7 avatar Jan 17 '25 22:01 flysand7

Virtual File System, working similarly to allocators, in that you call it with a Mode, instead of having to have vtable with all of stat , open , close , read ,write, etc.

I put it on the list under "non blocking but can have api consequences"

laytan avatar Jan 17 '25 23:01 laytan

I have a private branch that has a chunk of what's outlined here already done (for os->os2 conversion), including a test suite.

It's been a while since I touched it (~2 months), so I'll have to revisit what's changed since. The main blocker for me was rewriting @jasonKercher's heap allocator to be thread-safe, which took quite some time, debugging, research, and learning of new skills.

I'm aiming to get a draft PR of the new allocator up in a couple days. I'll need some help with testing and reviewing non-Linux system calls with regards to virtual memory, in particular.

I'm currently working through integrating it into base and fixing some cyclic import issues. I had hoped to avoid exposing too many new symbols in base:runtime by using a sub-package such as base:runtime/heap, but fixing the cyclic import would've been too much of a pain, so I'm just putting everything in base now. I'll be using a liberal application of prefixes so it's clear what's only for the heap API. @(private="file") wouldn't work either, because my allocator test bench uses the internals, and I think it's just as well for a low-level API to be available.

EDIT: To be perfectly clear, when I say draft PR, I mean that the allocator is largely finished from a design and implementation standpoint, but it's untested outside of microbenchmarks, my test bench, and the Odin core: test suite. If someone's willing to hook it up to a real application once the PR's up, that'd be great.

Feoramund avatar Jan 21 '25 00:01 Feoramund

Sounds great @Feoramund , excited to see it!

laytan avatar Jan 21 '25 18:01 laytan

Me too, and I'm very sorry ;]

jasonKercher avatar Jan 21 '25 22:01 jasonKercher

Some things I've noted when looking at os2/file API:

  1. create() does not allow to set the permission bits or specify the behavior when the file exists (.Trunc or .Excl). I would like to see truncate parameter to that procedure that is false by default. If it's not specified, .Excl is the behavior on existing files, otherwise existing file is truncated.
  2. open_buffered() is still commented out and .Unbuffered_IO flag is unused.
  3. There is no procedure to free a file handle without closing it (which can be used in tandem with new_file, if i.e. a library returned a handle, and that library is responsible for closing the file).
  4. It would be nice to have unlink :: remove alias.

flysand7 avatar Jan 22 '25 02:01 flysand7

Some things I've noted when looking at os2/file API:

  1. create() does not allow to set the permission bits or specify the behavior when the file exists (.Trunc or .Excl). I would like to see truncate parameter to that procedure that is false by default. If it's not specified, .Excl is the behavior on existing files, otherwise existing file is truncated.
  2. open_buffered() is still commented out and .Unbuffered_IO flag is unused.
  3. There is no procedure to free a file handle without closing it (which can be used in tandem with new_file, if i.e. a library returned a handle, and that library is responsible for closing the file).
  4. It would be nice to have unlink :: remove alias.

I added 1, 3 and 4 to the list, 2 was already there

laytan avatar Jan 22 '25 18:01 laytan

The heap allocator PR is up at #4749

Feoramund avatar Jan 24 '25 01:01 Feoramund

I've been working on this again lately, specifically reimplementing some of the functionality that filepath had into os2 directly in order to work around the cyclic import problem. I think filepath should be removed, possibly in favor of platform-agnostic procedures for converting between systems.

So far I've simplified the path splitting API into split_path(path) -> (dir, filename) and split_filename(filename) -> (base, ext), with a separate proc split_filename_all which returns the longest extension possible tar.gz instead of gz in the case of foo.tar.gz.

Feoramund avatar Feb 28 '25 00:02 Feoramund

Updated with the addition of #4954 🔥

laytan avatar Mar 24 '25 18:03 laytan

@laytan This issue also needs to be resolved for os2: #4999. I've written down the gist of it, and looked at os2 code for reading from console, and well, it is still there hidden behind a TODO.

https://github.com/odin-lang/Odin/blob/9516084e5806b7410cb0de28c1cbe56dce11891e/core/os/os2/file_windows.odin#L323

flysand7 avatar Apr 14 '25 09:04 flysand7

Experienced systems engineer but new Odin user here. Is there anyway I can help out with this effort?

deepankarsharma avatar Jun 24 '25 14:06 deepankarsharma

Hi @deepankarsharma. That's a very kind offer.

I'd like to suggest familiarizing yourself with Odin first and foremost, using what is in core:os/os2 already as if it was finished. Then raise any pain points you run into as points of improvement. We can discuss any suggestions you may have and whether those warrant a PR. Similarly, anything you come across that lacks documentation, or where documentation is sparse or unclear. That would be beneficial as well.

Kelimion avatar Jun 24 '25 15:06 Kelimion

Starting conversion of core:os usage in core:* in #5858.

Kelimion avatar Oct 27 '25 19:10 Kelimion

My os -> os2 remap is now on os-new. PR closed.

Kelimion avatar Nov 02 '25 14:11 Kelimion