Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

Update travis scripts to install with pip #343

Closed
wants to merge 4 commits into from

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Jul 15, 2016

This is a proof of concept to illustrate how installing with pip can help with dependency management.

There is no need for oca_dependencies.txt anymore as all dependencies are expressed in the __openerp__.py, possibly refined by setup.py, itself refined by requirements.txt if present.

This is IMHO a important enabler to progressively move towards smaller repos.

An example build with this PR is here: https://travis-ci.org/acsone/account-financial-reporting/builds/144847025 (including cross-repo dependencies).

If there is interest, I can finish this PR (ie update mqt test suite mainly).

@moylop260
Copy link
Contributor

moylop260 commented Jul 15, 2016

  • Is possible consider auto-create the setup folder when this one is not exists?

    Many people are using MQT to build theirs projects but we will require to create the setup folder and this matter maybe is not welcome.

  • After merging what file we will need delete from our repositories? (oca_dependencies.txt, requirements.txt...)

  • There is no need for oca_dependencies.txt anymore as all dependencies are expressed in the openerp.py

    What about if I want use a module from my custom repo instead of official repo?
    What about use a dummy PR where I want to use a developer branch of a dependency to test it with other project e.g. reporting-engine-dev-x testing from account-financial-reporting?

@sbidoul
Copy link
Member Author

sbidoul commented Jul 15, 2016

@moylop260 thanks for looking into this.

The setup directory is easily created with setuptools-odoo-make-default -d . (part of setuptools-odoo package). It could be autogenerated in travis_install_nightly too. For OCA, it's built nightly by the git bot if it does not exist yet.

If you want to work with a specifc branch of a dependency instead of getting it from wheelhouse.odoo-community.org, you just add it to the requirements.txt file, for example
git+https://github.com/acsone/reporting-engine.git@somebranch#subdirectory=setup/report_xslx&egg=odoo-addon-report_xslx

You can even use different branches for different addons in the same repository.

You can also declare other places to look for dependencies by adding --find-links https://mywheelhouse/... in the same requirements.txt.

So it's very flexible.

@moylop260
Copy link
Contributor

moylop260 commented Jul 15, 2016

In principle, look good to me
Could you list the change that we will need execute in all oca projects after merging, please?

@sbidoul
Copy link
Member Author

sbidoul commented Jul 16, 2016

@moylop260 there is nothing to change in OCA projects. We can remove oca_dependencies.txt when we are satisfied everything works fine.

One limitation is that it currently does not work with OpenErp 7.0 because

Alternatively, I could preserve the current behaviour with oca_dependencies for 7.0, or use the new behaviour only if oca_dependencies.txt is not present.

@moylop260
Copy link
Contributor

I got it
Thank you

I like use pip if oca_dependencies is not present

@sbidoul
Copy link
Member Author

sbidoul commented Jul 16, 2016

I like use pip if oca_dependencies is not present

Ok, I will do that, it allows for a progressive deployment

@dreispt
Copy link
Member

dreispt commented Aug 21, 2016

It's a great idea to keep using the oca_dependencies.txt as now, and using pip installation when that file is not available.
I'll be using the dependencies logic for a the sattelite repo PoC (#338), so I need that code to be kept even if we use pip installation for dependencies.

@sbidoul
Copy link
Member Author

sbidoul commented Aug 21, 2016

use pip if oca_dependencies is not present

That mean pip will be used for all repos that have no oca_dependencies.txt today. There are many of them. I'm personally fine with that. I'll also need to disable the pip approach for 7.0 as we don't build wheels for 7.0 yet.

@dreispt
Copy link
Member

dreispt commented Aug 23, 2016

That mean pip will be used for all repos that have no oca_dependencies.txt today.

Yes, but I would expect for no Odoo module to be actually pip installed in those repos: if you have no oca_dependencies.txt, you're not using any modules outside the repo.

If you want to play safe, maybe you can use pip install only if a requirements.txt file is present, even if empty...

@sbidoul
Copy link
Member Author

sbidoul commented Oct 4, 2016

This one should be good now. The new behavior is triggered if oca_dependencies.txt is absent.

@dreispt
Copy link
Member

dreispt commented Oct 12, 2016

AFAICS when we have an "oca_dependencies.txt" in the repo, behaviour is unchanged.
If it is missing, we then setuptools package the repo addons, and then pip install Odoo and these addons.

