Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Pathconf names#4508

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
youknowone merged 2 commits intoRustPython:mainfrommarvinmednick:pathconf_names
Feb 21, 2023
Merged

Conversation

marvinmednick
Copy link

Implementation for os.pathconf_namesresolves#4494

@marvinmednick
Copy link
Author

marvinmednick commentedFeb 16, 2023
edited
Loading

Let me know if I did something incorrectly -- I see as part my pull requet there are commits authored by others.... The two changes I'm looking to pull in for pathconf_names ared8fe778 anddf48266

@fanninpm
Copy link
Contributor

After CI completes, feel free to do agit rebase ... followed by agit push --force.

youknowone reacted with thumbs up emoji

@youknowone
Copy link
Member

I rebased it.git rebase is very useful tool to send patches.
https://docs.gitlab.com/ee/topics/git/git_rebase.html

@marvinmednick
Copy link
Author

I rebased it.git rebase is very useful tool to send patches.https://docs.gitlab.com/ee/topics/git/git_rebase.html

Thanks -- I was looking at trying to do that but it didn't seem to look right as I still ended up with 2 additional changes -- I was thinking my baseline wasn't quite right or I did something incorrectly.

@marvinmednick
Copy link
Author

marvinmednick commentedFeb 20, 2023
edited
Loading

I'm looking at the error -- I found an online free online macos simlulator and was able to get the error 22 manually trying the command os.pathconf('/',21) but that isn't conclusive. (pathconf worked for indexes 0-20).
I'd like to check the the PathconfVar entries for darwin... but darwin doesn't seem to be a confguration check within PathconfVar

It looks like there are some platforms which may have pathconf indexes that are greater then 20 (e.g. in libc
libc: src/unix/haiku/mod.rs:pub const _PC_REC_MAX_XFER_SIZE: ::c_int = 32;
though on linux the same parameter is index 15.
libc src/unix/linux_like/linux/mod.rs:pub const _PC_REC_MAX_XFER_SIZE: ::c_int = 15;

I suppose this is more a of libc specfic question does anyone know how can I tell which of the libc flavor will be used on macos/darwin? (and therefore which pathconf indexes value will show up in PathconfVar)

Clarifying -
Change is now failing due to test running on darwin (which started running when making the change to the conditional to allow it to run on darwin. It suspect that one of the entries in PathconfVar isn't valid on that platform - the code takes all the variants in enums and converts them to the dict form. The test verifies the the value that one gets when using the name is the same as the value when using the index which should always be true, However the test is failing when it gets an error value as the parameter -- which I belive is coming from the enum.

As a result I suspect there may be a value in PathconVar which isn't valid for pathconf on that platform.... (and causing the error)

The test could be simplified to only check a few manually selected values rather , but if this is actually I think it would be better to fix PathconfVar to have the correct values for the platform.

Copy link
Member

@youknowoneyouknowone left a comment
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

You don't need to fix every macOS issue if if you don't have macOS machine. Please fill free to ask help if you have hard trouble to fix.

I think adding it only for linux is a working option now. I made a change about it. We can fix macOS issue later.

Thank you for contributing!

@@ -1881,6 +1883,24 @@ pub mod module {
pathconf(PathOrFd::Fd(fd), name, vm)
}

// TODO: this is expected to be run on macOS as a unix, but somehow not.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I put linux cfg until it is fixed for macos

@youknowoneyouknowone merged commit746cb04 intoRustPython:mainFeb 21, 2023
@youknowone
Copy link
Member

Thank you for contributing!

@marvinmednick
Copy link
Author

Thanks!

@marvinmednickmarvinmednick deleted the pathconf_names branchFebruary 26, 2023 22:53
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@youknowoneyouknowoneyouknowone approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhance compatibility of os.pathconf_names
4 participants
@marvinmednick@fanninpm@youknowone@mmednick

[8]ページ先頭

©2009-2025 Movatter.jp