Skip to Content
  2. Mailing Lists
  3. Contributors
  4. Re: Procedure to create 16.0 branches

Archives

Contributors

Re: Procedure to create 16.0 branches

Re: Procedure to create 16.0 branches

Re: Procedure to create 16.0 branches

Hi Pedro,
  • This will increase the size of the repository the same, as no common ancestor and then different SHA.
You're wrong on this as this is the case currently (different SHA). Example with stock_cycle_count module:


image.png


image.png

When basing the new main branch on preceding one:


image.png


image.png

  • Derived from this, it requires that the reviewers alw ays need to check if the migration pull request contains the latest commits.
That's already the case depending on how long the migration takes and if reviewers detect the missing ones or not.

  • It will generate extra burden for deprecated modules, requiring to make a pull request to remove them,
I see advantages here as currently if a module is not there, that does not mean if it is migrated yet or has been deprecated. With proposed approach, you can detect it more easily (present in previous branch, not in current. Moreover in the commit history you can see why it has been deprecated and learn about changes you maybe missed).

  • In the past, having uninstallable modules has cost a lot of problems. Remember the static folder problem that loads the Python files, the switch from version 2 to 3. Now they are not present, but nothing prevents a new side effect from happening in the future. Better to have the house cleaned.
Predicting behaviour on possible things is not an argument IMHO. With pre-commit, I think we can manage most of things that can happen (e.g.: generating a clean setup folder, ...).


IMHO the situation in the early years of OCA is not the same as the one we know now. Context and tools have changed.

From several same approach repositories knowledge, I would say this will lead to a cleaner situation and an easier newcomers contribution processes understanding.


On Wed, Jul 20, 2022 at 4:52 PM Pedro M. Baeza (Tecnativa) <notifications@odoo-community.org> wrote:
Returning back to this approach is a mistake IMO, as it has more cons than pros. First of all, one of the advantages that you mention is not correct, or at least, depending from the perspective, that is the "slow git repo growth": if you download a single new branch, it will cost a lot more, as it has all the commit history from all the modules. This gets even worse when the modules are deprecated (more on this later).

And about the drawbacks, there are still bad interactions with modules with previous versions code due to Python or ORM peculiarities (check for example this PR for fixing a code that shouldn't be executed not being the module installed), and for sure more will appear. Going to the process of discovering these problems and fixing this is an unneeded maintenance burden. The discoverability will have the same problem, as people don't get used to having useless folders an d to need to go to that file to check it or open the manifest. Even more, I think the OCA apps store module will have problems with this approach (but that's only suspicions).

Now let me summarize again the rest of the cons that were the reason to change to this approach:
  • When the branches are created for the new 16.0 version, activity in the prior branch 15.0 may happen until the migration of the module is done, but the 16.0 branch won't reflect these new commits. There's a high risk of the contributors of the migration not including latest changes, or to work on the deprecated source code and come to OCA with their pull request to be discouraged by one qualified reviewer saying that they need to rework the migration including the missing commits. Forcing and teaching people to use tools like oca-port is a high entry barrier. Someone has proposed to develop a bot for automatically doing these forward-ports, but it will have lots of problems as well:
    • First of all, it needs to be developed, which is not an easy task and needs to be done. Adding extra tools for something that is not needed with another approach is not the convenient way to go.
    • If there's already a migration PR proposed, the new commits won't be there, and it will require a rebase by the migrator (again more contribution friction).
    • Those extra commits will lose the security signing that may be present in previous branches. The only way to not lose them is that the same author makes the commit signing it in the new branch.
    • There can potentially be the same CLA problems.
    • When the bot does the forward-ports, CIs may be red due to external reasons, but then these commits won't never reach the new branch unless manual actions are done.
    • This will increase the size of the repository the same, as no common ancestor and then different SHA.
  • Derived from this, it requires that the reviewers alw ays need to check if the migration pull request contains the latest commits. With the previous approach, there are less chances that they are not included, and we light up the reviewing process.
  • Also derived from the first point, it's the lack of homogeneity in the processes. With the current one, everything follows the same procedure. This is VERY IMPORTANT: to have only one way of doing everything, not having to know different processes depending on the state of the branch.
  • It will generate extra burden for deprecated modules, requiring to make a pull request to remove them, and creating a very big diff commit of such removal, increasing the branch and repository size (remember that there will be at least one commit adding the module, and another removing it, so double diff).
  • Modules being migrated with a jump of version will have the same problem of not having any of the commit history.
  • Rebel modules configuration and similar CI tricks may cause problems on new branches.
  • In the past, having uninstallable modules has cost a lot of problems. Remember the static folder problem that loads the Python files, the switch from version 2 to 3. Now they are not present, but nothing prevents a new side effect from happening in the future. Better to have the house cleaned.
The fact that some of you are using this approach in some repositories is not representative, as that repositories have a very limited scope (few modules), and very controlled by few contributors, but try it in OCA/sale-workflow or OCA/purchase-workflow...

Please reconsider your approach. If not, we at Tecnativa don't know if we can afford this process in our contributions to OCA.

Regards.

_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe



--

Denis Roussel
Software Engineer
T    : +32 2 888 31 49
M : +32 472 22 00 57


Val Benoit, Quai Banning 6 | B-4000 Liège | Belgium
Atrium Building, Drève Richelle 167 | B-1410 Waterloo | Belgium
Zone industrielle 22 | L-8287 Kehlen | Luxembourg
by Denis Roussel - 07:46 - 20 Jul 2022

4 attachments 4 attachments

image.png
image.png
image.png
image.png

Reference

  • Procedure to create 16.0 branches
    Dear contributors,

    I'm starting to think about the process to create the 16.0 branches. And the more I think about it, the more I'm convinced we should do it by adding "installable": False in the module manifests, instead of creating empty branches.

    This would have several benefits:
    • Improve security. Indeed, currently migration PRs have a lot of commits and reviewers only look at the last 2 commits. By accident or malice, it would be easy for a contributor to sneak bad code in older commits, that would go unnoticed. As the community grows, I think this a very important topic.
    • Avoid CLA bot issues: currently, the CLA bot is flagging old commits that were ok at the time they were created, but may not be valid today as contributors may have changed email, or revoked their CLA.
    • Reduce oca-github-bot complexity: work has to be done to make the bot aware of other branches in migration PRs (notably to look-up maintainers). This would be unnecessary if a migration PR is a normal PR to an existing addon directory. On the contrary, the bot could even detect migration PRs automatically by noticing the change to the installable flag, so this could simplify some processes.
    • Slow git repo growth: by avoiding the recreation of identical commits in several branches we would slow down the git repo size increase.
    About the possible drawbacks, I am under the impression that all the reasons we had back then to create empty branches have faded away:
    • Today, Odoo and all the OCA tooling work perfectly well when there are addons marked as uninstallable. They are correctly ignored by linters, tests, and Odoo does not attempt to import the code.
    • Regarding discoverability, the addons table in the README shows a clear view of what is not migrated.
    The migration procedure and tools should continue to work as today, to pick up commits that would have been added after branching (basically the git-am process would simply work as it does today)

    All we'd need maybe is to agree on a process to remove modules that have not been migrated for several versions. But in a first approach, regular PRs to remove now useless modules would probably be sufficient.

    Are there any other arguments (pro or con) that I would have missed ?

    Looking forward to reading your feedback on this proposal.

    -Stéphane

    by Stéphane Bidoul - 12:45 - 20 Jul 2022

    36 replies 36 replies

    • Re: Procedure to create 16.0 branches
      Hi,

      I'm no techie, and don't pretend to deeply understand this stuff. Also not a PSC of any repo. I am not very personally affected by whatever decision is made here and will happily defer to any decision.

      Regards benefits of this approach, the first question I ask is can this be achieved in other ways. Usually people tell me it is impossible immediately. I won't argue back, just random thoughts.
      • Improve security. One thought regards nefarious commits injected into migration. Is this solvable in other ways? Something like an AST/semantic diff maybe or even just a regular diff against last release. In fact a regular diff against last release may identify missed commits too.  But anyway some kind of semantic diff outside of migration may be useful too as it would just ignore format changes. However, I think security is a separate important topic that needs a wider discussion regardless of whatever outcomes here are and we should be vigilant to those risks. But also as a reviewer, I think I'd rather review a semantic diff anyway. 
      • Avoid CLA bot issues: Do we really need to preserve ALL history every release - what about a squash merge - that maintains all the prior authors? Then maybe we can get an excpetion for that case in bot. For that 1 time ever you need a blame on a file 6 years ago, you can still traceback through versions.
      • Reduce oca-github-bot complexity: I actually thought that work was done already. But in any case, is the argument here valid? Because not all modules in prior repo(s) would be merged at time the new one is created. So don't you avoid the complexity, until you don't? Wouldn't we need the functionality regardless? I don't know, genuine question.
      • Slow git repo growth: As above for CLA Bot.

      My general feeling I am getting - 

      • Maintainers with tightly controlled, homogenous repos with a small number of maintainers and modules are in favour. These also tend to be "additive" repos, i.e. it is new functionality with not a lot of integration points. They also tend to be a bit more "technical". There is a very high likelihood of migration and also fairly rapidly. However, the contributors on those repos are more than capable of a 5 minute dance to do the port work.
      • PSC's with large heterogeneous repos with many modules and contributors aren't so much in favour. Of course in these repos there tends to be a LOT of flux from version to version. Place like product-attribute, sale-workflow. These are often "insertive" type modules, putting themselves in middle of business processes. Design changes, upstream functionality changes, lots of modules dropping, reappearing.

      I know for myself and some modules I have some involvement with.
      office 365 auth - hasn't changed between versions for 7 years. I don't even have any clients paying for it anymore - but 1 guy emails me once a year asking me to bump version number because it still works.
      partner statements - we did a big refactor for 11 or 12.0 maybe, a few minor changes for 13.0 accounting but less than you'd think because it is fundamentally just adding a couple reports.
      Now when I'm doing/reviewing those, I'll admit it does seem a bit wasteful this whole dance but it is no big deal.

      operating-unit - literally just a bunch of modules sticking a single field on a model, these are often awful to migrate because that happens in the middle of all sorts of functions, lots of interdependencies, stupid broken Odoo record rules. Plus they look easy, so often people seem to pick them for their first ever migration, review sucks on these and we get quite a few unexpected oversights. You really do need to download and test these PR's.
      I know it should make no difference, but my head just thinks better starting clean.

      sale-workflow/product-attribute type repos - honestly, a lot of the time in these repos it is about merging modules, deprecating modules, combining functionality, finding better repos as focus changes. I'm probably wrong, but I feel like prepopulating uninstallable is probably more work and missed opportunities.

      Anyway just my thoughts.

      But also, even if it is OK for 16, what about odd number version :) (This is a joke).

      On Thu, Jul 21, 2022 at 10:42 PM Pedro M. Baeza (Tecnativa) <notifications@odoo-community.org> wrote:
      > To make sure people reason about the correct arguments, let me emphasize again that the migration process would be strictly identical.

      Although theoretically you can use the same commands (but I repeat, I remember some problems in the past with this - I will try to dig on the history to find any -), what will happen for sure is that people forget to do/don't know such initial steps and appear that the migration pull request is correct, while it doesn't contain the missing commits, charging on the reviews to check this, so that's the real issue, together with other strong arguments like the IDE search, cleaning, etc.

      And I also agree having some repositories with one approach and another with the other approach is a mess. You have done the installable=False approach in the past for your specific repositories (mis-builder, storage, product-pim and queue mostly), because you do the migration yourself, not involving other contributors, but the rest of the repositories can't apply this method.

      Regards.

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      by Graeme Gellatly - 03:15 - 21 Jul 2022
    • Re: Procedure to create 16.0 branches
      > To make sure people reason about the correct arguments, let me emphasize again that the migration process would be strictly identical.

      Although theoretically you can use the same commands (but I repeat, I remember some problems in the past with this - I will try to dig on the history to find any -), what will happen for sure is that people forget to do/don't know such initial steps and appear that the migration pull request is correct, while it doesn't contain the missing commits, charging on the reviews to check this, so that's the real issue, together with other strong arguments like the IDE search, cleaning, etc.

      And I also agree having some repositories with one approach and another with the other approach is a mess. You have done the installable=False approach in the past for your specific repositories (mis-builder, storage, product-pim and queue mostly), because you do the migration yourself, not involving other contributors, but the rest of the repositories can't apply this method.

      Regards.
      by Pedro M. Baeza - 12:41 - 21 Jul 2022
    • Re: Procedure to create 16.0 branches
      To make sure people reason about the correct arguments, let me emphasize again that the migration process would be strictly identical.

      On Thu, Jul 21, 2022 at 11:26 AM LuisDaniel Lafaurie <notifications@odoo-community.org> wrote:
      Hello everyone,

      I agree with what Rémi has just explained.

      It wouldn't be wise to put more pressure on the migration process for the ones (PSCs and/or reviewers) working more actively on that subject.

      Keeping the current method will be the best option so far, IMHO.

      Thus, my vote is -1

      Thanks,
       

      On Thu, 21 Jul 2022 at 10:02, Rémi CAZENAVE - Le Filament <notifications@odoo-community.org> wrote:

      Hi all,

      I like better the current method (having only working/tested/maintained modules in each branch) for the following reasons :
      - I find it easier to see what is available un a given version, what would still need to be migrated, and also to get an idea of which versions are being actively used (though I guess I could look at READMEs, migration issues and overall statistics to get the same info)
      - on each Odoo instance we deploy we download specific branch and having lots of unusable code would increase bandwidth and storage (although mostly text that could be compressed, so probably not dramatic neither)

      I agree that the process to migrate a module from one version to another is not really easy (at least the first times), but I understand that it would still probably need to be done in order to make sure you get the latest commits from previous branch. I am afraid that this would often be forgotten if not strictly necessary in migration process, and for sure we do not want to transfer this extra burden of checking for each PR whether based on latest commits or not to reviewers / PSC.

      Last, I am not in favour of having 2 different processes that can be applied per repo, since it would increase complexity for understanding why 2 repos are not behaving the same, and we may lose newcomers (but also for people not so much involved in OCA) - although here again I understand from discussions this is already the case for some repos.

      My 2 cents...

      Best Regards,
      Rémi CAZENAVE
      SCOP Le Filament

      Le July 21, 2022 3:52:02 AM UTC, "Raphaël Valyi" <notifications@odoo-community.org> a écrit :
      Hello,

      My vote goes to the opt-in option at least unless Pedro get convinced, because going against the will of somebody processing such a large portion of the PRs would be a terrible shot in the feet.

      That being said, earlier Pedro raised the concern about the possibility to craft an attack commit inside a missing commit that would be cherry-picked just like with the current way of migrating. But:

      On Wed, Jul 20, 2022, 3:42 PM Roussel, Denis <notifications@odoo-community.org> wrote:
      To summarize:

      • commits SHA are different with current behaviour
      • commits SHA are equal with proposed one
      Denis could you confirm this SHA conservation could make us safe against such crafted attack commits in the middle of the missing commits that one would need to cherry-pick with the new procedure?

      This crafted commit attack thing is indeed extremely concerning...

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      by Stéphane Bidoul - 12:05 - 21 Jul 2022
    • Re: Procedure to create 16.0 branches
      Hi,

      For the moment, I prefer the empty branches. The lifecycle of the module is really different from the lifecycle of major version releases.
      I think the focus should be on improving tools /workflows for keeping in sync modules across versions.


      Regards,

      Le jeu. 21 juil. 2022 à 10:02, Rémi CAZENAVE - Le Filament <notifications@odoo-community.org> a écrit :

      Hi all,

      I like better the current method (having only working/tested/maintained modules in each branch) for the following reasons :
      - I find it easier to see what is available un a given version, what would still need to be migrated, and also to get an idea of which versions are being actively used (though I guess I could look at READMEs, migration issues and overall statistics to get the same info)
      - on each Odoo instance we deploy we download specific branch and having lots of unusable code would increase bandwidth and storage (although mostly text that could be compressed, so probably not dramatic neither)

      I agree that the process to migrate a module from one version to another is not really easy (at least the first times), but I understand that it would still probably need to be done in order to make sure you get the latest commits from previous branch. I am afraid that this would often be forgotten if not strictly necessary in migration process, and for sure we do not want to transfer this extra burden of checking for each PR whether based on latest commits or not to reviewers / PSC.

      Last, I am not in favour of having 2 different processes that can be applied per repo, since it would increase complexity for understanding why 2 repos are not behaving the same, and we may lose newcomers (but also for people not so much involved in OCA) - although here again I understand from discussions this is already the case for some repos.

      My 2 cents...

      Best Regards,
      Rémi CAZENAVE
      SCOP Le Filament

      Le July 21, 2022 3:52:02 AM UTC, "Raphaël Valyi" <notifications@odoo-community.org> a écrit :
      Hello,

      My vote goes to the opt-in option at least unless Pedro get convinced, because going against the will of somebody processing such a large portion of the PRs would be a terrible shot in the feet.

      That being said, earlier Pedro raised the concern about the possibility to craft an attack commit inside a missing commit that would be cherry-picked just like with the current way of migrating. But:

      On Wed, Jul 20, 2022, 3:42 PM Roussel, Denis <notifications@odoo-community.org> wrote:
      To summarize:

      • commits SHA are different with current behaviour
      • commits SHA are equal with proposed one
      Denis could you confirm this SHA conservation could make us safe against such crafted attack commits in the middle of the missing commits that one would need to cherry-pick with the new procedure?

      This crafted commit attack thing is indeed extremely concerning...

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe



      --
      Raphaël Reverdy
      Mobile +33 6 38 02 03 93
      Fixe +33 4 82 53 84 60
      by Raphaël Reverdy - 11:46 - 21 Jul 2022
    • Re: Procedure to create 16.0 branches

      Hi All,

      I am not in favor of this and most of this has already been said by others, but I want to re-iterate:

      1. It is currently easy to see at a glance the state of the branch and which modules have been migrated

      2. The current migration process for most modules is relatively easy

      3. If it hinders or causes friction for current OCA contributors and causes them to engage less in OCA it shouldn't be done. Period.

      4. The most important reason:

      Except for the few modules that are widely used most modules are migrated after 1+ years of the Odoo SA version release. Quite a few modules are also migrated after more than 1 Odoo SA version release. I.E. version 12 module skips 13 and is migrated to 14 and then someone else migrates it to 13. This will create chaos.

      I am also not in favor of an opt-in option because it's just an additional difference that we (developers and users) have to keep in mind (re: #1) and frankly the purported benefits do not outweigh the headaches it will cause.


      Regards,

      Mike.

      On 21/07/2022 12:12, Aarón Henríquez Quintana wrote:
      As a developer I love the way it is now.

      • You know clearly what is migrated, and all the history
      • Easy to track what is being migrated
      • The process is clean, and you get used to it very fast, barely 4 commands in git.
      • I think it is safe also, it is a responsibility of the repo maintainer that approves the PR to look at the code and check everything is correct.

      If we apply the proposal:

      • It is way more complex for developers, it open a bunch of possibilities:
        • If it was not in v15 at the time the 16.0 branch was created: in this case we will be doing the current procedure
        • If it was, and there is no improvement, then we will do a "normal" migration with 1 single commit, that's ok
        • if it was, but it was improved, we have to include those improvements as well, the diff is not reduced as well.
      • Deleted modules will create greater diffs.
      • If the module was deleted because no one wanted to use it, but later on someone decides to migrate it, the diff is going to be super huge.
      • It is going to be harder to know what is migrated, and being migrated, and what is not necessary.
      I understand the improvements of the change but I think those improvements are now important enough with respect to the drawbacks. 

      On Thu, 21 Jul 2022 at 10:02, Rémi CAZENAVE - Le Filament <notifications@odoo-community.org> wrote:

      Hi all,

      I like better the current method (having only working/tested/maintained modules in each branch) for the following reasons :
      - I find it easier to see what is available un a given version, what would still need to be migrated, and also to get an idea of which versions are being actively used (though I guess I could look at READMEs, migration issues and overall statistics to get the same info)
      - on each Odoo instance we deploy we download specific branch and having lots of unusable code would increase bandwidth and storage (although mostly text that could be compressed, so probably not dramatic neither)

      I agree that the process to migrate a module from one version to another is not really easy (at least the first times), but I understand that it would still probably need to be done in order to make sure you get the latest commits from previous branch. I am afraid that this would often be forgotten if not strictly necessary in migration process, and for sure we do not want to transfer this extra burden of checking for each PR whether based on latest commits or not to reviewers / PSC.

      Last, I am not in favour of having 2 different processes that can be applied per repo, since it would increase complexity for understanding why 2 repos are not behaving the same, and we may lose newcomers (but also for people not so much involved in OCA) - although here again I understand from discussions this is already the case for some repos.

      My 2 cents...

      Best Regards,
      Rémi CAZENAVE
      SCOP Le Filament

      Le July 21, 2022 3:52:02 AM UTC, "Raphaël Valyi" <notifications@odoo-community.org> a écrit :
      Hello,

      My vote goes to the opt-in option at least unless Pedro get convinced, because going against the will of somebody processing such a large portion of the PRs would be a terrible shot in the feet.

      That being said, earlier Pedro raised the concern about the possibility to craft an attack commit inside a missing commit that would be cherry-picked just like with the current way of migrating. But:

      On Wed, Jul 20, 2022, 3:42 PM Roussel, Denis <notifications@odoo-community.org> wrote:
      To summarize:

      • commits SHA are different with current behaviour
      • commits SHA are equal with proposed one

      Denis could you confirm this SHA conservation could make us safe against such crafted attack commits in the middle of the missing commits that one would need to cherry-pick with the new procedure?

      This crafted commit attack thing is indeed extremely concerning...

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      _______________________________________________
      Mailing-List: https://odoo-community.org/groups/contributors-15
      Post to: mailto:contributors@odoo-community.org
      Unsubscribe: https://odoo-community.org/groups?unsubscribe

      by Raphael Makonnen - 11:41 - 21 Jul 2022