Final Comment: Automated build and submit pipeline

[philpax]

First DIP, yay!

This is a first draft with a few unanswered questions. I've left them vague so that we can specify them.

Would like input from @ayyaruq and @NotAdam if possible.

[NotNite]

Would a developer be able to test the build process to make sure it works before PRing? One concern of mine is merging a PR and then the plugin build fails in the action. Perhaps we could provide a standalone action runnable through act, or a Docker container.

also can we please make a cool dalamud logo ascii art for the script that'll build the plugins

[NotNite]

Also, it should be of note we set up dependencies properly. The script should make sure to clone Git submodules and install NuGet packages. We should also have it post webhooks to #github so we'll know if a build is failing.

[KazWolfe]

A general question, but how would we handle plugins with non-game dependencies? This DIP makes sense for self-contained plugins, but creates a bit of an interesting challenge when other things need to happen at (roughly) the same time as a new plugin version goes out.

Some plugins will require deployment of code to a dev-controlled webserver (e.g. Waitingway), others have plugins in other systems (e.g. XIVDeck). We could modify our own build processes to generate/do whatever we need to do and then bump the commit over in the Dalamud repos, but this still opens a question of certain issues with version de-synchronization (especially with breaking changes).

[karashiiro]

That shouldn't be an issue, this wouldn't run any plugin code. If the deployed code is a build requirement, that could be done as a build task, but if it's a runtime requirement then it shouldn't matter.

[karashiiro]

Of course, that also partially-undermines the point of this DIP, which is something else to think about.

[kalilistic]

A general question, but how would we handle plugins with non-game dependencies? This DIP makes sense for self-contained plugins, but creates a bit of an interesting challenge when other things need to happen at (roughly) the same time as a new plugin version goes out.

Some plugins will require deployment of code to a dev-controlled webserver (e.g. Waitingway), others have plugins in other systems (e.g. XIVDeck). We could modify our own build processes to generate/do whatever we need to do and then bump the commit over in the Dalamud repos, but this still opens a question of certain issues with version de-synchronization (especially with breaking changes).

May add some new complexity to those plugins but I think would still be manageable. I'm not saying we're at this stage yet, but if there are one or two "edge case" plugins - perhaps they could be moved to the "blessed second-party repositories" we've been talking about in a separate DIP.

[KazWolfe]

if it's a runtime requirement then it shouldn't matter.

I'd tend to agree, but I do want to bring it up because it does add a bit of thought to how/when/why things would be triggered on the Dalamud side for devs of plugins like this. These plugins are more likely to do Other Things™ as well that may cause varying degrees of headaches under the current proposal:

• Both Ava and I place all of our components in our respective repositories. We'd need to have some way to tell the build system which .csproj file to use and which to ignore. While a forced repository restructure would work here, I'm hesitant to propose that as a solution.
• It is completely possible for dependencies/other components to require different build environments for one reason or another. The Stream Deck Plugin, for example, builds inside of a Ubuntu GitHub Action. A client/server model may want to build with different dependencies in a different context. I don't think it's reasonable to push these build deps over to Goatcorp, nor require they handle external CI.
• As alluded to, plugins that inherently depend on external things (be they build-time or runtime) already somewhat break the entire point of this DIP. Goatcorp doesn't (can't) know what the library I'm building is doing necessarily, especially if the build is grabbing it from my Releases page in real-time. Similarly, a client-server plugin with a runtime dep can have behavior changed at any time.

If the solution to this problem becomes "trigger your own CI, do whatever you need to do, and then automatically submit a PR once you're ready", that's perfectly acceptable (at least to me).

perhaps they could be moved to the "blessed second-party repositories" we've been talking about in a separate DIP.

If this becomes a thing, I'd like some way for my plugin(s) to still show to all users for discovery purposes. Users "window shop" for plugins a lot; I'd hazard a guess that a fair number of installs come from "oh, cool, there's a plugin that can do this?" rather than specifically seeking out a function. Blessed repos should be default-visible, but I suppose that's a discussion for that DIP instead of this one

[philpax]

Would a developer be able to test the build process to make sure it works before PRing? One concern of mine is merging a PR and then the plugin build fails in the action. Perhaps we could provide a standalone action runnable through act, or a Docker container.

~also can we please make a cool dalamud logo ascii art for the script that'll build the plugins~

Fine by me! I'm also entirely fine letting people fail their builds on submission, failures happen all the time. Just need to make sure they're graceful :)

I'll make a note about making sure the action is compatible for local running...

[philpax]

I've updated the DIP to address all of the feedback mentioned here, as well as discussion in Discord. I expect the proposed changes to the csproj to be a little controversial, but the rest should be more or less acceptable. Let me know what y'all think!

[kalilistic]

Should probably add @daemitus to this one too given his work on the existing github action.

[KazWolfe]

As part of this process, we should add Actions to the PR process to verify that:

1. The plugin actually builds properly.
2. The resulting manifest contains all required fields (e.g. name, punchline, version)
3. The plugin passes a basic automated test suite (containing what? Signature validation? Security?)

[karashiiro]

Oh yeah, I'm not sure if it's useful or not but I just remembered I had this https://github.com/karashiiro/Dalamud-Checksummer/blob/main/Dalamud-Checksummer.ps1 that I was working on a while back.

[ayyaruq]

I'll make a note about making sure the action is compatible for local running...

Generally stuff that relies on GitHub specific APIs (like cache) doesn't work in act, anything else should be gucci.

[kalilistic]

Maybe I got excited and approved early. Looks like this is pending some cleanup by @philpax based on the additional comments.

[philpax]

Just updated with latest round of feedback. Instituting informal final comment period closure in a week's time, so the 31st. Get your feedback in now!

[karashiiro]

DIP number in the filename? Looks like it's still 0000.

[philpax]

goat raised the very important issue of ensuring that only the right people get to submit manifest updates for their plugins. Still discussing best way to do that, but no-merge until that's in the DIP...

[NotAdam]

just pull the userid from the gh api?

[philpax]

just pull the userid from the gh api?

How do you know which user ids are valid, especially for multi-contributor projects?

Anyhow, the solution I've settled on for now is to let people specify usernames for owners in the manifest they submit, and the pre-merge hook will just replace the usernames with IDs + username comment. Should handle changing usernames + future access control relatively easily, and we can always change it if it doesn't work.

Just need to write it up...

[wolfcomp]

Just gonna quickly comment, is it possible to maybe extend the automated pipeline to be a sort of plug and compile GitHub CI/CD system instead of locked to a specific NuGet package is required. That would require a specific file output for the upload instead of a specific build system. This would possibly take a little longer to make, but would open up the gates a little for how many drawbacks there are going to be otherwise being locked to one specific NuGet package.

Making this comment because DalamudPackager errors on my system complaining about some files missing when it's not missing.

[lmcintyre]

Just gonna quickly comment, is it possible to maybe extend the automated pipeline to be a sort of plug and compile GitHub CI/CD system instead of locked to a specific NuGet package is required. That would require a specific file output for the upload instead of a specific build system. This would possibly take a little longer to make, but would open up the gates a little for how many drawbacks there are going to be otherwise being locked to one specific NuGet package.

Making this comment because DalamudPackager errors on my system complaining about some files missing when it's not missing.

I would assume this should be something to look into (more descriptive errors) in DalamudPackager rather than rethinking the process. There is definitely a file missing in your case - it's just extremely difficult to determine where, because the error message just says that it couldn't find a file.

[wolfcomp]

Just gonna quickly comment, is it possible to maybe extend the automated pipeline to be a sort of plug and compile GitHub CI/CD system instead of locked to a specific NuGet package is required. That would require a specific file output for the upload instead of a specific build system. This would possibly take a little longer to make, but would open up the gates a little for how many drawbacks there are going to be otherwise being locked to one specific NuGet package. Making this comment because DalamudPackager errors on my system complaining about some files missing when it's not missing.

I would assume this should be something to look into (more descriptive errors) in DalamudPackager rather than rethinking the process. There is definitely a file missing in your case - it's just extremely difficult to determine where, because the error message just says that it couldn't find a file.

The error was in an outdated structure of SamplePlugin that was fixed with #12 that I followed to how I set up the project.

[philpax]

Yup, we sorted out Wolf's issue on the Discord (thanks for pointing it out!) Wolf's mentioned that they're happy with this now that their issue's been fixed 👍

As I mentioned on the Discord, my preference is to make the happy path trouble-free than to provide ways off the path. Happy to revisit once we get some more experience, though!