Skip to content

Pass root directory URL as env variable so that plugins can use it#1322

Closed
JaewonHur wants to merge 1 commit intoapple:mainfrom
JaewonHur:add-plugin-state-root
Closed

Pass root directory URL as env variable so that plugins can use it#1322
JaewonHur wants to merge 1 commit intoapple:mainfrom
JaewonHur:add-plugin-state-root

Conversation

@JaewonHur
Copy link
Contributor

This PR adds a PluginStateRoot class so that a plugin can figure out its root URL where the plugin specific data should be stored.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

[Why is this change needed?]

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

Copy link
Contributor

@jglogan jglogan left a comment

Choose a reason for hiding this comment

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

Using request changes to prevent merge for one sec - why can't the plugin root be determined by convention as ${APP_ROOT}/plugin-state? What is the use case for needing to locate it in some arbitrary location?

@JaewonHur
Copy link
Contributor Author

I guess the reason I pass it as env variable is that we get pluginStateRoot as an argument in registerWithLaunchd (Sources/ContainerPlugin/PluginLoader.swift:L212).

I was not sure it should be always the default path.

@jglogan
Copy link
Contributor

jglogan commented Mar 17, 2026

I think we only need environment variables for our base directories:

  • The installation base for the application.
  • The base application data directory.
  • A base directory for logging, when writing file based logs (this is really a develop/debug capability, not good for production as the logs grow unbounded)

@JaewonHur JaewonHur closed this Mar 18, 2026
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.

3 participants