Questions:
a) I assume that this will seamlessly handle Python dependencies declared in the manifests, right?
b) What happens if our pip-installed addons have dependencies from other repos that are not packaged for pip installation?

@sbidoul
Copy link
Member Author

sbidoul commented Oct 12, 2016

a) I assume that this will seamlessly handle Python dependencies declared in the manifests, right?

Yes, setuptools-odoo does that.

b) What happens if our pip-installed addons have dependencies from other repos that are not packaged for pip installation?

OCA addons can only depend on OCA addons. All OCA repos are packaged for pip installation automatically by a script running nightly.

An improvement I'm considering is to have travis push to wheelhouse.odoo-community.org on success, so we don't have to wait the next day in case we have new cross-repo dependencies.

@sbidoul
Copy link
Member Author

sbidoul commented Oct 12, 2016

One thing that remains TODO here is to disable the pip install behavour for Odoo versions <= 7.

@lmignon
Copy link
Contributor

lmignon commented Oct 12, 2016

@sbidoul May be this change is enough... acsone@01d3c99
not yet tested

@pedrobaeza
Copy link
Member

You should also do the same for lower versions (6.1, or even 5.0, 4.2 as we have in OCA/l10n-spain).

@lmignon
Copy link
Contributor

lmignon commented Oct 12, 2016

@pedrobaeza I'm not aware of projects using travis on branches < 7.0 In OCA/l10n-spain travis is not configured on 6.1, or even 5.0, 4.2 )

@pedrobaeza
Copy link
Member

OK, you're right. Transifex is not mandatory for that old ones.

@sbidoul
Copy link
Member Author

sbidoul commented Oct 12, 2016

I pushed the check for versions <= 7.0. I hate bash.

@blaggacao
Copy link

blaggacao commented Jan 14, 2017

So resuming the tooling, we will see in this space in the future:

  • pip (this PR and odoo-setuptools).

    • theoretical advantage: formalized release cycle through packaging, pip ubiquity
    • theoretical disadvantage: special tooling for packaging required, private code requires additional dedicated infrastructure
  • git + mv (oca dependencies).

    • theoretical advantage: home made and well known
    • theoretical disadvantage: entry barrier (although it's actually dead simple, but the fact of sounding unknown and specialized scares away). Somewhat a replica of git submodule in bash.
  • git submodules

    • theoretical advantage: git, production / development parity
    • theoretical disadvantage: somewhat complicated
  • git subtree

    • theoretical advantage: git, many say easier than submodule
    • theoretical disadvantage: huge repositories due to multiple histories in single repo, well odoo shallow is already 110MB, so it's relative.
  • other wrappers?

We use git submodule as it is as agnostic as it can be. For us: agnostic == strategic; True
Never the less @sbidoul I like the suggestion!

@sbidoul
Copy link
Member Author

sbidoul commented May 6, 2017

I updated this PR for odoo 10.0 and pypi support.

An example build of account-financial-reporting without oca_dependencies.txt is here: https://travis-ci.org/OCA/account-financial-reporting/jobs/229481888#L403

I think it is ready to merge now.

cc/ @dreispt

@@ -91,13 +112,17 @@ echo "Install webkit (wkhtmltopdf) patched"
# |___ <OdooRepo>-<Branch>/ <-- Odoo Server
# |___ maintainer-quality-tools/
# |___ build/<Owner>/<TestedRepo>/
# |___ dependencies/<DependencyRepo1>/
# |___ dependencies/<DependencyRepo2>/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dependency dirs will still be there if we have an oca_dependencies.txt, so IMO we should keep this.

@sbidoul sbidoul force-pushed the pip-install-sbi branch 2 times, most recently from 90bd451 to 2776c04 Compare May 6, 2017 19:40
@sbidoul sbidoul force-pushed the pip-install-sbi branch from 2776c04 to 1fb65e5 Compare May 6, 2017 19:44
@sbidoul
Copy link
Member Author

sbidoul commented May 6, 2017

@dreispt indeed, that was a remnant of the initial prototype. Fixed and rebased.

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code work for months for the Alfodoo project... https://github.com/acsone/alfodoo/blob/10.0/.travis.yml#L32

@bealdav
Copy link
Member

bealdav commented May 8, 2017

nice feature, thanks a lot for this work 👍

@sbidoul
Copy link
Member Author

sbidoul commented Mar 12, 2020

superseded by #646

@sbidoul sbidoul closed this Mar 12, 2020
@sbidoul sbidoul deleted the pip-install-sbi branch March 12, 2020 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants