Skip to content

Remove epicscorelibs.path dependency on compiler#43

Merged
AlexanderWells-diamond merged 1 commit intomasterfrom
path-no-probe
Jul 22, 2025
Merged

Remove epicscorelibs.path dependency on compiler#43
AlexanderWells-diamond merged 1 commit intomasterfrom
path-no-probe

Conversation

@coretl
Copy link
Collaborator

@coretl coretl commented Jul 15, 2025

Before this commit, an import of epicscorelibs.path would compute some variables, including about the compiler, to determine the OS_CLASS. However this was unused except in tests. This commit removes epicscorelibs.path.OS_CLASS and replaces references to it with platform.system() instead.

Fixes #42, fixes #41

@coretl coretl force-pushed the path-no-probe branch 2 times, most recently from 98ec98d to 573058f Compare July 15, 2025 12:58
Copy link
Contributor

@AlexanderWells-diamond AlexanderWells-diamond left a comment

Choose a reason for hiding this comment

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

This appears to not work as expected as it does not suppress the logging messages; I still see the messages appear when following the instructions in this comment.

This is after a pip install -e . of this branch checkout.

@coretl
Copy link
Collaborator Author

coretl commented Jul 17, 2025

This appears to not work as expected as it does not suppress the logging messages; I still see the messages appear when following the instructions in this comment.

This is after a pip install -e . of this branch checkout.

The instructions provoke it by doing a direct import of epicscorelibs.config. However in production it is provoked by doing an import of epicscorelibs.path.pyepics, which no longer does an import of epicscorelibs.config internally...

@AlexanderWells-diamond
Copy link
Contributor

Ah understood, my reproduce wasn't accurate to the actual production issue. As I think further changes are incoming based on discussion in the issue, I'll hold off further review at the moment.

Copy link
Contributor

@AlexanderWells-diamond AlexanderWells-diamond left a comment

Choose a reason for hiding this comment

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

I think these changes are good, but I would like to see the commit message and PR title / description updated to better reflect what was done.

I'm also not sure why the version number appears to be marked as a change; the master version of setup.py definitely already has the version with a2 on the end...

Before this commit, an import of epicscorelibs.path would compute
some variables, including about the compiler, to determine the
OS_CLASS. However this was unused except in tests. This commit
removes epicscorelibs.path.OS_CLASS and replaces references to
it with platform.system() instead.
@coretl coretl changed the title Remove epicscorelibs.path dependency on ProbeToolchain Remove epicscorelibs.path dependency on compiler Jul 22, 2025
Copy link
Contributor

@AlexanderWells-diamond AlexanderWells-diamond left a comment

Choose a reason for hiding this comment

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

All looks good now

@AlexanderWells-diamond AlexanderWells-diamond merged commit a66d7f6 into master Jul 22, 2025
72 checks passed
@AlexanderWells-diamond AlexanderWells-diamond deleted the path-no-probe branch July 22, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract OS_CLASS into different function Logging on import may break downstream dependencies

2 participants