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

Add a PackageCompilerLib plugin #299

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kmsquire
Copy link

@kmsquire kmsquire commented Jul 1, 2021

This generates files which facilitate creating C libraries from Julia code, using PackageCompiler.jl.

The MR there (JuliaLang/PackageCompiler.jl#490) actually hasn't been merged there yet, but should be very soon. I'll also be presenting this functionality at JuliaCon, and am hoping/planning to use PkgTemplates as part of this presentation.

It probably shouldn't be merged until the corresponding PackageCompiler.jl code is merged, so I'm marking it as a draft. However, I would love for it to be reviewed, if someone has time.

@codecov
Copy link

codecov bot commented Jul 1, 2021

Codecov Report

Merging #299 (ad1926a) into master (bcea5d2) will decrease coverage by 20.50%.
The diff coverage is 0.00%.

❗ Current head ad1926a differs from pull request most recent head 978af1a. Consider uploading reports for the commit 978af1a to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master     #299       +/-   ##
===========================================
- Coverage   94.42%   73.91%   -20.51%     
===========================================
  Files          20       21        +1     
  Lines         610      648       +38     
===========================================
- Hits          576      479       -97     
- Misses         34      169      +135     
Impacted Files Coverage Δ
src/PkgTemplates.jl 100.00% <ø> (ø)
src/plugin.jl 98.36% <ø> (-0.03%) ⬇️
src/plugins/package_compiler_lib.jl 0.00% <0.00%> (ø)
src/plugins/develop.jl 0.00% <0.00%> (-100.00%) ⬇️
src/plugins/register.jl 0.00% <0.00%> (-100.00%) ⬇️
src/plugins/coverage.jl 0.00% <0.00%> (-90.91%) ⬇️
src/plugins/ci.jl 18.66% <0.00%> (-80.00%) ⬇️
src/plugins/citation.jl 20.00% <0.00%> (-60.00%) ⬇️
src/plugins/project_file.jl 66.66% <0.00%> (-22.23%) ⬇️
src/plugins/tagbot.jl 88.88% <0.00%> (-11.12%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcea5d2...978af1a. Read the comment docs.

@kmsquire
Copy link
Author

kmsquire commented Jul 1, 2021

I just noticed #290, which hasn't been reviewed yet. There will be conflicts between these, which I can of course fix if that one is reviewed and merged. Is there interest in either of these?

src/plugins/package_compiler_lib.jl Show resolved Hide resolved
src/plugins/package_compiler_lib.jl Outdated Show resolved Hide resolved
src/plugins/package_compiler_lib.jl Outdated Show resolved Hide resolved
lib_name=nothing,
build_jl="$(contractuser(default_file("build", "build.jl")))",
generate_precompile_jl="$(contractuser(default_file("build", "generate_precompile.jl")))",
additional_precompile_jl="$(contractuser(default_file("build", "additional_precompile.jl")))",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case for this additional_precompile file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_precompile.jl runs code which is traced to determine which functions to precompile.
additional_precompile.jl contains explicit precompile statements for compiling versions of functions that were not captured by generate_precompile.jl

Both files exist under the templates/build directory.

Do you need/want me to add any of this description to the PR?

# glob here in the prehook, so it can be added by gitignore() later.
pkg = basename(pkg_dir)
library_name = lib_name(p, pkg)
push!(p.additional_gitignore, "/$(library_name)-*")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this workaround the main purpose of having the additional_gitignore file? It might be simpler to just force a default library name (e.g. MyLibrary).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the main purpose. Your suggestion is certainly easier--I was just trying to have a complete solution out of the box.

I can remove the additional_gitignore member if you prefer.

templates/Makefile Outdated Show resolved Hide resolved
@kmsquire kmsquire force-pushed the feature/package_compiler_library_creation branch from ad1926a to cf65bf7 Compare July 29, 2021 04:08
@kmsquire
Copy link
Author

@mjram0s @Wynand thank you for reviewing, and sorry not to get back to this sooner.

I addressed most of the comments, and left a few questions. I have one more change to push to remove the Makefile line (the moment the PackageCompiler PR is merged), plus whatever else you want me to do here.

Cheers!

* This generates files which facilitate creating C libraries from Julia
  code, using PackageCompiler.jl.
@kmsquire kmsquire force-pushed the feature/package_compiler_library_creation branch from cf65bf7 to 978af1a Compare July 29, 2021 14:11
@nickrobinson251 nickrobinson251 added the Plugin Idea A proposal for a feature that can be accomplished with a plugin label Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Plugin Idea A proposal for a feature that can be accomplished with a plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants