use type declaration for globals on 1.9+ #46

KristofferC · 2022-09-28T10:53:18Z

Fixes #39

giordano · 2022-09-28T10:57:25Z

Having to change the tests is a good indication this would be a breaking change, no? In #14 we already had to revert similar changes for this reason.

KristofferC · 2022-09-28T11:03:36Z

Having to change the tests is a good indication this would be a breaking change, no?

Well, that depends on what the test is testing. After all, we change tests in Julia all the time. :).

If you can point me to the docs where it says the public API for jlls, we can check. If you want, I can take away the PATH and LIBPATH changes which is what the tests happen to use.

Also, ironically, the tests actually used to be written like this: 32c80b6

giordano · 2022-09-28T11:07:02Z

Also, ironically, the tests actually used to be written like this: 32c80b6

Which tried to fix tests after #12, partially reverted in #14 😉

KristofferC · 2022-09-28T11:10:37Z

LIBPATH[] seems to be used: https://github.com/JuliaLang/julia/blob/6aaedecc447e3d8226d5027fb13d0c3cbfbfea2a/stdlib/LibUnwind_jll/src/LibUnwind_jll.jl#L29. Let's get rid of that change then.

Lesson learned, function interfaces are better :P

giordano · 2022-09-28T11:12:46Z

Lesson learned function interfaces are better :P

Yeah, we learnt this the hard way here. Elliot would like to make everything functions, but we still need to keep old variables around.

KristofferC · 2022-09-28T12:07:59Z

src/toplevel_generators.jl

@@ -197,6 +203,7 @@ macro generate_main_file_header(src_name)
        generate_compiler_options(src_name),
        # import Artifacts module
        generate_imports(src_name),
+        global_typeassert_available ? :(global artifact_dir::String) : :()


This is here just so the global artifact_dir call in the __init__ function will have a known type.

KristofferC · 2022-09-30T07:08:54Z

I believe this should be non-breaking now.

giordano · 2022-10-03T18:42:34Z

Failures on nightly might be genuine? I had restarted the jobs but they're still failing, while #47 is not failing.

KristofferC · 2022-10-03T20:22:55Z

I'm pretty sure I tried this locally on Linux but will check again (tomorrow).

codecov · 2022-10-19T07:15:48Z

Codecov Report

Merging #46 (5d09258) into master (9ae1b7c) will decrease coverage by 0.99%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   55.04%   54.05%   -1.00%     
==========================================
  Files           7        7              
  Lines         218      222       +4     
==========================================
  Hits          120      120              
- Misses         98      102       +4

