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

Excise FileWatching by moving pidfile to Base #51463

Closed
wants to merge 1 commit into from
Closed

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Sep 26, 2023

I am slowly redefining what it means to be a stdlib.
Previously we have used stdlibs as a way to organize code,
outside the monolith that is base/. With a move towards
stdlibs using pkgimages as their caching mechanism and them
becoming upgradeable, we have to redefine the relationship
between Base and stdlibs.

In my opinion Base should not have a direct dependency on
functionality in standard libraries. If we need something to be used by Base,
we must include the minimal set of functionality.
Functions in Base should either be universally useful,
or require some sort of compiler/runtime support.

Since we are not breaking functionality we must allow for
auto-loading/delayed loading of packages, but that mechanism,
must allow for the loading to fail.

Pidfile support was introduced in #44367, but it only uses
a watch_file from FileWatching.jl.

Here I moved pidfile.jl to base, removing the usage of FileWatching.
I decided against delayed initialization, since there would be a
rather annoying recusrion potential, and we don't want users to
disallow @stdlib and then suddenly have cache races again.

@vchuravy vchuravy added excision Removal of code from Base or the repository stdlib Julia's standard library labels Sep 26, 2023
@IanButterworth
Copy link
Member

The strategy sounds reasonable to me but I defer to @vtjnash about the removal of use of watch_file in Pidfile.

@IanButterworth
Copy link
Member

bump @vtjnash

@vchuravy vchuravy marked this pull request as ready for review November 9, 2023 14:02
@IanButterworth IanButterworth added the triage This should be discussed on a triage call label Nov 9, 2023
@LilithHafner
Copy link
Member

Triage didn't haven anyone who understood these code paths deeply but it seemed like a good idea from the outside.

  • Where does this functionality really belong?
  • Is it worth splitting the library (e.g. do we want pidfiles and file watching to be in the same place)

@LilithHafner
Copy link
Member

I'm a big fan of excision and want to make sure that this gets any support it needs, but I'm not sure how discussing this on triage would help. Feel free to re-add the label with a note about what specifically you would like discussed.

@LilithHafner LilithHafner removed the triage This should be discussed on a triage call label Jan 3, 2024
@vchuravy vchuravy closed this Mar 11, 2024
@vchuravy vchuravy deleted the vc/excise_fw branch March 11, 2024 23:59
@IanButterworth
Copy link
Member

IanButterworth commented Mar 12, 2024

Any particular reason for the close? (didn't mean to sound negative. Just interested)

@vchuravy
Copy link
Member Author

Mostly me just un-licking the cookie and acknowledging that I won't be able to work on this until I graduated

@KristofferC
Copy link
Member

I won't be able to work on this until I graduated

@vchuravy you graduated now, right? Reopen? 😆 😆

@IanButterworth IanButterworth restored the vc/excise_fw branch November 19, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
excision Removal of code from Base or the repository stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants