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

Migrate rest of OpenUpgrade branches to GitHub #137

Closed
pedrobaeza opened this issue Nov 25, 2014 · 50 comments
Closed

Migrate rest of OpenUpgrade branches to GitHub #137

pedrobaeza opened this issue Nov 25, 2014 · 50 comments

Comments

@pedrobaeza
Copy link
Member

We can use the same script that Odoo uses for merging the three projects and respect the history to migrate to Github entire project.

What do you think?

@pedrobaeza pedrobaeza mentioned this issue Nov 25, 2014
@bwrsandman
Copy link

👍

  • 6.0
  • 6.1
  • 7.0
  • master?

@hbrunn
Copy link
Member

hbrunn commented Nov 27, 2014

yes, that would be more than welcome. It's quite a bit of work though, so whoever volunteers, go ahead. Then we can also get rid of the convoluted code in migrate.py to support bzr and git

@hbrunn
Copy link
Member

hbrunn commented Nov 27, 2014

...but please also to 6.0 then too

@pedrobaeza
Copy link
Member Author

Yeah, I was doing 6.0 tonight 😉

So, all is done now! Please check it and tell me if you any problem.

@hbrunn
Copy link
Member

hbrunn commented Nov 27, 2014

thanks a lot @pedrobaeza ! I use those branches now in #139 and did a quick test, seems to work fine

@StefanRijnhart
Copy link
Member

Hi @pedrobaeza,

did you simply do a bzr import of the lp:openupgrade-* projects? That means we lose the connection with the Odoo git projects. I believe the correct way is to port the diff between launchpad Odoo and launchpad OpenUpgrade onto a branch of GitHub Odoo like I did for 5.0

@pedrobaeza
Copy link
Member Author

