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

    hundreds-father-404

    04/29/2020, 6:52 PM
    Agreed to both points. I’m conflicted on whether to use write or materialize. FWIT, if we stick with materialize, we can rename 
    DirectoryToMaterialize
    to 
    MaterializeDigest
    . Or, actually, maybe 
    MaterializeToBuildRoot
    WriteToBuildRoot
    ? It’s long enough that we discourage using it. And it resolves one of the biggest confusions: you can only write to the build root, not any arbitrary place
  • a

    aloof-angle-91616

    04/29/2020, 6:57 PM
    "write" still seems far too legitimate. 
    write()
    is a single syscall. materializing is also a syscall of sorts if pants is the OS but it's many different blocking operations in sequence. "write" doesn't feel like it describes what's happening
  • h

    hundreds-father-404

    04/29/2020, 6:58 PM
    And 
    SaveToBuildRoot
    is the same issue, right? Probably even worse, because it’s more vague.
  • w

    witty-crayon-22786

    04/29/2020, 7:01 PM
    “materialize”, “checkout”, “write”, “publish”, etc. the type name is probably less important than the method name in that case, because the method is synchronous.
  • w

    witty-crayon-22786

    04/29/2020, 7:02 PM
    i’d suggest looking at “synchronous actions on the workdir” independently from types that rules operate on asynchronously.
    👍 1
  • h

    hundreds-father-404

    04/29/2020, 8:03 PM
    Python 3.9 adds 
    removeprefix
    to the stdlib, rather than 
    stripprefix
    , via PEP 616: https://www.python.org/dev/peps/pep-0616/#alternative-method-names Any opposition to 
    RemovePrefix
    instead of 
    StripPrefix
    ? I think I prefer “strip” to “remove”, but I prefer parity with the stdlib even more.
  • a

    aloof-angle-91616

    04/29/2020, 8:03 PM
    yes, +1, especially if we mention the naming rationale in the docstring
    👍 1
  • w

    witty-crayon-22786

    04/29/2020, 8:07 PM
    i sortof agree with Greg that a holistic look at a Snapshot manipulation API might be good. https://pantsbuild.slack.com/archives/C0D7TNJHL/p1588123784367500
  • w

    witty-crayon-22786

    04/29/2020, 8:08 PM
    moreso than name changes, per-se.
  • w

    witty-crayon-22786

    04/29/2020, 8:09 PM
    maybe we’re close enough with the current API that doing that can wait. but it feels like an eventual thing.
  • w

    witty-crayon-22786

    04/29/2020, 8:12 PM
    total strawperson:
    Copy code
    SnapshotOf(
  input=input_snapshot,
  changes=[
    Rename('x', 'y'),
    MkDir('z'),
  ],
)
    💯 1
  • h

    hundreds-father-404

    04/29/2020, 8:18 PM
    That would be really nice. In the meantime, these changes are fairly trivial to make and get us some forward progress
  • w

    witty-crayon-22786

    04/29/2020, 9:37 PM
    @hundreds-breakfast-49010: mm: noticed one issue in the workunits for UI change: rendering in Scheduler::execute won’t limit to the roots that are currently being executed
  • w

    witty-crayon-22786

    04/29/2020, 9:38 PM
    … not a huge issue, actually. since the workunitstore should only contain things for the current run…? ok. maybe that’s not an issue at all.
    👌 2
  • h

    hundreds-father-404

    04/30/2020, 1:49 AM
    Any opinions on 
    SnapshotSubset
    -> 
    DigestSubset
    ? The name is a misnomer atm because it takes a digest, not a Snapshot. 
    DigestSubset
    would also go nicely with 
    MergeDigests
    a
    • 2
    • 3
  • a

    aloof-angle-91616

    04/30/2020, 1:56 AM
    Copy code
    = note: ld: warning: ignoring file /Users/dmcclanahan/tools/p1/src/rust/engine/target/debug/build/engine_cffi-0cfc0b2057922e53/out/libnative_engine_ffi.a, building for macOS-x86_64 but attempting to link with file built for unknown-unsupported file format ( 0x21 0x3C 0x61 0x72 0x63 0x68 0x3E 0x0A 0x2F 0x20 0x20 0x20 0x20 0x20 0x20 0x20 )
          Undefined symbols for architecture x86_64:
            "_wrapped_PyInit_native_engine", referenced from:
                _PyInit_native_engine in engine_cffi.2e9skz2skowhnlv2.rcgu.o
            "_wrapped_initnative_engine", referenced from:
                _initnative_engine in engine_cffi.2e9skz2skowhnlv2.rcgu.o
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)
    does anyone know when this might happen? for what it's worth, 
    libnative_engine_ffi.a
    is a normal 
    ar
    archive, and it contains 
    native_engine.so
    , which is definitely a mach-o x86_64 file:
    Copy code
    X> file /Users/dmcclanahan/tools/p1/src/rust/engine/target/debug/build/engine_cffi-0cfc0b2057922e53/out/src/cffi/native_engine.o

/Users/dmcclanahan/tools/p1/src/rust/engine/target/debug/build/engine_cffi-0cfc0b2057922e53/out/src/cffi/native_engine.o: Mach-O 64-bit object x86_64
    not sure if this is possibly due to missing 
    macosx-version-min
    value? investigating
    👀 1
  • a

    aloof-angle-91616

    04/30/2020, 1:59 AM
    oh sorry, to be clear, the real error is the recurring:
    Copy code
    Exception message: dlopen(/Users/dmcclanahan/tools/p1/native_engine.so, 2): Symbol not found: _wrapped_PyInit_native_engine
  Referenced from: /Users/dmcclanahan/tools/p1/native_engine.so
  Expected in: flat namespace
 in /Users/dmcclanahan/tools/p1/native_engine.so
    i removed 
    -undefined dynamic_lookup
    from 
    engine_cffi/build.rs
    to try to get it to fail at build time instead of runtime, and produced the above message, which was progress
    👍 1
  • a

    aloof-angle-91616

    04/30/2020, 6:20 AM
    i didn't solve it yet, but i was able to get 
    ./v2 goals
    running by manually adding the 
    native_engine.o
    file to the 
    rustc-cdylib-link-arg
    set
    💯 1
  • a

    aloof-angle-91616

    04/30/2020, 6:21 AM
    so one way to work around this is to manually search for and find the 
    native_engine.o
    file in the target directory, and manually add it to the list of files to link, because the 
    cc
    crate is producing an incorrect command line     
    ar
    is producing an incorrect 
    libnative_engine_ffi.a
    somehow
  • a

    aloof-angle-91616

    04/30/2020, 6:22 AM
    converting 
    -Wl,--exported_symbols_list,/var/...
    to 
    -Wl,-export_dynamic
    also made the link work     EDIT: this is wrong
  • a

    aloof-angle-91616

    04/30/2020, 6:22 AM
    so i'm looking now to just do that, which would involve temporarily forking the 
    cc
    crate, unless there's an easy way to do this without forking it (which i don't think will be the case)
  • a

    aloof-angle-91616

    04/30/2020, 6:24 AM
    looking to fix the 
    cc
    crate's creation of 
    libnative_engine_ffi.a
    , rather
  • a

    aloof-angle-91616

    04/30/2020, 6:29 AM
    i noticed that the homebrew version of 
    ar
    is earlier on my path than the one in 
    /usr/bin
    and that seems extremely suspicious
  • a

    aloof-angle-91616

    04/30/2020, 6:37 AM
    there's so much ad-hoc shell script caching in the engine creation process and it makes debugging all of this a nightmare every time. note to self to get back to the rust v2 impl now that the target API will make it easier to review
  • a

    aloof-angle-91616

    04/30/2020, 6:39 AM
    wow, yep. it was the homebrew 
    ar
    , which comes from 
    coreutils
    i'm pretty sure
  • a

    aloof-angle-91616

    04/30/2020, 6:39 AM
    need to make sure 
    /usr/bin/ar
    is used on OSX at all times, i think (going to verify that it was indeed just 
    ar
    causing this). that can be set with the 
    AR
    env var so the diff should be small
  • a

    aloof-angle-91616

    04/30/2020, 6:57 AM
    have a tiny diff. rebuilding all the rust again to make sure it works...
  • a

    aloof-angle-91616

    04/30/2020, 7:46 AM
  • h

    hundreds-breakfast-49010

    04/30/2020, 7:47 AM
    ugh bugs based on PATH
  • h

    hundreds-breakfast-49010

    04/30/2020, 7:47 AM
    I think PATH is a bad idea to be honest
1...723724725...906Latest