Skip to content

Conversation

@davidtaylorhq
Copy link
Member

We will soon start bundling many more plugins in Discourse core, which means that existing git clone lines in container YAMLs will need to be removed. This commit adds a more informative message for this situation.

We will soon start bundling many more plugins in Discourse core, which means that existing `git clone` lines in container YAMLs will need to be removed. This commit adds a more informative message for this situation.
@pfaffman
Copy link
Contributor

pfaffman commented Jul 9, 2025

Nice work! Better still would be to just have sed remove those automatically and have them run ./launcher again.

Something like this . . . (maybe commenting them out would be better?)

clean_bundled_plugins_from_config() {
  config_data=$(cat "$config_file")
  for plugin in "${BUNDLED_PLUGINS[@]}"; do
    if grep -q "git clone https://github.com/discourse/$plugin" "$config_file"; then
      echo "---"
      echo "NOTICE: The plugin '$plugin' is now bundled with Discourse and does not need to be in your container config."
      echo "Removing 'git clone https://github.com/discourse/$plugin' from $config_file"
      echo "For more information, see https://meta.discourse.org/t/373574"
      echo "---"
      sed -i.bak "/git clone https:\/\/github.com\/discourse\/$plugin/d" "$config_file"
    fi
  done
}

@davidtaylorhq
Copy link
Member Author

Nice work! Better still would be to just have sed remove those automatically and have them run ./launcher again.

I don't think we should be messing with people's config files without warning. They may wish to backup the file before making a change, or perhaps they version-control the config file directory (that's what we do on our own hosting). Plus, this check is very naiive, so there's a chance it could be a false-positive (e.g. if there was a git clone line commented out in the config file).

I'd be ok with this kind of auto-fix happing after a prompt, perhaps as part of discourse-doctor? But having it in launcher itself is too risky.

@pfaffman
Copy link
Contributor

pfaffman commented Jul 9, 2025

I don't think we should be messing with people's config files without warning

True enough. Maybe offer to do it automatically behind a switch, but then that's starting to be work.

And also, it is deceptively hard to do right. I've tried to do those edits in my ansible script (and also for plugins that change name/path) and have sometimes caused more problems than I fixed.

So, I guess you're right. :-)

@davidtaylorhq davidtaylorhq merged commit 2ddd90f into main Jul 10, 2025
5 checks passed
@davidtaylorhq davidtaylorhq deleted the plugin-clone-error-help branch July 10, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants