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

civix repair? #252

Closed
artfulrobot opened this issue Jul 26, 2022 · 4 comments
Closed

civix repair? #252

artfulrobot opened this issue Jul 26, 2022 · 4 comments

Comments

@artfulrobot
Copy link

The mixin thing is really clever, but I do keep breaking extensions, leading to situations where I'm not sure how to get back to a working state.

I think this comes from things like async changes: extension changed on site A and site B separately, then merge the changes and it breaks. Typically, I think it's like:

  • A
    • some civix interaction, e.g. add a page.
  • B
    • some civix interaction,maybe an upgrade.

Then merge. I've ended up with functions in extension.php which want to call out to _civix_something() in extension.civix.php which no longer exists. e.g. on one extension, that uses settings, the hook_civicrm_alterSettingsFolders was left calling a non-existant civix function. I didn't want to remove that because presumably the settings would not be picked up, then, but I didn't know how to add the mixin for settings either.

mixin contains mixin files generated by Civix. mixin files are not (yet) documented.
https://docs.civicrm.org/dev/en/latest/extensions/structure/#extension-directory-structure

It leads to a situation where it kills Civi until you can resolve it. I've sometimes found removing the civix version from info.xml and re-running civix upgrade provides a fix.

Anybody else had this trouble and got any tips? Is civix repair a non-sense idea?

@totten
Copy link
Owner

totten commented Jul 27, 2022

@artfulrobot It might be a good idea. Let me share a couple related thoughts first:

  • In the current implementation, there is a de-facto requirement to run civix upgrade before running other commands. The problem is that many commands will regenerate files from the latest template regardless (eg they'll regen *.civix.php). Doing this separately (without a prior civix upgrade) leaves the code in an inconsistent state (eg stale references to hook_alterSettingsFolders; eg missing the replacement settings-php).
  • I suspect this usability issue is at the root of civix the hook_civicrm_menuXml is gone #244, generate:entity-boilerplate clean up issues #250, civix repair? #252. I've probably hit it once or twice on my own.
  • In generate:entity-boilerplate clean up issues #250 (comment), there's some discussion about a way to address it.
  • I'm open-minded that there could be other flows that leave inconsistencies. But I don't know of any -- so I can't really comment about that. (I'd need to see some example commands or example repos.)
  • I have sometimes flirted with the idea of civix advisor or civix refresh command -- as an alternative to civix upgrade.
    • Both civix upgrade and civix advisor are conceived as migration-support, but they would be framed a bit differently. I wouldn't want to maintain two implementations of "migration-support".
    • Comparison:
      • civix upgrade would (does) track a <format> version. It checks the start version and then walks through each incremental update. It makes suggestions and applies changes.
        • (Ex: "If you start with format<22.05, then it will prompt you about removing alterSettingsFolders and enabling setting-php. After you set format to 22.05, it will won't bother you about that")
      • civix advisor would scan the codebase for tell-tale signs of old/new code patterns. Based on the patterns that are present or missing, it would makes suggestions.
        • (Ex: "If your extension has *.setting.php, then it will prompt you about enabling setting-php. If it lacks *.setting.php, then it will prompt about disabling setting-php. In either case, it will prompt about removing alterSettingsFolders.")
    • There's probably a lot of overlap in how these two would work. In writing upgrade rules, I've tried to use/consider a lot of guards/edges (as one would for advisor rules) -- and I suspect most could be converted, although I'm not certain.
    • Maybe there are some trade-offs? Maybe trade-offs in questions like:
      • How deeply does it have to scan? How does that perform?
      • When implementing a check/migration... how many edge-cases do you have to consider?
      • How opinionated is it? Does it strongly push ext's to be organized the same? Does it allow them to evolve in unforeseeable ways?
      • How well does it heal from mistakes or unexpected code/patterns?
      • Can it (should it / how does it) perform a pre-flight validation? (eg "Don't run civix commands unless you know the starting-point is clean")

Anyway, that's a bit of rant. I need to stop typing. :)

It leads to a situation where it kills Civi until you can resolve it. I've sometimes found removing the civix version from info.xml and re-running civix upgrade provides a fix.

I wonder if it would make sense to have an option like civix upgrade --reset or civix upgrade --all or civix upgrade --from=<version>?

@totten
Copy link
Owner

totten commented Jul 29, 2022

@artfulrobot I've added an option civix upgrade --start=0 which will replay all the upgrade checks.

Closing because #254 and #255 should reduce the incidence a lot. Released v22.07.2.

There could still be scenarios for something like civix repair or civix advisor, but I would need more concrete examples. Feel free to reopen or file related issues.

(An ideal report is probably something like "Clone the repo https://foobar.git, run these two commands, and see an error".)

@totten totten closed this as completed Jul 29, 2022
@artfulrobot
Copy link
Author

Thanks @totten, makes sense.

there is a de-facto requirement to run civix upgrade before running other commands.

I didn't know that, I suspect that's where I've fallen down.

Wouldn't it make sense to either:

  1. internally call upgrade before calling whatever the user specified (e.g. generate:angular-module: that way you'd ensure the code was up-to-date, or refuse to run.

    or

  2. check the info.xml <civix-version> <= running civix version before running a command, and refuse to run if not, suggesting a civix upgrade is run?

@totten
Copy link
Owner

totten commented Jul 29, 2022

@artfulrobot Yup,#2 is exactly what #254 implements. 😄

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

No branches or pull requests

2 participants