witty-crayon-22786
06/22/2019, 12:33 AMfn is_union(TypeId) -> Option<TypeId>
hundreds-breakfast-49010
06/22/2019, 12:38 AMwitty-crayon-22786
06/22/2019, 12:39 AM@union
can only possibly by used with the three argument form of Get
. example: https://github.com/pantsbuild/pants/blob/62b566fb55096bc8ef44864667813257e792cb62/src/python/pants/rules/core/test.py#L74-L76fn is_union(TypeId) -> Option<TypeId>
to see whether we failed due to a union, or just a type mismatchhundreds-breakfast-49010
06/24/2019, 6:32 PMPyGeneratorResponse::Get
variant directly correspond to the 3-argument Get call in python?generator_send
in externs.rswitty-crayon-22786
06/24/2019, 6:36 PMhundreds-breakfast-49010
06/24/2019, 6:42 PMwitty-crayon-22786
06/24/2019, 6:43 PMhundreds-breakfast-49010
06/24/2019, 6:43 PMwitty-crayon-22786
06/24/2019, 6:43 PMhundreds-breakfast-49010
06/24/2019, 6:44 PMwitty-crayon-22786
06/24/2019, 6:44 PM./pants --no-v1 --v2 test $something-that-is-not-a-python-test
hundreds-breakfast-49010
06/24/2019, 6:45 PMwitty-crayon-22786
06/24/2019, 6:45 PMhundreds-breakfast-49010
06/24/2019, 6:45 PMwitty-crayon-22786
06/24/2019, 6:45 PM./pants --no-v1 --v2 test 3rdparty/jvm/com/fasterxml/jackson/module:scala
hundreds-breakfast-49010
06/24/2019, 6:46 PMwitty-crayon-22786
06/24/2019, 6:46 PMhundreds-breakfast-49010
06/24/2019, 6:47 PMjar_library
function and that has a name and that's where 'scala' comes fromwitty-crayon-22786
06/24/2019, 6:47 PM./pants --no-v1 --v2 test examples/src/java/org/pantsbuild/example/hello/simple
jvm_binary
, which is definitely not a member of the test target unionException: WithDeps(Inner(InnerEntry { params: {Address, OptionsBootstrapper}, rule: Task(Task { product: TestResult, clause: [Select { product: HydratedTarget }], gets: [Get { product: TestResult, subject: PythonTestsAdaptor }], func: coordinator_of_tests(), cacheable: true }) })) did not declare a dependency on JustGet(Get { product: TestResult, subject: JvmBinaryAdaptor })
JvmBinaryAdaptor
TestTarget
union: https://github.com/pantsbuild/pants/blob/2b11b72e4a51d3bd84ef8f4d14463137b390e204/src/python/pants/rules/core/core_test_model.py#L24-L29hundreds-breakfast-49010
06/24/2019, 7:01 PMException: WithDeps(Inner(InnerEntry { params: {Address, OptionsBootstrapper}, rule: Task(Task { product: TestResult, clause: [Select { product: HydratedTarget }], gets: [Get { product: TestResult, subject
: PythonTestsAdaptor }], func: coordinator_of_tests(), cacheable: true }) })) did not declare a dependency on JustGet(Get { product: TestResult, subject: TargetAdaptor })
witty-crayon-22786
06/24/2019, 7:10 PM./pants --no-v1 --v2 test examples/src/java/org/pantsbuild/example/hello/simple
hundreds-breakfast-49010
06/24/2019, 8:20 PMget_type_for
expects an argument of type Value
, whihc seems to be a wrapper around a c void pointerget_union_for
dependency_key
, and that would give me back an Option<TypeId>witty-crayon-22786
06/24/2019, 8:34 PMGet
hundreds-breakfast-49010
06/24/2019, 8:48 PMsrc/nodes.rs
thoughdependency_key
is a Get
but that's not the same Get
as that variant of PyGeneratorResponse
, right?dependency_key
, I'm not sure why that would entail changing how PyGeneratorResponse::Get
workswitty-crayon-22786
06/24/2019, 9:06 PMhundreds-breakfast-49010
06/25/2019, 5:26 PMTypeId
on the rust side, and gets back the python object corresponding to the class so I can check for the '__is_union' attribute on it_extern_decl
decorator to say that the return type of that function is a rust-level Option<TypeId>
witty-crayon-22786
06/25/2019, 6:50 PMdecl
is a c-level decl, not rustcffi
, which is not rust awarecbindgen
runs before that code, and whether the types it generates would be usable in that positionaloof-angle-91616
06/25/2019, 6:53 PM@_extern_decl
thing is not supposed to be anything other than better than beforewitty-crayon-22786
06/25/2019, 6:54 PMhundreds-breakfast-49010
06/25/2019, 6:54 PMOption<TypeId>
into rustaloof-angle-91616
06/25/2019, 6:55 PMrust-bindgen
(or just bindgen
?) is the crate that generates rust methods from a C header -- i think cbindgen
is the inversecbindgen
running first, then the @_extern_decl
code is built including the header file from cbindgen
witty-crayon-22786
06/25/2019, 6:57 PMenum
on the rust side, and then seeing whether you can use its cbindgen
type in that positionaloof-angle-91616
06/25/2019, 7:00 PM@_extern_decl
working in the first placehundreds-breakfast-49010
06/25/2019, 7:00 PMaloof-angle-91616
06/25/2019, 7:00 PMhundreds-breakfast-49010
06/25/2019, 7:00 PMPyGeneratorResponse
, right?PyGeneratorResponse
isn't paramaterizedaloof-angle-91616
06/25/2019, 7:01 PMOption<T>
Option<u8>
or something that should have a specific set of C code it can correspond to i thinkwitty-crayon-22786
06/25/2019, 7:05 PMi don't think we'd be able to (easily) have something return a literalthe confusing thing though is thatOption<T>
cbindgen
does such a good job with "`enum`s in general"... why would Option not be accounted for?aloof-angle-91616
06/25/2019, 7:05 PMOption<u8>
for a given T
would be very within scopewitty-crayon-22786
06/25/2019, 7:05 PMaloof-angle-91616
06/25/2019, 7:05 PMwitty-crayon-22786
06/25/2019, 7:05 PMaloof-angle-91616
06/25/2019, 7:05 PMwitty-crayon-22786
06/25/2019, 7:06 PMaverage-vr-56795
06/25/2019, 7:06 PMOption
is that Option
doesn’t have a repr
, so doesn’t have a stable representationaloof-angle-91616
06/25/2019, 7:06 PMPyGeneratorResponse
not being parameterized, and i was saying that in the context of thinking it might be a blocker to making PyGeneratorResponse
parameterizableaverage-vr-56795
06/25/2019, 7:07 PMhundreds-breakfast-49010
06/25/2019, 7:07 PMwitty-crayon-22786
06/25/2019, 7:07 PMaloof-angle-91616
06/25/2019, 7:07 PMwitty-crayon-22786
06/25/2019, 7:07 PMrepr(c)
, basicallyrepr(c)
would workhundreds-breakfast-49010
06/25/2019, 7:08 PMwitty-crayon-22786
06/25/2019, 7:08 PMaverage-vr-56795
06/25/2019, 7:08 PMhundreds-breakfast-49010
06/25/2019, 7:08 PMwitty-crayon-22786
06/25/2019, 7:08 PMrepr(c)
bit is the overriding factor.repr(c)
you disable a lot of that.aloof-angle-91616
06/25/2019, 7:09 PM#[repr(C)]
, right??)@_extern_decl
magic, can ping this slack for any qswitty-crayon-22786
06/25/2019, 7:34 PMhundreds-breakfast-49010
06/25/2019, 7:47 PMenum OptionalType { None, Some(TypeId) }
can have a #[repr(C)]
?witty-crayon-22786
06/25/2019, 8:28 PMExternOption
or somethinghundreds-breakfast-49010
06/25/2019, 11:35 PM--- stderr
Traceback (most recent call last):
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/cparser.py", line 276, in _parse
ast = _get_parser().parse(fullcsource)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/c_parser.py", line 152, in parse
debug=debuglevel)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/ply/yacc.py", line 331, in parse
return self.parseopt_notrack(input, lexer, debug, tracking, tokenfunc)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/ply/yacc.py", line 1199, in parseopt_notrack
tok = call_errorfunc(self.errorfunc, errtoken, self)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/ply/yacc.py", line 193, in call_errorfunc
r = errorfunc(token)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/c_parser.py", line 1848, in p_error
column=self.clex.find_tok_column(p)))
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/pycparser/plyparser.py", line 67, in _parse_error
raise ParseError("%s: %s" % (coord, msg))
pycparser.plyparser.ParseError: <cdef source string>:3:18: before: extern_get_union_for
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
File "/home/gregs/code/pants/src/python/pants/engine/native.py", line 148, in bootstrap_c_source
ffibuilder.cdef(_FFISpecification.format_cffi_externs())
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/api.py", line 107, in cdef
self._cdef(csource, override=override, packed=packed)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/api.py", line 121, in _cdef
self._parser.parse(csource, override=override, **options)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/cparser.py", line 315, in parse
self._internal_parse(csource)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/cparser.py", line 320, in _internal_parse
ast, macros, csource = self._parse(csource)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/cparser.py", line 278, in _parse
self.convert_pycparser_error(e, csource)
File "/home/gregs/code/pants/build-support/pants_dev_deps.py37.venv/lib/python3.7/site-packages/cffi/cparser.py", line 307, in convert_pycparser_error
raise CDefError(msg)
cffi.error.CDefError: cannot parse "OptionalTypeId extern_get_union_for(ExternContext*, Handle*);"
<cdef source string>:3:18: before: extern_get_union_for
Execution of "/home/gregs/code/pants/build-support/bin/native/bootstrap_cffi.sh" failed with exit code Some(1)
OptionalTypeId
was?aloof-angle-91616
06/25/2019, 11:37 PMls src/rust/engine/src/cffi/
scheduler.h
is i think generated from cbindgen and the native_engine.c
is something we generate from @_extern_decl
i believehundreds-breakfast-49010
06/25/2019, 11:42 PMaloof-angle-91616
06/25/2019, 11:43 PMhundreds-breakfast-49010
06/25/2019, 11:43 PM./build-support/bin/native/cargo build --manifest-path src/rust/engine/process_execution/Cargo.toml
doesn't seem to add the string OptionalTypeId
to any of those filesaloof-angle-91616
06/25/2019, 11:44 PMOptionalTypeId
coming from?hundreds-breakfast-49010
06/25/2019, 11:45 PMPyGeneratorResponse
aloof-angle-91616
06/25/2019, 11:49 PM./pants -V
in case it's doing some invalidation that the raw cargo build command isn'thundreds-breakfast-49010
06/25/2019, 11:49 PMaloof-angle-91616
06/25/2019, 11:49 PM./pants --version
hundreds-breakfast-49010
06/25/2019, 11:51 PMOptionalTypeId
show up therealoof-angle-91616
06/25/2019, 11:55 PMNoTypeId
case contain a value? it's possible the cbindgen settings require a flag for that to be on. are you able to edit any of the other structs in the file and see the change in output?#[no_mangle] pub extern fn
to go along with it to be visiblehundreds-breakfast-49010
06/25/2019, 11:57 PMaloof-angle-91616
06/25/2019, 11:59 PMgit clean -xfd src/ && rm -rf ~/.cache/pants/bin/native-engine
MODE=debug
hundreds-breakfast-49010
06/26/2019, 12:01 AMscheduler.h
which still lacks my new typealoof-angle-91616
06/26/2019, 12:06 AMhundreds-breakfast-49010
06/26/2019, 12:07 AMGetUnionForExtern
I addedGetTypeForExtern
which is definedin basically the same way right next to it in the file<http://externs.rs|externs.rs>
I meanaloof-angle-91616
06/26/2019, 12:08 AMscheduler.h
before and after your changes might be helpful<http://lib.rs|lib.rs>
(so just add your types to that use
at the top of src/rust/engine/src/lib.rs
?)#[no_mangle]
pub extern "C" fn scheduler_create(
hundreds-breakfast-49010
06/26/2019, 12:13 AMexterns_set
aloof-angle-91616
06/26/2019, 12:14 AMhundreds-breakfast-49010
06/26/2019, 12:14 AMaloof-angle-91616
06/26/2019, 12:16 AMhundreds-breakfast-49010
06/26/2019, 12:18 AMaloof-angle-91616
06/26/2019, 12:19 AMhundreds-breakfast-49010
06/26/2019, 12:25 AMaloof-angle-91616
06/26/2019, 12:26 AMaverage-vr-56795
06/26/2019, 1:41 PMthere’s a set of things you need to clearYou shouldn’t need to do this, ever. Any time you do, please find out why and fix it. @aloof-angle-91616
hundreds-breakfast-49010
06/26/2019, 7:30 PMFatal Python error: ffi.from_handle() detected that the address passed points to garbage. If it is really the result of ffi.new_handle(), then the Python object has already been garbage collected
Current thread 0x00007dca243f6700 (most recent call first):
File "/home/gregs/code/pants/src/python/pants/engine/native.py", line 293 in extern_get_union_for
extern_get_union_for
function in the same way as other python extern functionscontext_handle
is being provided by the same with_externs
helper that every other rust function in <http://externs.rs|externs.rs>
is usingaloof-angle-91616
06/26/2019, 7:37 PMYou shouldn’t need to do this, ever. Any time you do, please find out why and fix it.any time i try to describe RTB we really need to be doing, including specifically with respect to the rust build, it gets shot down, so i get to do it on my own time. i did say specifically it was a huge hack in order to avoid creating the impression that we think this is ok.
from_handle()
call in that method?Handle
hundreds-breakfast-49010
06/26/2019, 7:42 PMaloof-angle-91616
06/26/2019, 7:42 PMhundreds-breakfast-49010
06/26/2019, 7:43 PMextern_type_to_str
aloof-angle-91616
06/26/2019, 7:45 PMhundreds-breakfast-49010
06/26/2019, 7:50 PMaloof-angle-91616
06/26/2019, 7:50 PMhundreds-breakfast-49010
06/26/2019, 7:51 PMaloof-angle-91616
06/26/2019, 8:15 PMhundreds-breakfast-49010
06/26/2019, 8:15 PMaloof-angle-91616
06/26/2019, 8:16 PMhundreds-breakfast-49010
06/26/2019, 8:46 PMaloof-angle-91616
06/26/2019, 8:46 PMeprintln!()
to gen_get
rngen_get()
): let declared_type_union = get.declared_subject
.and_then(|ty| {
let t = externs::type_to_str(ty);
eprintln!("this type??? {:?}", t);
externs::get_union_for(ty)
});
eprintln!("declared: {:?}", declared_type_union);
this type??? "TestTarget"
`
then the same fatal python error immediately afterhundreds-breakfast-49010
06/26/2019, 8:48 PMaloof-angle-91616
06/26/2019, 8:48 PMhundreds-breakfast-49010
06/26/2019, 8:48 PMaloof-angle-91616
06/26/2019, 8:49 PMwith_externs_exclusive
, i'll try that next although i don't think that's it). it might also be because (and this would be ridiculous) the cffi-generated C in native_engine.c
has this conditional based on sizeof()
which might be differentc = self._ffi.from_handle(context_handle)
hundreds-breakfast-49010
06/26/2019, 8:52 PMaloof-angle-91616
06/26/2019, 8:53 PMwith_externs_exclusive()
method and since we happen to have a bad context maybe a write lock would helphundreds-breakfast-49010
06/26/2019, 8:53 PMaloof-angle-91616
06/26/2019, 8:54 PMsrc/rust/engine/src/cffi/native_engine.c
, there's: static struct _cffi_externpy_s _cffi_externpy__extern_get_type_for =
{ "native_engine.extern_get_type_for", (int)sizeof(TypeId) };
static TypeId extern_get_type_for(void * a0, void * * a1)
{
char a[sizeof(TypeId) > 16 ? sizeof(TypeId) : 16];
char *p = a;
*(void * *)(p + 0) = a0;
*(void * * *)(p + 8) = a1;
_cffi_call_python(&_cffi_externpy__extern_get_type_for, p);
return *(TypeId *)p;
}
static struct _cffi_externpy_s _cffi_externpy__extern_get_union_for =
{ "native_engine.extern_get_union_for", (int)sizeof(OptionalTypeId) };
static OptionalTypeId extern_get_union_for(void * a0, TypeId a1)
{
char a[sizeof(OptionalTypeId) > 16 ? sizeof(OptionalTypeId) : 16];
char *p = a;
*(void * *)(p + 0) = a0;
*(TypeId * *)(p + 8) = &a1;
_cffi_call_python(&_cffi_externpy__extern_get_union_for, p);
return *(OptionalTypeId *)p;
}
sizeof(OptionalTypeId)
happens to be greater than 16, or if that's different compared to TypeId
hundreds-breakfast-49010
06/26/2019, 9:09 PMaloof-angle-91616
06/26/2019, 9:09 PMcontext_handle
before calling .from_handle()
for type_to_str
and the subsequent get_union_for
, and it is different<http://logger.info|logger.info>('h: {}'.format(context_handle))
to the top of extern_get_union_for
(3) adding <http://logger.info|logger.info>('i: {}'.format(context_handle))
to the top of extern_type_to_str
(4) running RUST_BACKTRACE=full ./pants --no-v1 --v2 test examples/src/java/org/pantsbuild/example/hello/simple
(5) i see 21:16:15 [INFO] i: <cdata 'void *' 0x110d02728>
21:16:15 [INFO] Starting tests: examples/src/java/org/pantsbuild/example/hello/simple
21:16:15 [INFO] i: <cdata 'void *' 0x110d02728>
this type??? "TestTarget"
21:16:15 [INFO] h: <cdata 'void *' 0x70000dcb9d58>
Fatal Python error: ffi.from_handle() detected that the address passed points to garbage. If it is really the result of ffi.new_handle(), then the Python object has already been garbage collected
Thread 0x000070000d4b4000 (most recent call first):
Current thread 0x000070000dcc0000 (most recent call first):
File "/Users/dmcclanahan/tools/pants-v3/src/python/pants/engine/native.py", line 294 in extern_get_union_for
(6) turning the self._ffi.from_handle()
to use .new_handle()
fails with illegal hardware instruction
(7) mainly, i do not understand why the context object would be different when passed to extern_get_union_for
(as shown in the above output) immediately after being passed to extern_type_to_str
externs::get_union_for
whatsoeverhundreds-breakfast-49010
06/26/2019, 9:28 PMget_union_for
is the new function that I'm adding with this commit, meant to be used right here