-
Notifications
You must be signed in to change notification settings - Fork 993
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
Added endeavouros to the distro list in with_pacman
in oss.py #11970
#11971
Added endeavouros to the distro list in with_pacman
in oss.py #11970
#11971
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing
The integration that you are modifying is legacy, and shouldn't be used anymore.
The recommended one is in from conan.tools.system ...
Please have a look to https://docs.conan.io/en/latest/reference/conanfile/tools.html, and the place to do the modifications is in https://github.com/conan-io/conan/blob/develop/conan/tools/system/package_manager.py
@memsharded done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried this manually and it is working?
I thought it was ok, but I did a routine check on https://github.com/python-distro/distro/blob/master/src/distro/distro.py, and I don't think that distro.id()
will be returning the endeavouros
string. Am I missing something? Is distro
capable of returning this even if not documented explicitly in the project?
well this is why I had the previous change, I traced the code back to oss.py, though you said it was legacy, that's what led me to that file in the first place |
Actually I should be more specific, the initial commit to oss.py was traced back from OpenGL's recipe, tools which is imported from Give me a couple of more days to test this. If you have any ideas as to how to proceed please let me know. |
No, the code is ok, the change you did put the fix in the right place. I think the problem would be the same in the other legacy place as well. The underlying issue is that Conan uses the return of |
I had my hard drive fail, its going to take me a bit to get back to you, I will ping you as soon as I am able to, would be back up and running, but I have work, I may not be able to get back to this until this upcoming weekend |
@memsharded sorry for that week-long delay, got around to it, and yes, distro.id() on my OS the method returns correctly: >>> import distro
>>> distro.id()
'endeavouros'
>>> Is it safe to merge this in now? |
Changelog: Bugfix: Add "endeavouros" to the list of distros with pacman.
Docs: Omit
Close #11970
develop
branch, documenting this one.Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.
Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.