Impacted Files	Coverage Δ
src/JLLWrappers.jl	`90.00% <ø> (ø)`
src/products/executable_generators.jl	`60.00% <0.00%> (-4.29%)`	⬇️
src/products/file_generators.jl	`92.30% <ø> (ø)`
src/products/library_generators.jl	`80.00% <ø> (ø)`
src/toplevel_generators.jl	`31.25% <0.00%> (-0.81%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

KristofferC · 2022-10-19T07:20:49Z

Seems ok now, or?

giordano · 2022-10-19T08:08:29Z

Segfault on aarch64 with 1.8 (so relevant for this PR) looks reproducible (restarted the job twice):

        Info Packages marked with ⌅ have new versions available but compatibility constraints restrict them from upgrading. To see why use `status --outdated -m`
┌ Warning: The MPI abi has changed, you will need to restart Julia for the change to take effect
│   abi = :MPItrampoline
└ @ LAMMPS_jll /tmp/cirrus-ci-build/test/LAMMPS_jll/src/LAMMPS_jll.jl:27
 Downloading artifact: LAMMPS
signal (11): Segmentation fault
in expression starting at none:2
jl_field_offset at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/julia.h:1133 [inlined]
set_nth_field at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/datatype.c:1486
jl_f_tuple at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/builtins.c:837 [inlined]
jl_f_tuple at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/builtins.c:825
jl_method_error at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:1895
jl_lookup_generic_ at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2530 [inlined]
ijl_apply_generic at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2545
jl_apply at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/julia.h:1839 [inlined]
do_call at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/interpreter.c:126
eval_value at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/interpreter.c:215
eval_stmt_value at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/interpreter.c:166 [inlined]
eval_body at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/interpreter.c:612
jl_fptr_interpret_call at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/interpreter.c:686
_jl_invoke at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2549
jl_apply at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/julia.h:1839 [inlined]
jl_module_run_initializer at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:75
ijl_init_restored_modules at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/dump.c:2602
_include_from_serialized at ./loading.jl:831
_tryrequire_from_serialized at ./loading.jl:978
_require at ./loading.jl:1347
_require_prelocked at ./loading.jl:1200
macro expansion at ./loading.jl:1180 [inlined]
macro expansion at ./lock.jl:223 [inlined]
require at ./loading.jl:1144
jfptr_require_42793 at /root/julia/lib/julia/sys.so (unknown line)
_jl_invoke at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2549
jl_apply at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/julia.h:1839 [inlined]
call_require at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:462 [inlined]
eval_import_path at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:499
jl_toplevel_eval_flex at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:725
jl_toplevel_eval_flex at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:850
ijl_toplevel_eval_in at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/toplevel.c:965
eval at ./boot.jl:368 [inlined]
exec_options at ./client.jl:276
_start at ./client.jl:522
jfptr__start_49479 at /root/julia/lib/julia/sys.so (unknown line)
_jl_invoke at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2367 [inlined]
ijl_apply_generic at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/gf.c:2549
jl_apply at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/julia.h:1839 [inlined]
true_main at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/jlapi.c:575
jl_repl_entrypoint at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/src/jlapi.c:719
main at /cache/build/default-armageddon-0/julialang/julia-release-1-dot-8/cli/loader_exe.c:59
unknown function (ip: 0xffffbb6373fb)
__libc_start_main at /lib/aarch64-linux-gnu/libc.so.6 (unknown line)
_start at /root/julia/bin/julia (unknown line)
_start at /root/julia/bin/julia (unknown line)
Allocations: 1292899 (Pool: 1290756; Big: 2143); GC: 2
JLLWrappers.jl: Test Failed at /tmp/cirrus-ci-build/test/runtests.jl:123
  Expression: success(pipeline(`$(Base.julia_cmd()) --project=$dir -e $script`, stderr = stderr, stdout = stdout))
Stacktrace:
 [1] macro expansion
   @ ~/julia/share/julia/stdlib/v1.8/Test/src/Test.jl:464 [inlined]
 [2] (::var"#1#10")(dir::String)
   @ Main /tmp/cirrus-ci-build/test/runtests.jl:123
Test Summary:  | Pass  Fail  Total   Time
JLLWrappers.jl |   38     1     39  25.4s

😢

KristofferC · 2022-10-19T08:11:39Z

But it passes on nightly (where it is also active)...?

KristofferC · 2022-10-19T08:55:44Z

Let's go 1.9+ then if ARM is buggy on 1.8.

KristofferC force-pushed the kc/typeassert_globals_18 branch from fab932b to be826e5 Compare September 28, 2022 11:00

KristofferC force-pushed the kc/typeassert_globals_18 branch from be826e5 to 1184273 Compare September 28, 2022 11:08

KristofferC commented Sep 28, 2022

View reviewed changes

use type declaration for globals on 1.8+

7b0da8a

KristofferC force-pushed the kc/typeassert_globals_18 branch from c3d2651 to 7b0da8a Compare September 30, 2022 07:08

Merge branch 'master' into kc/typeassert_globals_18

9c27452

test this

5d09258

giordano changed the title ~~use type declaration for globals on 1.8+~~ use type declaration for globals on 1.9+ Oct 19, 2022

giordano merged commit f6f6abe into master Oct 19, 2022

giordano deleted the kc/typeassert_globals_18 branch October 19, 2022 22:38

KristofferC mentioned this pull request Nov 29, 2022

Add type annotations to global variables in LLVMLibUnwind_jll JuliaLang/julia#47731

Closed

contra-bit pushed a commit to contra-bit/JLLWrappers.jl that referenced this pull request Jul 10, 2024

use type declaration for globals on 1.9+ (JuliaPackaging#46)

86a8939

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use type declaration for globals on 1.9+ #46

use type declaration for globals on 1.9+ #46

KristofferC commented Sep 28, 2022

giordano commented Sep 28, 2022

KristofferC commented Sep 28, 2022 •

edited

Loading

giordano commented Sep 28, 2022

KristofferC commented Sep 28, 2022 •

edited

Loading

giordano commented Sep 28, 2022

KristofferC Sep 28, 2022

KristofferC commented Sep 30, 2022

giordano commented Oct 3, 2022

KristofferC commented Oct 3, 2022

codecov bot commented Oct 19, 2022 •

edited

Loading

KristofferC commented Oct 19, 2022

giordano commented Oct 19, 2022 •

edited

Loading

KristofferC commented Oct 19, 2022 •

edited

Loading

KristofferC commented Oct 19, 2022

use type declaration for globals on 1.9+ #46

use type declaration for globals on 1.9+ #46

Conversation

KristofferC commented Sep 28, 2022

giordano commented Sep 28, 2022

KristofferC commented Sep 28, 2022 • edited Loading

giordano commented Sep 28, 2022

KristofferC commented Sep 28, 2022 • edited Loading

giordano commented Sep 28, 2022

KristofferC Sep 28, 2022

Choose a reason for hiding this comment

KristofferC commented Sep 30, 2022

giordano commented Oct 3, 2022

KristofferC commented Oct 3, 2022

codecov bot commented Oct 19, 2022 • edited Loading

Codecov Report

KristofferC commented Oct 19, 2022

giordano commented Oct 19, 2022 • edited Loading

KristofferC commented Oct 19, 2022 • edited Loading

KristofferC commented Oct 19, 2022

KristofferC commented Sep 28, 2022 •

edited

Loading

KristofferC commented Sep 28, 2022 •

edited

Loading

codecov bot commented Oct 19, 2022 •

edited

Loading

giordano commented Oct 19, 2022 •

edited

Loading

KristofferC commented Oct 19, 2022 •

edited

Loading