https://pantsbuild.org/ logo
Powered by
#development
  • h

    hundreds-breakfast-49010

    09/05/2019, 10:23 PM
    how should I generate a 
    UrlToFetch
    from a 
    BinaryToolBase
    ?
    a
    w
    • 3
    • 131
  • w

    witty-crayon-22786

    09/05/2019, 11:30 PM
    @early-needle-54791: a lot going on in the thread above, but the bit about "downloading a binary tool and then using it in an ExecuteProcessRequest" feels highly relevant... once your current PR is landed, would be good to ensure that's covered by the design doc
  • h

    hundreds-breakfast-49010

    09/06/2019, 5:46 PM
    @witty-crayon-22786 what's this PR?
  • a

    aloof-angle-91616

    09/06/2019, 6:42 PM
    @hundreds-breakfast-49010 it's got a bit of a confusing title: https://github.com/pantsbuild/pants/pull/8216
  • a

    aloof-angle-91616

    09/06/2019, 6:42 PM
    mentioning the relevance to platform-specific process executions would make it easier for others to find maybe
  • a

    average-vr-56795

    09/09/2019, 9:40 PM
    @witty-crayon-22786 I’m somewhat lost in graph ambiguity… I switched https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/python/rules/inject_init.py to use https://github.com/pantsbuild/pants/pull/8268 and as a result get:
    Copy code
    Ambiguous rules to compute Get(Digest, DirectoryWithPrefixToStrip) with parameter types (Digest+OptionsBootstrapper+PythonTestsAdaptor):
      (Digest, [DirectoryWithPrefixToStrip], <intrinsic>) for DirectoryWithPrefixToStrip
      Param(Digest)
    which confuses me, because I don’t see any places that my change may introduce and ambiguity…
  • a

    aloof-angle-91616

    09/09/2019, 9:40 PM
    i think @hundreds-breakfast-49010 might have been having an issue that looked like that last week?
  • h

    hundreds-breakfast-49010

    09/09/2019, 9:41 PM
    I was running into all sorts of graph ambiguities
  • h

    hundreds-breakfast-49010

    09/09/2019, 9:41 PM
    uh, I think one of the things we found was that if a single rule has more than one rule of the form 
    x = yield Get(Y, ...)
    , that will break?
  • h

    hundreds-breakfast-49010

    09/09/2019, 9:41 PM
    even if the arguments to 
    Get
    are otherwise different
  • a

    average-vr-56795

    09/09/2019, 9:41 PM
    Oh wow, each time I run pants I get a different ambiguity error :S
  • a

    average-vr-56795

    09/09/2019, 9:42 PM
    This rule only has one `yield Get`…
  • a

    aloof-angle-91616

    09/09/2019, 9:43 PM
    there is at least one definite bug: https://github.com/pantsbuild/pants/pull/8227
  • a

    average-vr-56795

    09/09/2019, 9:43 PM
    The entire rule is:
    Copy code
    @rule(InjectedInitDigest, [Snapshot])
def inject_init(snapshot):
  """Ensure that every package has an __init__.py file in it."""
  missing_init_files = tuple(sorted(identify_missing_init_files(snapshot.files)))
  if not missing_init_files:
    new_init_files_digest = EMPTY_DIRECTORY_DIGEST
  else:
    new_init_files_digest = yield Get(Digest, InputFilesContent(FilesContent(tuple(
      FileContent(
        path=path,
        content=b'',
      ) for path in missing_init_files
    ))))
  # TODO(#7710): Once this gets fixed, merge the original source digest and the new init digest
  # into one unified digest.
  yield InjectedInitDigest(directory_digest=new_init_files_digest)
  • a

    aloof-angle-91616

    09/09/2019, 9:48 PM
    oh, i think this is another bug i told greg i was working on
  • a

    average-vr-56795

    09/09/2019, 9:48 PM
    Do you know what the bug is?
  • a

    aloof-angle-91616

    09/09/2019, 9:48 PM
    Ambiguous rules to compute Get(Digest, DirectoryWithPrefixToStrip) with parameter types (Digest+OptionsBootstrapper+PythonTestsAdaptor):
  • a

    aloof-angle-91616

    09/09/2019, 9:48 PM
    yes
  • a

    aloof-angle-91616

    09/09/2019, 9:48 PM
    i think this is what happens if you 
    yield Get()
    for the same thing twice in the same rule too
  • a

    aloof-angle-91616

    09/09/2019, 9:49 PM
    i think the above means 
    Digest
    is already provided as a 
    Param
    somewhere, either explicitly as a 
    RootRule
    or just as a 
    yield Get()
    from another rule
  • a

    aloof-angle-91616

    09/09/2019, 9:49 PM
    i think this is fixable, but the issue is with doing a 
    yield Get()
    for something that is somehow transitively already provided
  • a

    aloof-angle-91616

    09/09/2019, 9:49 PM
    this is exactly the kind of thing that happens when people try to write more rules :) -- we figure out fun issues
  • a

    average-vr-56795

    09/09/2019, 9:50 PM
    Ugh, this sounds like it’s going to end up with a combinatorial explosion of newtypes for each rule+type :S
  • a

    aloof-angle-91616

    09/09/2019, 9:50 PM
    that's the current workaround, but this is definitely a fixable bug imo
  • a

    average-vr-56795

    09/09/2019, 9:51 PM
    Ok 🙂 Thanks
  • w

    witty-crayon-22786

    09/09/2019, 10:34 PM
    Repros appreciated!
  • w

    witty-crayon-22786

    09/09/2019, 10:35 PM
    I think that there is only one open issue about this, but would appreciate more
  • a

    average-vr-56795

    09/09/2019, 10:55 PM
  • w

    witty-crayon-22786

    09/09/2019, 11:11 PM
    The issue I was thinking of is https://github.com/pantsbuild/pants/issues/7710, which I think might have already been created for this case?
  • w

    witty-crayon-22786

    09/09/2019, 11:12 PM
    But if you can minimize it a bit, or bump that ticket because it sounds like it's the same issue, that would be appreciated
1...643644645...906Latest