Run civix upgrade
Merge request reports
Activity
@jaapjansma are you able to merge this - without it Smarty3 + rules gives red noise & will break for people who switch to Smarty3
@jaapjansma I'm curious to understand that story. (To make civix better and to resolve the problem in your extension.)
The thing is... the
civix upgradecommand is supposed to be upfront and warn you before doing things like "Raise requirements to version XYZ". It would be good to figure out if the problem was:- (a) The upgrader failed to give advice about the compatibility-change. (Then I really want to know the specific problem/break - so it can give better advice.)
- (b) The upgrader showed the advice, but there was miscommunication in handling that advice. (Then we should figure out how to improve that communication.)
Since I was curious, I skimmed a bit and found the upgrade MR-120 and some reverts (part 1, part 2). Unfortunately, I couldn't see any notes about which part broke 5.37. But this was enough info that I could try running the upgrade for myself:
## Get code from *just before* the original upgrade (~1) cd dataprocessor git checkout 0d7d4d6ae0a57ae9d7e07a39eafeda1f0a81876c~1 -b re-run-upgrade ## Re-run the upgrade from that starting point civix upgradeThis step jumped out at me:
If that change is the issue, then we can come up with a tighter patch that lets you retain 5.37 compat while applying the rest of the updates. (Or if it was something else, then it would be good to know - so that we can whatever notice is missing.)
Edited by totten
That warning is very recently introduced in Civix. But the main problem is that the upgrader class and most mixin do not work on older versions of civicrm. Such as civi version 5.37. Civix and mixin assumes there is code in civicrm core which is not available yet in civi version 5.37. Also if you press no at that warning the command does not get executed. So the yes is basically obligatory.
And it is not only data processor suffering from this. Any extension which is made with a newer version of civix does not work on Civi 5.37
Edited by jaapjansma-
But the main problem is that the upgrader class and...
Aah, right, thanks.
CRM_Extension_Upgrader_Baseis a significant consideration. It was added incivicrm-core@5.38. The missing class is almost fixable, but the hard part is the newinfo.xmltag from 5.38 (<upgrader>My_Class_Name</upgrader>). That made civix templates a lot simpler, and we don't have a quick alternative. Possibilities are:- Stay on the old version of
civix - Copy the boilerplate into each
myextension.phpin a way that coexists with newer versions ofcivix. (When you're ready to drop 5.37, then manually delete it.) - Re-add old boilerplate to newer versions of
civix(with lots more conditionals) - Backport the
<upgrader>tag to your copy of 5.37. (*Probably 20091 + 20090 + 20116... but maybe a few others.) - Write a polyfill specifically for
<upgrader>andCRM_Extension_Upgrader_Base.
(FWIW, the
civix upgradecommand does show a warning about the 5.38 requirement -- but if you need 5.37, then the warning doesn't make the job easy. This differs fromentity-type-php@1.0.0, which could be replaced with a few lines of custom code. This one is harder.)Anyway, I admit I'm not keen on
#3(re-adding the old boilerplate tocivix.gitwith more conditonals). But if this is a problem for several people/extensions, then I might dig further. Maybe we look into#2or#4or#5?... and most mixin do not work on older versions of civicrm... Any extension which is made with a newer version of civix does not work on Civi 5.37
This part... I don't believe. (At least, not as written.) Whenever we add a mixin to
civix.git, I make a push to evaluate backward-compatibility. It's a mandatory piece of data in civix:mixin-backports.php. Each item has aminimumflag, and that comes from actual testing. AFAICS, 9 of 11 entries are compatible with 5.37.But there are a couple things that may look similar:
- There was the
polyfill.phpbug that you found last year, which could present symptoms on any arbitrary mixin. (But obviously, in retrospect, it was a bug inpolyfill.php) - If you develop locally on a new CiviCRM and then try to deploy on old CiviCRM, then it's very important to keep an eye on the
<compatibility>flag. Previously,civixignored this flag. It tended to over-generate compatibility code ("polyfills"/"backports"/etc), and I got pushback from several devs who wanted to squeeze this stuff out. The compromise was forcivixto read the<compatibility>flag. For example:- If
info.xmldeclares<compatibility><ver>5.37<ver></compatibility>...- then
civixgenerates more compatibility code (with an aim to supporting5.37).
- then
- If
info.xmldeclares<compatibility><ver>5.60<ver></compatibility>...- then it generates less compatibility code (with an aim to supporting
5.60).
- then it generates less compatibility code (with an aim to supporting
- (There are limits to this. It can't generate compatibility code for every feature+version, but it can do a lot. This is why it's mandatory for civix:mixin-backports.php to specify the
minimumandprovided-byfor every item.) - (Also, for new extensions, the default value of
<compatibility>is based on your local CiviCRM runtime. If you want to deploy a new extension on an old CiviCRM, then you need to set<compatibility>explicitly and reruncivix upgrade.)
- If
--
One more question, out of curiousity... is there a way to explain the importance of 5.37 for you guys? Like, is it specifically 5.37 that you're concerned with? Or is it actually a mix of different (but similarly old) versions? Is it because of one big deployment, or one stack of similar deployments, or a group of 20 independent deployments? (I'm generally pretty sympathetic about backward-compat, but 5.37 is pretty old now, and... if I push for something to help 5.37, then... people will ask me why...)
- Stay on the old version of
-
Thanks @totten my workaround was to copy the boilerplate code back into the extension. I did not know about the compatibility mode of civix. Will try that out.
Regarding the neeed for civi 5.37 (I also have clients who are still on civi 5.14). It is not my need. It is the need of the organisation using CiviCRM. They end users need to test the business process whether that still works at every upgrade. Meaning that an upgrade project takes around 3 - 9 months because those end users need to do this next to the day-to-day work. So my experience is that end user organizations tend to upgrade once in the 3-6 years. And becuase CiviCRM is behind a firewall there is no need for upgrading from a security perspective.
For example last week we turned on wkhtmltopdf at a civicrm installation making pdf letter generations quicker. However the monthly process of sending out a magazine and printing a membership card got broken, because the template was not correct for wkhtmltopdf. So when doing an upgrade it this kind of issues we try to stay on top off. Otherwise the business processes will fail in delivering continuity.
Edited by jaapjansma I am afraid this is going to be an iteration but the customers I work with do all do a CiviCRM upgrade once every 3 years because of all the business processes that have to be tested. For example they all use CiviSepa and CiviBanking and have to ensure that all continues to work as this is a vital business process. Following CiviCRM (which is part of their complete stack, not their complete stack!) upgrades to the latest version would in most cases mean they will have to form a continuous testing team, which is simply unexplainable and unfundable. As they all use CiviCRM behind a firewall with a separate website they are not so much touched by security risks and do an full upgrade process (CiviCRM + all extensions + all external services and data integrations that they run) once every 2-3 years. I totally understand that this is not something that should stop you from using the latest tools and continuing on the way forward. But it helps if we try to keep the impact to a minimum level as this kind of stuff is costing the non-profits money that they can not use for their mission?
@ErikHommel I understand. But it also applies the other way. Eg. By incrementing the minimum version on an extension I maintain I reduce the cost to my clients (who are all on 5.65+) because I don't have to worry about maintaining support for older versions. I've added in support for older versions before on request when a client is able to offer funding to support that.
It is never my choice, it is always the customer that decides. And in my 13 years of CiviCRM experience they always select to postpone until they have testing time. And I would never recommend them to upgrade without serious business process testing. It is not a question of if they should or should not use newer versions, it is about ensuring as much as possible that we do also support customers with older versions and try to keep the impact to a minimum. Not interested in a yes but... discussion
Edited by ErikHommel-
@mattwire as @ErikHommel says. It is the choice of the customer, and one of the consequences is that keeping extension backwards compatible can become a burden. But I have never came across where the customer said, ok that is too much burden lets do an upgrade process. It is rather the other way around, ok spend time on it to keep the extension backwards compatible. And also at customers we also don't do extension upgrades, unless other functionality is needed.
-
OK, thanks guys. I appreciate that there are general pressures+trade-offs+strategies re:features/risks/tasks/versions/extensions/maintainability/QA/etc.
The important thing to me is specifics. In this thread, these are the specifics that stick out to me:
-
Communication about
<compatibility>: This flag was not historically important forcivix. (It was important for other things, like in-app distribution. But historically, there have been plenty of valid use-cases where it could be ignored.) Somewhere aroundcivixv22-v23, the<compatibility>became more important. -
Database Upgraders: The
generate:upgraderfeature now uses simpler code, based on<upgrader>andCRM_Extension_Upgrader_Base. This simplification was originally written circa Apr 2021 (released 5.38), so an extension that uses it requires 5.38. I heldcivix.gitback on the old technique for ~20 months (circa Nov 2022), but I couldn't rationalize holding it back forever (esp since civix is a dev-tool/code-generator -- it doesn't force any user to upgrade). -
Entity Types Mixin: This mixin requires v5.45. IIRC, this is because 5.45 fixed some quirks in how civicrm-core dispatches
hook_entityTypes. (But I don't remember precisely and haven't looked at it since Nov 2022.) -
CiviRules and 5.37: It sounds like the primary developers managing
civirules.gitwere placed in a bind -- providing services to some client(s) running 5.37, while using some code-generation options (#2+#3) which no longer support 5.37 (but which are needed in order to get other code-generation options).
It is rather the other way around, ok spend time on it to keep the extension backwards compatible.
Sure. And I've been in similar positions, so you get no recriminations from me.
FWIW, if I were in a position like
#4, then I would seriously consider backporting some updates for./CRM/Extension/. (My guess is that it's not as hard as you'd think - but it does require specialized knowledge.) The upshot is that a bounded project (backport 2+3) resolves an unbounded problem (random/growing/open-set of extension-versions which rely on 2+3).But obviously, a detailed cost-benefit analysis that speaks to you+me+all your clients would... require more time+information than any one person has. So I'll stop talking about it. But if you come to think that backports would resolve the bind, then let me know.
-
Communication about
-
@totten regarding your option 1:
I just tested an extension with
civix upgrade. Which resulted in a new upgrader class. Whilst info.xml contains:<compatibility><ver>5.37</ver></compatibility>.So I would expect the upgrade function of civix to keep the existing upgrader class in this extension. As it would now break the compatibility with CiviCRM version 5.37
Is my assumption right? Do you want me to report an issue at the civix code base?
-
- I think it boils down to how many deployments+extensions are of concern.
- Yes, it probably makes more sense to track this discussion (re: database upgraders) under "civix issues" rather than "civirules issues".
-
@totten thanks for the clarification. I will re-consider whether we can safely merge this merge requests and what it would mean to keep backwards compatibility.
-
Merged via 82892a9a
I talked with @jaapjansma and we will make a release 3.0 which is dropping support for older versions so that we can support new things in CiviCRM. The first one (merged) is the Rules overview is converted to Searchkit/Formbuilder.
