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

[IMP] Add option to automatically install repo requirements #28

Merged
merged 2 commits into from
Aug 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ ONBUILD ARG DEPTH_MERGE=100
ONBUILD ARG CLEAN=true
ONBUILD ARG COMPILE=true
ONBUILD ARG CONFIG_BUILD=true
ONBUILD ARG AUTO_REQUIREMENTS=true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should have been false

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, as a good sane compatibility default, but anyway the problem is still there, and you will face it when you set this param.

Copy link
Contributor Author

@lasley lasley Aug 24, 2017

Choose a reason for hiding this comment

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

This is more for a development box, and while it can cause issues it also is incredibly helpful (at least it has been for me).

Is there any reason to not leave this feature as is, just with the good default? This is one of the last things I need to move off of my fork.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issues are during build too, which is the perfect time to resolve them. It really isn't a problem when you're setting up the box, only when a bunch of pre-setup boxes get the wrong default IMO

Copy link
Member

Choose a reason for hiding this comment

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

OK, please resend the PR with that defaulting to False.

ONBUILD ARG PIP_INSTALL_ODOO=true
ONBUILD ARG ADMIN_PASSWORD=admin
ONBUILD ARG SMTP_SERVER=smtp
Expand Down
5 changes: 0 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -881,11 +881,6 @@ Dockerfile similar to this one:
FROM tecnativa/odoo-base@sha256:fba69478f9b0616561aa3aba4d18e4bcc2f728c9568057946c98d5d3817699e1
```

### What is `/opt/odoo/custom/src/requirements.txt`?

This is the deprecated file path for the PIP requirements. Use
`/opt/odoo/custom/dependencies/pip.txt` instead.

### How can I help?

Just [head to our project](https://github.com/Tecnativa/docker-odoo-base) and
Expand Down
35 changes: 0 additions & 35 deletions build.d/100-dependencies

This file was deleted.

File renamed without changes.
28 changes: 28 additions & 0 deletions build.d/200-dependencies
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env python
# Copyright 2017 LasLabs Inc.

from os.path import join

from odoobaselib import (
addons_config,
AUTO_REQUIREMENTS,
CUSTOM_DIR,
FILE_APT_BUILD,
SRC_DIR,
)
from odoobaselib.installer import install, INSTALLERS


# Build dependencies installed before any others
install("apt", FILE_APT_BUILD)

for name in INSTALLERS:
req_files = []
# pip `requirements.txt` files found in repositories
if name == "pip" and AUTO_REQUIREMENTS:
for repo in addons_config():
req_files.append(join(SRC_DIR, repo, 'requirements.txt'))
# Normal dependency installation
req_files.append(join(CUSTOM_DIR, 'dependencies', '%s.txt' % name))
for req_file in req_files:
install(name, req_file)
1 change: 1 addition & 0 deletions lib/odoobaselib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
ADDONS_YAML = os.path.join(SRC_DIR, 'addons')
ADDONS_DIR = "/opt/odoo/auto/addons"
CLEAN = os.environ.get("CLEAN") == "true"
AUTO_REQUIREMENTS = os.environ.get("AUTO_REQUIREMENTS") == "true"
LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR")
FILE_APT_BUILD = os.path.join(
CUSTOM_DIR, 'dependencies', 'apt_build.txt',
Expand Down
11 changes: 9 additions & 2 deletions lib/odoobaselib/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ def cleanup(self):
def install(self):
"""Install the requirements from the given file."""
if self._requirements:
self._run_command(self._install_command + self._requirements)
return not self._run_command(
self._install_command + self._requirements)
else:
logging.info("No installable requirements found in %s",
self.file_path)
return False

def remove(self):
"""Uninstall the requirements from the given file."""
Expand Down Expand Up @@ -102,7 +104,7 @@ class PipInstaller(Installer):

def requirements(self):
"""Pip will use its ``--requirements`` feature."""
return [self.file_path]
return [self.file_path] if exists(self.file_path) else []


INSTALLERS = {
Expand All @@ -111,3 +113,8 @@ def requirements(self):
"npm": NpmInstaller,
"pip": PipInstaller,
}


def install(installer, file_path):
"""Perform a given type of installation from a given file."""
return INSTALLERS[installer](file_path).install()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/usr/bin/env python
from cfssl import CFSSL
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cfssl