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

Fix a typo while gathering dependencies from Manifest.toml #593

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

zhubonan
Copy link
Contributor

I got this error while using the 1.7.1 release:

ERROR: type Dict has no field deps
Stacktrace:
 [1] getproperty(x::Dict{Base.UUID, Pkg.Types.PackageEntry}, f::Symbol)
   @ Base ./Base.jl:33
 [2] create_sysimage(packages::Vector{String}; sysimage_path::String, project::String, precompile_execution_file::Vector{String}, precompile_statements_file::Vector{String}, incremental::Bool, filter_stdlibs::Bool, replace_default::Bool, cpu_target::String, script::Nothing, sysimage_build_args::Cmd, include_transitive_dependencies::Bool, base_sysimage::Nothing, isapp::Bool, julia_init_c_file::Nothing, version::Nothing, compat_level::String, soname::Nothing)
   @ PackageCompiler ~/.julia/packages/PackageCompiler/PmmEa/src/PackageCompiler.jl:582

This was probably due to a type to typo as ctx.env.manifest is a Dict.

Perhaps the tests should also be updated?

@zhubonan
Copy link
Contributor Author

Not sure how to add a test for this.
Maybe it was uncaught because Example package used does not have any dependencies itself?

@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #593 (e24d498) into master (1d1b2a2) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #593   +/-   ##
=======================================
  Coverage   84.33%   84.33%           
=======================================
  Files           2        2           
  Lines         498      498           
=======================================
  Hits          420      420           
  Misses         78       78           
Impacted Files Coverage Δ
src/PackageCompiler.jl 83.73% <100.00%> (ø)

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 1d1b2a2...e24d498. Read the comment docs.

@KristofferC
Copy link
Member

KristofferC commented Oct 11, 2021

This was probably due to a type to typo as ctx.env.manifest is a Dict.

This is only true on 1.6.1. In 1.6.2 it is of type Pkg.Types.Manifest. However, this PR is also compatible with 1.6.2+ because of

https://github.com/JuliaLang/Pkg.jl/blob/e68aadf6e175228caa512e136caf7f9a4688c3a7/src/Types.jl#L270

So this can be merged and will fix the package on 1.6.1.

And to be explicit, the reason we are not seeing this in CI is that we are testing on 1.6.3.

@KristofferC KristofferC merged commit 1742e6a into JuliaLang:master Oct 11, 2021
@zhubonan
Copy link
Contributor Author

Oh, I see. Thanks for the explanation!
Forgot to mention I was on 1.6.1.

zhubonan added a commit to zhubonan/PackageCompiler.jl that referenced this pull request Oct 12, 2021
[Diff since v1.7.1](JuliaLang/PackageCompiler.jl@v1.7.1...v1.7.2)

**Closed issues:**
- System image compiled with 1.7 segfaults. (JuliaLang#588)
- ENAMETOOLONG regression with v1.7.1 (JuliaLang#592)

**Merged pull requests:**
- Fix a typo while gathering dependencies from Manifest.toml (JuliaLang#593) (@zhubonan)
- use a file to run the sysimage code in (JuliaLang#594) (@KristofferC)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants