overriding multiple recipe repos

[Paweł Hajdan, Jr.]

I was working on some complex changes today, requiring me to override both recipe_engine and depot_tools for build repo. That generated the following error:

~/recipes-py$ ./recipes.py --package ~/infra_internal/build/infra/config/recipes.cfg -O recipe_engine=~/recipes-py -O depot_tools=~/depot_tools simulation_test train

InconsistentDependencyGraphError: Package specs for recipe_engine do not match: PathRepoSpec{path="/usr/local/google/home/phajdan/recipes-py"} vs GitRepoSpec{project_id="recipe_engine", repo="https://chromium.googlesource.com/external/github.com/luci/recipes-py.git", branch="master", revision="b120624b72ec851b0d9f2accccf58e525555be25", path=""}

I remember this used to work, and indeed, it does after reverting https://codereview.chromium.org/2343733003 .

Enforcing consistency among overridden repos makes sense to me. However, no repo is going to match with PathRepoSpec anyway.

Should we make any repo match PathRepoSpec when overriding? Are there alternative solutions you'd recommend?

Paweł

[Robert Iannucci]

I suppose. I'm actually not really sure why we try to calculate any inconsistencies when we pass -O... Maybe they should just be warnings instead of errors?

I don't feel strongly by any change here. The goal of -O is to make it easy to work with recipes... if it's hard to do then it's not doing its job :).

R

[Paweł Hajdan, Jr.]

Thanks, Robbie!

I landed https://codereview.chromium.org/2657973006/ today as an attempt to tackle this. It didn't work though - the error is now different, see below. Only after I revert that change and f7ada90c926c78704f048767e29e8ae836d8a65b (https://codereview.chromium.org/2343733003 , already mentioned above) this works as expected.

Dan, I'd be really interested in rationale and context behind https://codereview.chromium.org/2343733003 . From what I see it's a clear regression from working behavior, and it's not obvious to me what that CL fixes.

To demonstrate the issue, I uploaded https://codereview.chromium.org/2663943002 for the recipe engine and https://chromium-review.googlesource.com/c/434279/ for depot_tools. I'm trying to train build recipe expectations at revision d18d3bc5e9df7008d8589f8d0252a099171b116b .

While in both cases they fail (I need to make changes to build repo to pass cwd via context rather than kwargs), in the really broken case the following error is present. Please note the recipe engine change adds get_context method to step recipe module, and despite overriding recipe engine path the override seems not to take effect:

~/recipes-py$ ./recipes.py --package ~/infra_internal/build/infra/config/recipes.cfg -O recipe_engine=~/recipes-py -O depot_tools=~/depot_tools simulation_test train

client.nacl.sdk.recipe_autogen.windows_sdk_multirel failed: Traceback (most recent call last):

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/third_party/expect_tests/pipeline.py", line 166, in generate_tests_results

    for subtest, subresult in test.process(process_test):

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/third_party/expect_tests/type_definitions.py", line 281, in process

    yield self, func(self.bind(context=None))

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/third_party/expect_tests/pipeline.py", line 143, in process_test

    subresult = subtest.run()

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/third_party/expect_tests/type_definitions.py", line 270, in run

    return self.func_call(context=context)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/third_party/expect_tests/type_definitions.py", line 195, in __call__

    return f.func(*f.args, **f.kwargs)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/simulation_test.py", line 124, in RunRecipe

    result = engine.run(recipe_script, api, test_data.properties)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/run.py", line 443, in run

    return self._old_run(recipe_script, api, properties)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/run.py", line 499, in _old_run

    recipe_result = recipe_script.run(api, properties)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/loader.py", line 82, in run

    self.run_steps, properties, self.PROPERTIES, api=api)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/loader.py", line 556, in invoke_with_properties

    **additional_args)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/loader.py", line 517, in _invoke_with_properties

    return callable_obj(*props, **additional_args)

  File "/usr/local/google/home/phajdan/infra_internal/build/scripts/slave/recipes/client.nacl.sdk.recipe_autogen.py", line 127, in RunSteps

    dispatch_directory[buildername](api)

  File "/usr/local/google/home/phajdan/infra_internal/build/scripts/slave/recipes/client.nacl.sdk.recipe_autogen.py", line 88, in sdk_multirel_steps

    api.chromium.runhooks(env=env)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/recipe_api.py", line 505, in _inner

    return func(*a, **kw)

  File "/usr/local/google/home/phajdan/infra_internal/build/scripts/slave/recipe_modules/chromium/api.py", line 667, in runhooks

    self.m.gclient.runhooks(**kwargs)

  File "/usr/local/google/home/phajdan/recipes-py/recipe_engine/recipe_api.py", line 505, in _inner

    return func(*a, **kw)

  File "/usr/local/google/home/phajdan/depot_tools/recipe_modules/gclient/api.py", line 252, in runhooks

    if not self.m.step.get_context().get('cwd'):

AttributeError: 'StepApi' object has no attribute 'get_context'

I'm attaching full output of the command for reference.

Paweł

[d...@google.com]

> Dan, I'd be really interested in rationale and context behind https://codereview.chromium.org/2343733003 . From what I see it's a clear regression from working behavior, and it's not obvious to me what that CL fixes.

I'm going to try and remember the problem that I hit 4 months ago, but understand that this isn't something I've used or thought about in quite some time, so I might be a bit off. The rationale/context was that a repository's pinned dependencies have to be allowed to be ignored for overriding.

/stuff/foo, requires Engine@X, bar @A
bar@A, requires Engine@Y

$ recipes.py -O foo=/stuff/foo

This override picture creates a conflict with respect to engine version. That CL makes it so that foo's version is implicitly "correct" and that conflicts are ignored.

[Paweł Hajdan, Jr.]

Hm, I'm quite confused by above scenario.

I'm not sure how we could legitimately arrive at a situation where a repo (foo?) requires engine@X and bar@A, while bar@A requires engine@Y.

Excluding possible confusion about scenario description, why not also override the engine at that point?

In fact, I uploaded https://codereview.chromium.org/2664223002/ which implements two relevant scenarios as tests. When applied on ToT they both fail, but after reverting 30a672d26b45289947783932d8062fc70dfb1611 and f7ada90c926c78704f048767e29e8ae836d8a65b they both pass.

Please let me know if behavior enforced in my tests looks reasonable, and if not, how would you change it.

I'd also be interested in suggestions for the next steps landing these (possibly changed) tests.

Paweł

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.
To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/3717bd3e-7c06-4974-a259-fbed96f03965%40chromium.org.

[Robert Iannucci]

I think it was supposed to solve the scenario if you override foo (depending on engine@1), and also bar (depending on engine@2), but not engine. I think I'm inclined to agree that you should, at that point, just be required to override engine as well.

However we sadly don't have a universal syntax for overrides, so you can't override the engine with something non-local (e.g. you should be able to do -Orecipe_engine=https://github.com/luci/recipes-py.git@deadbeef or something like that and have the engine fetch from github/gitiles/git), which means you need to clone it locally and then point to the local version. Not the end of the world though.

R

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

[Paweł Hajdan, Jr.]

To close the loop here, I landed https://codereview.chromium.org/2664223002/ which fixed issues I was encountering.

Thanks to the new regression tests, any new developments or further fixes should not regress existing functionality.

Paweł

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

[Nodir Turakulov]

InconsistentDependencyGraphError was annoying. Thanks!

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/3717bd3e-7c06-4974-a259-fbed96f03965%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

To post to this group, send email to infr...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAATLsPYZDu9F9jCp5LaSZL_djJGYtNz6Z1RVRCOnmrBFX-HD9w%40mail.gmail.com.