[WIP] Autogenerate repos.yaml for missing addons #86

PCatinean · 2017-10-03T16:32:41Z

…r automatic dependency resolution

yajo · 2017-10-03T16:37:46Z

build.d/300-oca-dependencies

+        if len(parts) > 2:
+            branch = parts[2]
+        else:
+            branch = os.environ.get('VERSION', '10.0')


This env var is called ODOO_VERSION, and you should not default to any value. It is considered an error if it is not available. Just use brackets and let the proper KeyError bubble up if it ever happens.

yajo · 2017-10-03T16:38:22Z

build.d/300-oca-dependencies

+
+# Method from maintainer-quality-tools
+def parse_depfile(depfile, owner='OCA'):
+    deps = []


I think this should be a set, to avoid possible duplicates.

yajo · 2017-10-03T16:38:55Z

build.d/300-oca-dependencies

+
+def process_dependencies(dep_list):
+    for dep in dep_list:
+        if dep in processed_dependencies:


declare this in the method?

yajo · 2017-10-03T16:39:37Z

build.d/300-oca-dependencies

+    return True
+
+addons_yaml = yaml.load(open('%s/addons.yaml' % CUSTOM_DIR, 'r'))
+repos_yaml = yaml.load(open('%s/repos.yaml' % CUSTOM_DIR, 'r'))


Get these from odoobaselib

yajo · 2017-10-03T16:39:49Z

build.d/300-oca-dependencies

+addons_yaml = yaml.load(open('%s/addons.yaml' % CUSTOM_DIR, 'r'))
+repos_yaml = yaml.load(open('%s/repos.yaml' % CUSTOM_DIR, 'r'))
+
+branch = os.environ.get('VERSION', '10.0')


Same as above, no .get!

yajo · 2017-10-03T16:40:03Z

build.d/300-oca-dependencies

+
+branch = os.environ.get('VERSION', '10.0')
+processed_dependencies = set()
+base_url = 'https://raw.githubusercontent.com/OCA'


This should be a constant at the top of the file

yajo · 2017-10-03T16:40:21Z

build.d/300-oca-dependencies

+base_url = 'https://raw.githubusercontent.com/OCA'
+
+# Find all dependency files after initial aggregation
+dep_files = glob.glob('%s/**/oca_dependencies.txt' % CUSTOM_DIR)


yajo · 2017-10-03T16:41:13Z

build.d/300-oca-dependencies

+process_dependencies(initial_dependencies)
+
+auto_addons_yaml = open('%s/addons.yaml' % ADDONS_DIR, 'w')
+auto_repos_yaml = open('%s/repos.yaml' % ADDONS_DIR, 'w')


Declare these at odoobaselib

yajo · 2017-10-03T16:41:46Z

build.d/300-oca-dependencies

+        'target': {'origin': '$ODOO_VERSION'},
+        'merges': [{'origin': '$ODOO_VERSION'}],
+    }
+    repos_yaml[dep[0]] = {'./%s' % dep[0]: repo_vals}


Remove the ./ please

lasley

LGTM outside of @yajo comments

PCatinean · 2017-10-08T19:51:30Z

@yajo added the mentioned changes. However in local tests it fails, i need to debug to fix the issues (one being it should not run if AGGREGATE env var is not defined). It also has to start the second aggregation once everything worked.

yajo · 2017-10-09T08:34:00Z

docker-compose.yml

@@ -0,0 +1 @@
+devel.yaml


Remove this file.

yajo · 2017-10-09T08:35:23Z

build.d/300-oca-dependencies


 # Find all dependency files after initial aggregation
-dep_files = glob.glob('%s/**/oca_dependencies.txt' % CUSTOM_DIR)
+dep_files = iglob('%s/**/oca_dependencies.txt' % CUSTOM_DIR)


No need for double asterisk, I think the file can only be found at repo root

yajo · 2017-10-09T08:38:02Z

build.d/300-oca-dependencies

+        if len(parts) > 1:
+            url = parts[1]
+        else:
+            url = 'https://github.com/%s/%s.git' % (owner, repo)


You should add to the dockerfile ONBUILD ARG DEFAULT_REPO_PATTERN="https://github.com/OCA/%s.git" and rely on that env variable instead.

yajo · 2017-10-09T08:39:04Z

build.d/300-oca-dependencies

+    repo_vals = {
+        'defaults': {'depth': '$DEPTH_DEFAULT'},
+        'remotes': {'origin': dep[1]},
+        'target': {'origin': '$ODOO_VERSION'},


The target is a string.

I think I checked the output of this and it made the proper output i.e:

target: origin $ODOO_VERSION

Isn't that right?

Yes, that's the proper output, although I'd be surprised if it gets there like that. I'd expect this line to be 'target': 'origin $ODOO_VERSION', instead.

Oh I see, because in the link you gave and in the default repos yaml it's not as a string

yajo · 2017-10-09T08:41:03Z

build.d/300-oca-dependencies

+
+# Generate auto_addons and auto_repos yaml files
+for dep in dependencies:
+    ADDONS_YAML[dep[0]] = ['*']


ADDONS_YAML only contains the string of the file location. The actual parsing is done at odoobaselib.addons_config()

Ah so I need to use the result from that method to find out the actual addons instead of just using the file itself. Any idea how I can debug easier maybe even use PDB to save time and not build everything from scratch to test the python script?

Try running a project with scaffolding and everything, and mounting this script as a volume under it. Then run it manually inside the container.

yajo · 2017-10-09T08:42:20Z

build.d/300-oca-dependencies

+        'target': {'origin': '$ODOO_VERSION'},
+        'merges': [{'origin': '$ODOO_VERSION'}],
+    }
+    REPOS_YAML[dep[0]] = {'%s' % dep[0]: repo_vals}


REPOS_YAML contains only the file string. Its actual parsing is done by git-aggregator itself by using the autoaggregate script. Possibly you'll need to add an argument to that string that lets you specify the file you are aggregating (from custom or from auto).

PCatinean · 2017-10-24T11:35:11Z

I will resume work on this when I get a bit more time. I couldn't follow every reply on the mailing list regarding the pip installing method vs Doodba. This is still needed right?

yajo · 2017-10-24T12:39:34Z

Yes, as explained in OCA/runbot-addons#144 (comment), I still see many benefits on addons.yaml system, I don't think it will fade away soon (although pip installing support is still there, of course)

yajo · 2017-11-02T08:01:23Z

According to OCA/runbot-addons#144 (comment) (and mostly to the whole conversation) it seems OCA is not interested in this project, so please @PCatinean could you split this PR in 2?

Autofill auto/repos.yaml when a repo is declared in custom/src/addons.yaml but not in custom/src/repos.yaml by using $DEFAULT_REPO_PATTERN.
Implement oca_dependencies.txt recursive translation to this image's systems.

... Or otherwise just do step 1. Step 2 was a step needed to use Doodba in OCA. Since that's not going to happen, most likely it means that it goes to the collection of stuff that we just won't need. 🤔

PCatinean · 2017-11-02T13:37:31Z

@yajo I see, so basically in step 1 whenever an addon is declared in addons.yaml but it's not present in repos.yaml assume it's an oca repo and update repos.yaml accordingly?

yajo · 2017-11-03T12:24:43Z

yep!

It should be OK now

With this change, definitely we can have a Doodba scaffolding without `repos.yaml` file. It will do this: 1. Download all git code from repos in `repos.yaml`, if any. 2. Get the list of expected code repos from `addons.yaml`. 3. Autogenerate a `repos.yaml` file for all expected but absent repos, based on the provided patterns. 4. Download all of that missing code. 5. Continue normal operation. As a 🎁, we now download git code in parallel if the building machine has more than 1 CPU. Some tests have been modified to ensure they still pass with this new feature.

- `l10n-spain` was never a good sample, since only Spanish projects would have it. Since Tecnativa/doodba#86 it's simply not needed in most cases. Devs can check the docs to know the structure. Thus, it can be removed from here. - `checksumdir` is not used anymore in `module_auto_update` since OCA/server-tools#1190. - `xlsxwriter` is included in Odoo 10+. - `requests` is included in Odoo 8+.

- `l10n-spain` was never a good sample, since only Spanish projects would have it. Since Tecnativa/doodba#86 it's simply not needed in most cases. Devs can check the docs to know the structure, and a vscode snippet has been added. Thus, it can be removed from here. - `checksumdir` is not used anymore in `module_auto_update` since OCA/server-tools#1190. - `xlsxwriter` is included in Odoo 10+. - `requests` is included in Odoo 8+.

Since Tecnativa/doodba#86, no addons or repos are required. This comment is an old true that is today a lie, so it's better away from here.

yajo suggested changes Oct 3, 2017

View reviewed changes

yajo force-pushed the master branch 7 times, most recently from 635050e to e88faa0 Compare October 5, 2017 07:20

yajo mentioned this pull request Oct 5, 2017

[RFC] Future of Runbot OCA/runbot-addons#144

Closed

lasley approved these changes Oct 5, 2017

View reviewed changes

PCatinean changed the title ~~[WIP - skip ci] Translating oca_dependencies to addons/repos .yaml fo…~~ [WIP] Translating oca_dependencies to addons/repos .yaml fo… Oct 8, 2017

yajo previously requested changes Oct 9, 2017

View reviewed changes

1st attempt to autodownload repos when specified only in addons.yaml

210188c

yajo changed the title ~~[WIP] Translating oca_dependencies to addons/repos .yaml fo…~~ [WIP] Autogenerate repos.yaml for missing addons Sep 13, 2018

yajo self-assigned this Sep 13, 2018

yajo added the enhancement label Sep 13, 2018

yajo merged commit 070323c into Tecnativa:master Sep 14, 2018

yajo added a commit that referenced this pull request Sep 18, 2018

Document new things landed in #86

9bfbac2

yajo mentioned this pull request Feb 25, 2019

[DCK] Remove outdated samples Tecnativa/doodba-scaffolding#23

Merged

yajo added a commit to Tecnativa/doodba-scaffolding that referenced this pull request Apr 23, 2019

[DCK] Odoo is no longer required

5581611

Since Tecnativa/doodba#86, no addons or repos are required. This comment is an old true that is today a lie, so it's better away from here.

yajo mentioned this pull request Apr 23, 2019

[DCK] Odoo is no longer required Tecnativa/doodba-scaffolding#32

Merged

yajo added a commit to Tecnativa/doodba-scaffolding that referenced this pull request Apr 23, 2019

[DCK] Odoo is no longer required

e2f883a

Since Tecnativa/doodba#86, no addons or repos are required. This comment is an old true that is today a lie, so it's better away from here.

mtelahun pushed a commit to trevi-software/doodba-scaffolding that referenced this pull request Jul 15, 2019

[DCK] Odoo is no longer required

08d6488

Since Tecnativa/doodba#86, no addons or repos are required. This comment is an old true that is today a lie, so it's better away from here.

mtelahun pushed a commit to trevi-software/doodba-scaffolding that referenced this pull request Jul 15, 2019

[DCK] Odoo is no longer required

bec5059

Since Tecnativa/doodba#86, no addons or repos are required. This comment is an old true that is today a lie, so it's better away from here.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Autogenerate repos.yaml for missing addons #86

[WIP] Autogenerate repos.yaml for missing addons #86

PCatinean commented Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

yajo Oct 3, 2017

lasley left a comment

PCatinean commented Oct 8, 2017 •

edited

Loading

yajo Oct 9, 2017

yajo Oct 9, 2017

yajo Oct 9, 2017

yajo Oct 9, 2017

PCatinean Oct 11, 2017

yajo Oct 11, 2017

PCatinean Oct 11, 2017

yajo Oct 9, 2017

PCatinean Oct 11, 2017

yajo Oct 11, 2017

yajo Oct 9, 2017

PCatinean commented Oct 24, 2017

yajo commented Oct 24, 2017

yajo commented Nov 2, 2017

PCatinean commented Nov 2, 2017

yajo commented Nov 3, 2017

		@@ -0,0 +1 @@
		devel.yaml

[WIP] Autogenerate repos.yaml for missing addons #86

[WIP] Autogenerate repos.yaml for missing addons #86

Conversation

PCatinean commented Oct 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasley left a comment

Choose a reason for hiding this comment

PCatinean commented Oct 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PCatinean commented Oct 24, 2017

yajo commented Oct 24, 2017

yajo commented Nov 2, 2017

PCatinean commented Nov 2, 2017

yajo commented Nov 3, 2017

PCatinean commented Oct 8, 2017 •

edited

Loading