More or less: I take Odoo script (https://github.com/odoo/odoo/wiki/GitHub-Transition#migration-from-bazaar) and adapt for OpenUpgrade. I haven't done that because it's a lot of more work that I think it doesn't worth the while:

  • OpenUpgrade "Odoo/OpenERP" is frozen when the mature release was out for each version.
  • In Github, we can still merge branches, make cherry-pickings, etc.

But if you think there is any advantage on doing that way, please point me to a little walkthrough for doing it and I'll do it.

@StefanRijnhart
Copy link
Member

What I did was branch both lp:openupgrade-server/5.0 and its corresponding bzr revision of lp:openobject-server/5.0. That allowed me to take the diff between these and apply to a GH branch of Odoo 5.0. For later versions, you would have to do the same for the addons. You could make separate commits, such as:

  • changes to core files
  • openupgrade_records module
  • migration scripts

for easier review.

@pedrobaeza
Copy link
Member Author

Sorry to insist, but I still don't see the benefits of doing your way (and implies a lot of more work). You can see perfectly each commit with a migration script in my approach. For example, here: 179a476.

@StefanRijnhart
Copy link
Member

I'm thinking that it would be more elegant to have for instance OpenUpgrade 6.1 be based on Odoo 6.1 proper, instead of on an unrelated import. That way, we can always merge Odoo 6.1 into OpenUpgrade 6.1 like we used to be able to do in bzr. In the current fashion, that is not possible.

@pedrobaeza
Copy link
Member Author

I'm trying to do it to make you happy, hehe. It's quite a work, because some later Odoo commits changes the way of doing some things and OpenUpgrade server modifies too, provoking a lot of conflicts, but I have changed your ocb2git script to have an "interactive" mode to allow to resolve each conflict on the fly, and I'm now finishing the 7.0 branch.

@StefanRijnhart
Copy link
Member

You don't have to do everything by yourself, Pedro ;-). This was on our roadmap, just not with a definite timeframe.

@StefanRijnhart
Copy link
Member

Also, you don't have to preserve OpenUpgrade specific commits as far as I am concerned, which you seem to do now...

@pedrobaeza
Copy link
Member Author

Don't worry, I'm trying for 7.0 branch. If I find some difficulties, then I let you know. About specific commits, it's the same for me squash them all together (even this is an extra step) or keep them split, so I have chosen the second one.

Regards.

@pedrobaeza
Copy link
Member Author

Done! 7.0 branch is ready catching last Odoo 7.0 commit, and making cherry-picking of difference between lp:openupgrade-server/7.0 and lp:openobject-server/7.0, lp:openupgrade-addons/7.0 and lp:openobject-addons/7.0.

This is a draft wiki page with the logs: https://github.com/OpenUpgrade/OpenUpgrade/wiki/Migration-from-Launchpad-to-Github-of-7.0-branch

Please check it and let me know if there is any problem.

@pedrobaeza
Copy link
Member Author

It's also done for the rest of the branches. Please check them, and close this issue when you think they are correct.

@pedrobaeza
Copy link
Member Author

I have seen some non renamed files, so I have to redo 7.0 branch, because the script doesn't do it automatically. Let me check how to do it.

@pedrobaeza
Copy link
Member Author

OK, I have it! It was an empty __init__.py that is not synced. I have recreated by hand, and now branch is working.

@hbrunn
Copy link
Member

hbrunn commented Dec 3, 2014

Thanks a lot for your work @pedrobaeza, it's really valuable to have this on git. I just started a 7.0 migration and it seems to work fine. What confuses me is that when I do a git merge odoo/7.0 I get a lot of changes, am I missing something with expecting the difference only being on OpenUpgrade's side?

@pedrobaeza
Copy link
Member Author

Well, I don't know what's happening. Last common antecesor of the two branches is cdce2e2, from 28 of november, that you can see the corresponding one here in Odoo: odoo/odoo@cdce2e2

@pedrobaeza
Copy link
Member Author

Maybe a reordering in the commit tree? Anyways, all Odoo stuff except last commit from december are there.

@StefanRijnhart
Copy link
Member

Thank you for your work, Pedro. Unfortunately, if Odoo does not merge cleanly then the result does not fulfil my requirement. Shall I prepare a branch with a fresh port of OpenUpgrade onto GH Odoo?

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

It does merge cleanly, it's just more changes than I expected

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

I just did a rebase on odoo/7.0: https://github.com/hbrunn/OpenUpgrade/tree/7.0-test-rebase - this works

@pedrobaeza
Copy link
Member Author

My tests have been a rebase and a merge from odoo, and both works correctly. Isn't enough?

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

I think merge wise, it's fine. But currently, I'm getting a weird error at the end of the base migration:

  File "/var/tmp/openupgrade/7.0/server/openerp/modules/loading.py", line 379, in load_modules
    processed_upgrade = load_marked_modules(cr, graph, states_to_load, force, status, report, loaded_modules, registry)
TypeError: load_marked_modules() takes exactly 9 arguments (8 given)

not done investigating what's happening here, but I'll come back to that soon

@pedrobaeza
Copy link
Member Author

That can be because a change on the way Odoo server is working. I had to resolve manually some conflicts about that method (inclusion of registry argument). Let me check and I'll fix it.

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

@pedrobaeza I hope you didn't yet spend time on this? To understand what's going on here, I did my own replay in https://github.com/OpenUpgrade/OpenUpgrade/tree/7.0-hbrunn - this works now with a real live migration. Maybe we should simply use this branch as 7.0? I also fixed a small problem with the Bolivian currency there.

My steps were as follows:

  • Write openupgrade2git on https://code.launchpad.net/~therp-nl/lp-community-utils/replay_openupgrade2git
  • Set up my odoo branch to ignore *.rej files
  • run openupgrade2git for the server
  • there, reject all merge commits from the upstream project (we already are on a newser version)
  • fix the real conflicts (not many)
  • add the init file for openupgrade that strangely enough is not created during the above process
  • do the above for openerp-addons

@pedrobaeza
Copy link
Member Author

I have just solved it in 9079d147eb8bdb88fc4418c59397bea548b051ce. The problem was that in the conflict resolution, an extra parameter for load_marked_modules was missing. Now all is correct.

@pedrobaeza
Copy link
Member Author

In your branch, I see some of the problems I have also taken care of in my migration:

  • Migration script doesn't put final backslash on addons, resulting in some of the paths misleading (https://github.com/OpenUpgrade/OpenUpgrade/tree/7.0-hbrunn/addonsauth_anonymous).
  • I fix __init__.py on openupgrade (but you have also find out this). The reason is that the file is empty, and bazaar doesn't inform about this.
  • The original script also doesn't take care of moved files, so you probably found more conflicts merging than me (but that was more critic in other branches, specially in 6.0, with the /bin/ move).
  • Binary files are not added, because bazaar doesn't show diff for them.

That's why I think we should stand with current branch. What do you think?

P.S.: For reference, this is the enhanced script I have made from Stefan's original one: https://github.com/pedrobaeza/maintainers-tools/blob/openupgrade2git/tools/openupgrade2git.py

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

I just tried to find out why our branches are so different, and it seems to me that you used a somewhat old version of openupgrade-addons. Your branch misses all of September's and October's addon migrations (ie. your own from https://code.launchpad.net/~pedro.baeza/openupgrade-addons/7.0-stock_location/+merge/231396). And then the rest of the changes also seems to stem from old odoo sources. I used the versions from today. Considering that it's some handwork to fix that, my branch might be more convenient.

Shouldn't 7.0 and 7.0-hbrunn be basically the same after you merged in odoo/7.0? (save for the missing commits from openupgrade-addons)

@pedrobaeza
Copy link
Member Author

No, I have used the last ones. The question is that you have made it in a reverse order: I merge first addons, and then server, and you have merge server, and then addons. If you fix the problems you have in your branch, I have no problem in switching to your branch.

@pedrobaeza
Copy link
Member Author

Looking deeper, I'm missing addons revisions 8169 to 8174. I'm so puzzled... I can add it that revisions now to my branch if you want. I have checked that all server revisions are there.

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

Okay, thanks for the heads up about the addons* files, that's fixed now. Moved files are indeed not really a problem in 7.0, and the one binary file I encountered I added by hand.

Now how to go on? Given that both our branches contained fuckups, I'm not sure if I trust either of them. We certainly should rerun a lot of migrations with whichever branch we choose.

But concerning actuality of the code: In safe_eval.py, you miss the fixes from this year's security advisory (or turned them back during merge maybe?) That's also what I was aiming at in #137 (comment) - how come that simply merging in odoo/7.0 creates so many changes? Shouldn't this just be the diff from the last three days?

@pedrobaeza
Copy link
Member Author

In my migration, I have introduced an "interactive" mode to allow to merge some revisions and resolve merge conflicts on the fly. I decided not to merge revisions that are tagged as "Merge lp:openobject-xxxx...", because I assume that all that revisions are already on odoo branch. Maybe that's why safe_eval that you're commenting is not there, because there has been something weird also in odoo's migration?

Here is the output log of my migration: https://github.com/OpenUpgrade/OpenUpgrade/wiki/Migration-from-Launchpad-to-Github-of-7.0-branch

If you're more confident with your branch, please point me to the conflicts resolution revisions, and I'll check the rest of the things I have made on my own.

@hbrunn
Copy link
Member

hbrunn commented Dec 4, 2014

hmmm, I resolved conflicts while patching - in my version of openupgrade2git he drops to a shell when the patch didn't work, then you can do stuff, exit the shell and the changes will be applied as the original revision that introduced the problem. So no separate conflict resolving commits...

We need a third person's opinion, I think. And better a fourth's and fifth's one, too.

@pedrobaeza
Copy link
Member Author

I have been checking all the conflicts I had in my migration, comparing with your branch, and all is good. I have also run a migration and all seems correct, so I'm confident enough with your branch, so for me it's OK to switch to yours. May I make the switch?

@hbrunn
Copy link
Member

hbrunn commented Dec 5, 2014

I think that not only two people should decide that. Anybody listening here? Sometimes, we wait for three thumbs up for spelling mistakes...

@pedrobaeza
Copy link
Member Author

Yeah, but due to that the branch is published, I think it's better to switch them now, and later see if there is any problem. Don't you think?

@hbrunn
Copy link
Member

hbrunn commented Dec 6, 2014

yes, good point. Let's do so. Still, others: Please look into that too

@pedrobaeza
Copy link
Member Author

I have already made the switch.

@StefanRijnhart
Copy link
Member

I would just remove the branches and not replace them with any other
unvalidated branch.

@pedrobaeza
Copy link
Member Author

As I have said, Stefan, I'm confident enough with that branch now that I have made some migration real test. Anyways, we can use usual PR patch method for anything we find. Let's trust a litlle, now that it's Christmas, hehe!

@StefanRijnhart
Copy link
Member

I have been validating the current 7.0 branch. Great work @hbrunn! Just got two differences.

OpenUpgrade tolerance for invalid views seem new here: https://github.com/OpenUpgrade/OpenUpgrade/blob/7.0/openerp/addons/base/ir/ir_ui_view.py#L155 and https://github.com/OpenUpgrade/OpenUpgrade/blob/7.0/openerp/addons/base/ir/ir_ui_view.py#L164 (See http://bazaar.launchpad.net/~openupgrade-committers/openupgrade-server/7.0/view/head:/openerp/addons/base/ir/ir_ui_view.py#L154). Looks like a good idea though.

Obviously, I miss your own commit 46901f9. What is the history of this change?

@hbrunn
Copy link
Member

hbrunn commented Dec 8, 2014

Yes, I took those from 8.0.

As for the last commit: The Venezuelan bolivar's xmlid was renamed from 'VEB' to 'VEF' during the life cycle of 6.1. But given that currencies are noupdate records, nothing happens with that in an existing database. So I rename the xmlid to the currency that has the same name ('VEB', with xmlid 'VUB') in 7.0. From my understanding, that is the old version of the bolivar, so it might also be an option to simply drop the record. But it seems safer to keep it and move it to the legacy record. If this xmlid doesn't exist, nothing happens, so it won't hurt.

Here the corresponding diff:

=== modified file 'openerp/addons/base/base_data.xml'
--- openerp/addons/base/base_data.xml   2011-09-19 14:48:21 +0000
+++ openerp/addons/base/base_data.xml   2011-09-20 12:58:04 +0000
@@ -1086,16 +1086,17 @@
             <field eval="time.strftime('%Y-01-01')" name="name"/>
         </record>

-        <record id="VEB" model="res.currency">
-            <field name="name">VEB</field>
-            <field name="symbol">Bs</field>
-            <field name="rounding">2.95</field>
+        <!-- VEF was previously VEB -->
+        <record id="VEF" model="res.currency">
+            <field name="name">VEF</field>
+            <field name="symbol">Bs.F</field>
+            <field name="rounding">0.0001</field>
             <field name="accuracy">4</field>
             <field name="company_id" ref="main_company"/>
         </record>
-        <record id="rateVEB" model="res.currency.rate">
-            <field name="rate">2768.45</field>
-            <field name="currency_id" ref="VEB"/>
+        <record id="rateVEF" model="res.currency.rate">
+            <field name="rate">5.864</field>
+            <field name="currency_id" ref="VEF"/>
             <field eval="time.strftime('%Y-01-01')" name="name"/>
         </record>

@pedrobaeza
Copy link
Member Author

I think we can close this issue, and if any issue arises, create another.

@hbrunn
Copy link
Member

hbrunn commented Dec 31, 2014

I tested a 6.1 migration which went fine. let's close this then

@hbrunn hbrunn closed this as completed Dec 31, 2014
@StefanRijnhart
Copy link
Member

Forgot I had started an analysis verifying 6.1. Some files were missing, I'm readding them in #190. Reopening this one because 6.0 has not been verified yet.

@StefanRijnhart StefanRijnhart reopened this Jan 1, 2015
@pedrobaeza
Copy link
Member Author

For the records, I have completed a 5.0 > 6.0 migration without problems.

@pedrobaeza
Copy link
Member Author

This can be considered done, so I close.

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

4 participants