Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance compatibility of os.pathconf_names #4494

Open
marvinmednick opened this issue Feb 12, 2023 · 6 comments · Fixed by #4508
Open

Enhance compatibility of os.pathconf_names #4494

marvinmednick opened this issue Feb 12, 2023 · 6 comments · Fixed by #4508

Comments

@marvinmednick
Copy link

Placeholder to track the implmentation of os.pathconf_names -- additional detail to follow
(related to Extend the os module #1175)

@marvinmednick
Copy link
Author

marvinmednick commented Feb 14, 2023

Here is the approach:
The pathconf data is stored in PathconVar Enum, which is configured 'manually' by setting the value constants from libc (which is the integer value needed for pathconf) Since its a simple enum (no data associated) each enum variant can converted to an integer representation using 'as u8/u16/...'

I'd want the code to iterate through Enum creating the data and create the string and the integer value to build the dictionary.

I initially looked at a proc_macro to do this, but then saw that pathconf uses a create strum_macro::EnumString to the variatnt name as string and there is another option EnumVariantNames which already support iterating through the different variants as str references (&&str).

And the code will look like this (I followed the os.envion dictionary as an example):

      #[pyattr]
      fn pathconf_names(vm: &VirtualMachine) -> PyDictRef {
          let pathconf_name_dict = vm.ctx.new_dict();
          // iterate through each item 
          for variant in PathconfVar::VARIANTS {
              // get the name of variant as a string to use as the dictionary key
              let key: PyObjectRef = vm.ctx.new_str(variant.to_string()).into();\
              // get the enum value itself  and convert it to an integer for he dictionary value 
              let value: PyObjectRef = vm
                  .ctx
                  .new_int(PathconfVar::from_str(variant).unwrap() as u8)
                  .into();
              pathconf_name_dict.set_item(&*key, value, vm).unwrap();
          }
          pathname_name_dict
      }

@youknowone youknowone reopened this Feb 21, 2023
@youknowone youknowone changed the title Implement os.pathconf_names Implement os.pathconf_names for non-linux unix Feb 21, 2023
@youknowone
Copy link
Member

youknowone commented Feb 21, 2023

#4508 implemented linux version of this function.
Because this implementation is using libc function, I expect it can be somehow also work on macOS

@naonus
Copy link
Contributor

naonus commented Feb 23, 2023

I will take this issue

@naonus
Copy link
Contributor

naonus commented Feb 23, 2023

Env: macOS11.5.2( Big Sur ), Intel CPU
I tried test it and shared the result blow. It worked well

{'PC_LINK_MAX': 1, 'PC_MAX_CANON': 2, 'PC_MAX_INPUT': 3, 'PC_NAME_MAX': 4, 'PC_PATH_MAX': 5, 'PC_PIPE_BUF': 6, 'PC_CHOWN_RESTRICTED': 7, 'PC_NO_TRUNC': 8, 'PC_VDISABLE': 9}

@youknowone
Copy link
Member

On macOS,

CPython result:

{'PC_ALLOC_SIZE_MIN': 16,
 'PC_ASYNC_IO': 17,
 'PC_CHOWN_RESTRICTED': 7,
 'PC_FILESIZEBITS': 18,
 'PC_LINK_MAX': 1,
 'PC_MAX_CANON': 2,
 'PC_MAX_INPUT': 3,
 'PC_MIN_HOLE_SIZE': 27,
 'PC_NAME_MAX': 4,
 'PC_NO_TRUNC': 8,
 'PC_PATH_MAX': 5,
 'PC_PIPE_BUF': 6,
 'PC_PRIO_IO': 19,
 'PC_REC_INCR_XFER_SIZE': 20,
 'PC_REC_MAX_XFER_SIZE': 21,
 'PC_REC_MIN_XFER_SIZE': 22,
 'PC_REC_XFER_ALIGN': 23,
 'PC_SYMLINK_MAX': 24,
 'PC_SYNC_IO': 25,
 'PC_VDISABLE': 9}

RustPython result:

{'PC_CHOWN_RESTRICTED': 7,
 'PC_LINK_MAX': 1,
 'PC_MAX_CANON': 2,
 'PC_MAX_INPUT': 3,
 'PC_NAME_MAX': 4,
 'PC_NO_TRUNC': 8,
 'PC_PATH_MAX': 5,
 'PC_PIPE_BUF': 6,
 'PC_VDISABLE': 9}

@youknowone youknowone changed the title Implement os.pathconf_names for non-linux unix Enhance compatibility of os.pathconf_names Feb 27, 2023
@youknowone
Copy link
Member

This comment is useful to understand missing variables. #4571 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants