Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsure how to handle JLL dependencies #484

Closed
Octogonapus opened this issue Apr 10, 2024 · 7 comments · Fixed by #485
Closed

Unsure how to handle JLL dependencies #484

Octogonapus opened this issue Apr 10, 2024 · 7 comments · Fixed by #485
Labels

Comments

@Octogonapus
Copy link
Contributor

I'm working on generating bindings for a family of JLLs (all the AWS CRT ones). Some of them depend on each other. For example, aws_c_io_jll depends on aws_c_common_jll. In order to get Clang.jl to generate the bindings for aws_c_io_jll, I need to include both it and its dependent JLLs:

for target in JLLEnvs.JLL_ENV_TRIPLES
    options = load_options(joinpath(@__DIR__, "generator.toml"))
    options["general"]["output_file_path"] = joinpath(@__DIR__, "..", "lib", "$target.jl")

    header_dirs = []
    args = get_default_args(target)
    inc = JLLEnvs.get_pkg_include_dir(aws_c_common_jll, target)
    push!(args, "-I$inc")
    inc = JLLEnvs.get_pkg_include_dir(aws_c_io_jll, target)
    push!(args, "-I$inc")
    push!(header_dirs, inc)

    headers = String[]
    for header_dir in header_dirs
        for (root, dirs, files) in walkdir(header_dir)
            for file in files
                if endswith(file, ".h")
                    push!(headers, joinpath(root, file))
                end
            end
        end
    end
    unique!(headers)

    ctx = create_context(headers, args, options)
    build!(ctx)
end

This makes sense from the perspective of Clang.jl needing to fully resolve the DAG to emit valid code, but it comes with a problem: the bindings for aws_c_common_jll are included in the output. We're trying to create a composable set of bindings packages, so we'd like to find a way to output only the bindings for aws_c_io_jll and include the bindings for the dependencies via using LibAwsCommon in the generated code (another bindings package we have).

To my understanding, this is not a feature Clang.jl currently has. Please do correct me if I'm wrong there. I've explored Clang.jl's implementation a bit but it is not clear to me how I could implement this. Do you have any ideas on where I could start?

cc @quinnj

@Octogonapus
Copy link
Contributor Author

One thought I have is maybe inside should_exclude_node I can use get_filename(node.cursor) to query against a new list of files which should be included in the output. That would let me emit code only from the header files of a given JLL. This list could be specified in the options like the other two. What do you think of that design?

@Gnimuc
Copy link
Member

Gnimuc commented Apr 11, 2024

The -isystem-trick should work. Clang.jl's generator doesn't emit code for system headers. Although those aws headers are not real system headers, you can use the compiler flag to force clang to treat them as system headers.

inc = JLLEnvs.get_pkg_include_dir(aws_c_common_jll, target)
push!(args, "-isystem$inc")

@Octogonapus
Copy link
Contributor Author

With that change, code for aws_c_common_jll is still emitted. For example, the struct definition of aws_allocator is emitted. I checked the header files, and it's only defined in aws_c_common_jll and not defined in aws_c_io_jll.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 11, 2024

Could you provide an MWE?

@Octogonapus
Copy link
Contributor Author

Here is an MWE: https://github.com/Octogonapus/LibAwsIO.jl/tree/clang_mwe

To reproduce, you can run ./gen/generate.sh. from the root of the repo.

@Gnimuc
Copy link
Member

Gnimuc commented Apr 11, 2024

well.. you're right. it only warns a list of symbols in the system headers.

those system symbols are in ctx.passes[5].dependents, so you can filter out them before running the printer pass.

┌ Warning: [CollectDependentSystemNode]: found symbols in the system headers: [:aws_priority_queue, :aws_thread_options, :aws_task_fn, :__darwin_pthread_t, :aws_linked_list_node, :aws_allocator, :pthread_cond_t, :aws_atomic_var, :aws_array_list, :aws_array_list, :aws_shutdown_callback_options, :aws_array_list, :aws_linked_list, :aws_byte_buf, :aws_byte_cursor, :aws_allocator, :aws_priority_queue_compare_fn, :_opaque_pthread_mutex_t, :aws_linked_list, :__darwin_pthread_mutex_t, :aws_byte_buf, :__darwin_pthread_t, :pthread_t, :_opaque_pthread_t, :_opaque_pthread_cond_t, :aws_task_fn, :pthread_cond_t, :hash_table_state, :aws_byte_buf, :aws_array_list, :aws_allocator, :aws_byte_buf, :aws_string, :_opaque_pthread_t, :__darwin_pthread_t, :aws_priority_queue_compare_fn, :aws_byte_buf, :aws_shutdown_callback_options, :aws_byte_buf, :aws_string, :aws_thread_detach_state, :__darwin_pthread_cond_t, :_opaque_pthread_cond_t, :aws_linked_list_node, :_opaque_pthread_mutex_t, :pthread_t, :__darwin_pthread_cond_t, :aws_linked_list_node, :__darwin_pthread_t, :pthread_t, :__darwin_pthread_handler_rec, :aws_array_list, :_opaque_pthread_cond_t, :__darwin_pthread_handler_rec, :aws_byte_cursor, :__darwin_pthread_mutex_t, :aws_priority_queue_node, :__darwin_pthread_t, :aws_byte_cursor, :aws_linked_list, :_opaque_pthread_cond_t, :aws_byte_cursor, :aws_simple_completion_callback, :aws_linked_list, :__darwin_pthread_cond_t, :pthread_mutex_t, :_opaque_pthread_cond_t, :aws_linked_list_node, :aws_task, :__darwin_pthread_t, :__darwin_pthread_handler_rec, :aws_array_list, :aws_byte_buf, :aws_byte_buf, :aws_mutex, :aws_thread, :__darwin_pthread_handler_rec, :__darwin_pthread_t, :aws_thread_options, :__darwin_pthread_handler_rec, :aws_condition_variable, :aws_task_scheduler, :__darwin_pthread_mutex_t, :__darwin_pthread_cond_t, :aws_array_list, :pthread_cond_t, :aws_priority_queue, :pthread_mutex_t, :_opaque_pthread_mutex_t, :_opaque_pthread_t, :pthread_cond_t, :__darwin_pthread_cond_t, :aws_allocator, :__darwin_pthread_mutex_t, :aws_task_scheduler, :pthread_t, :__darwin_pthread_handler_rec, :aws_mutex, :pthread_mutex_t, :aws_byte_buf, :aws_array_list, :pthread_t, :__darwin_pthread_cond_t, :_opaque_pthread_t, :__darwin_pthread_mutex_t, :_opaque_pthread_t, :aws_simple_completion_callback, :_opaque_pthread_mutex_t, :pthread_mutex_t, :aws_array_list, :pthread_mutex_t, :pthread_mutex_t, :aws_allocator, :_opaque_pthread_cond_t, :__darwin_pthread_handler_rec, :__darwin_pthread_mutex_t, :aws_array_list, :_opaque_pthread_t, :aws_array_list, :_opaque_pthread_mutex_t, :aws_linked_list, :aws_byte_cursor, :aws_linked_list, :pthread_mutex_t, :_opaque_pthread_cond_t, :_opaque_pthread_cond_t, :aws_linked_list, :_opaque_pthread_t, :aws_linked_list_node, :pthread_cond_t, :aws_atomic_var, :pthread_t, :aws_task, :pthread_mutex_t, :aws_array_list, :aws_byte_cursor, :aws_linked_list_node, :aws_crt_statistics_category_t, :aws_ref_count, :_opaque_pthread_cond_t, :aws_linked_list, :__darwin_pthread_cond_t, :_opaque_pthread_mutex_t, :aws_byte_cursor, :pthread_t, :_opaque_pthread_mutex_t, :pthread_mutex_t, :pthread_cond_t, :aws_simple_completion_callback, :_opaque_pthread_t, :pthread_mutex_t, :_opaque_pthread_mutex_t, :_opaque_pthread_t, :_opaque_pthread_cond_t, :aws_array_list, :aws_allocator, :aws_allocator, :pthread_mutex_t, :aws_linked_list, :__darwin_pthread_t, :_opaque_pthread_t, :__darwin_pthread_t, :_opaque_pthread_t, :aws_linked_list_node, :__darwin_pthread_mutex_t, :aws_allocator, :_opaque_pthread_cond_t, :aws_allocator, :pthread_t, :aws_thread_id_t, :__darwin_pthread_cond_t, :__darwin_pthread_mutex_t, :__darwin_pthread_handler_rec, :aws_byte_buf, :pthread_cond_t, :aws_linked_list, :__darwin_pthread_mutex_t, :_opaque_pthread_mutex_t, :__darwin_pthread_t, :aws_byte_cursor, :aws_byte_cursor, :aws_linked_list_node, :_opaque_pthread_cond_t, :__darwin_pthread_t, :__darwin_pthread_handler_rec, :aws_ref_count, :__darwin_pthread_cond_t, :__darwin_pthread_cond_t, :aws_task_fn, :__darwin_pthread_mutex_t, :pthread_t, :aws_mutex, :aws_thread_join_strategy, :pthread_t, :aws_linked_list_node, :__darwin_pthread_handler_rec, :aws_allocator, :pthread_cond_t, :__darwin_pthread_mutex_t, :aws_byte_buf, :__darwin_pthread_handler_rec, :aws_thread, :_opaque_pthread_cond_t, :pthread_cond_t, :aws_byte_cursor, :aws_array_list, :aws_byte_cursor, :aws_crt_statistics_category_t, :_opaque_pthread_mutex_t, :pthread_cond_t, :pthread_cond_t, :aws_priority_queue_node, :pthread_mutex_t, :_opaque_pthread_mutex_t, :aws_thread_id_t, :aws_task, :aws_linked_list_node, :aws_hash_table, :aws_crt_statistics_category_t, :aws_shutdown_callback_options, :aws_thread_join_strategy, :aws_task, :__darwin_pthread_t, :aws_byte_buf, :_opaque_pthread_t, :pthread_t, :aws_allocator, :__darwin_pthread_handler_rec, :aws_task, :__darwin_pthread_t, :aws_thread_detach_state, :__darwin_pthread_handler_rec, :aws_priority_queue_node, :aws_array_list, :aws_priority_queue_compare_fn, :aws_atomic_var, :pthread_mutex_t, :__darwin_pthread_cond_t, :aws_byte_buf, :aws_array_list, :_opaque_pthread_mutex_t, :pthread_t, :aws_linked_list, :aws_atomic_var, :pthread_cond_t, :aws_byte_cursor, :pthread_t, :_opaque_pthread_t, :aws_ref_count, :aws_priority_queue, :aws_allocator, :aws_byte_cursor, :__darwin_pthread_cond_t, :aws_task_scheduler, :__darwin_pthread_cond_t, :aws_string, :_opaque_pthread_mutex_t, :__darwin_pthread_mutex_t, :aws_allocator, :aws_linked_list_node, :aws_task, :aws_condition_variable, :pthread_cond_t, :__darwin_pthread_mutex_t]

@Gnimuc
Copy link
Member

Gnimuc commented Apr 11, 2024

we should add a flag for this feature and check it in the printer passes. (I thought I implemented it and I did tell everyone to use the -isystem trick, but apparently, I got it wrong.) 🤯

pretty_print(io, node, general_options)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants