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

Backports for Julia 1.8.3 #46984

Merged
merged 41 commits into from
Nov 8, 2022
Merged

Backports for Julia 1.8.3 #46984

merged 41 commits into from
Nov 8, 2022

Conversation

KristofferC
Copy link
Sponsor Member

@KristofferC KristofferC commented Sep 30, 2022

Backported PRs:

Need manual backport:

Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:

@KristofferC KristofferC added the release Release management and versioning. label Sep 30, 2022
@maleadt
Copy link
Member

maleadt commented Oct 27, 2022

Added the manual back-port of #46373

@DilumAluthge DilumAluthge removed their request for review October 27, 2022 21:22
N5N3 and others added 24 commits October 28, 2022 08:20
This patch fixes the shifting direction for
circshift!(x::AbstractVector, n::Integer), which was opposite the
direction of circshift(x, n) and circshift!(y, x, n). In addition, this
patch fixes the method to also work correctly with offset arrays.
Fixes #46533.

(cherry picked from commit f1c4d54)
* Build/win: Build with MSYS2 (#46140)

* Makefile: MSYS2: close path conversion for `DEP_LIBS`

Automatic path conversion will replace `:` with `;`

* Makefile: MSYS2: use `cygpath` for path convert

ref: https://www.msys2.org/docs/filesystem-paths/#manual-unix-windows-path-conversion

* devdoc/win/msys2: add build steps

* devdoc/win/msys2: Add x86/x64 build notes

* devdoc/win/msys2: apply sugestions

Co-Authored-By: Elliot Saba <[email protected]>

* Instead of `CC_WITH_ENV`, scope environment variables to targets

* Fix whitespace

Co-authored-by: Elliot Saba <[email protected]>

* Disable MSYS2's path munging for `stringreplace` (#46803)

This was causing our rewriting of the loader's RPATH emulation to fail
after running `make install`, as MSYS2 was rewriting our list that looks
like:

```
../bin/libgcc_s_seh-1.dll:../bin/libopenlibm.dll:@../bin/libjulia-internal.dll:@../bin/libjulia-codegen.dll:
```

Into one that looked like:
```
..\bin\libgcc_s_seh-1.dll;..\bin\libopenlibm.dll;@..\bin\libjulia-internal.dll;@..\bin\libjulia-codegen.dll;
```

By exporting `MSYS2_ARG_CONV_EXCL='*'` for all `stringreplace`
invocations, we dodge this issue, as documented by MSYS2 [0].

[0] https://www.msys2.org/wiki/Porting/#filesystem-namespaces

* [win] Disable MSYS2 path munging when calling `is.exe` (#46867)

Tragically, I believe MSYS2 is messing with options such as
`/VERYSILENT` turning them instead into `C:\msys2\VERYSILENT` or
similar.

Co-authored-by: Elliot Saba <[email protected]>
…by up to one microsecond (to compensate for Windows tar) (#45552)

(cherry picked from commit 4c39647)
Without this, alignment would count characters rather than
textwidth as well as counting inline escape sequences in
colored output. Fix that by using uncolored printing for
alignment and textwidth rather than number of codepoints.

(cherry picked from commit 626acd4)
Co-authored-by: Viral B. Shah <[email protected]>
(cherry picked from commit 5811825)
Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commit 45b96c4)
Only libjulia-internal is supposed to be linked against this, since it
is a static library.

Fix #47198

(cherry picked from commit e8aacc8)
As reported [here](https://discourse.julialang.org/t/test-failures-for-sockets-base-runtests-sockets/88898).

My guess on the original issue reported is that, for some reason, the host where the tests are run
is unable to listen on any ports, so we end up cycling through the entire UInt16 range (the test
starts at port 11011), but when we fail on port 65535, we do `addr.port + 1` and instead of wrapping
around as I believe this function intends to happen (as noted by the `addr.port == default_port` check
before we error), it gets promoted to `Int(65536)` which then throws an (unexpected) error in the `InetAddr`
constructor.

I'm not quite sure how to test this exactly though, because we'd need to simulate not being able
to listen on any ports? If anyone has any ideas, I'm all ears.

(cherry picked from commit a311f4d)
When trying to dlopen a file with non-standard extension (e.g. `foo.so`
instead of `foo.dylib` when running on macOS), if this failed (e.g.
because of a reference to an undefined symbol), then instead of printing
the error message returned by `dlerror` with a helpful notice what
went wrong, a message indicating something to the effect that
"foo.so.dylib was not found" was shown, which was not helpful at all.

To get the actual helpful error message, add a check so that when dlopen
fails for a file that actually exists, we don't retry loading from a
file with the standard extension attached, which might not even exist;
instead we just give up.

This matches what is already being done for relative paths. This patch
simply copies the relevant check to also be applied to the case dealing
with absolute paths.

(cherry picked from commit a490197)
* Mark as `inline` functions implemented in `src/serialize.h`

Also, remove duplicate definitions from `src/staticdata.c` and include
`serialize.h` in there.

(cherry picked from commit 081ae64)
@nanosoldier
Copy link
Collaborator

Your job failed.

@KristofferC
Copy link
Sponsor Member Author

@vtjnash , can you upload log?

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@KristofferC
Copy link
Sponsor Member Author

Wow...

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 28, 2022

I think the test failure for Evolutionary.jl might be real, then a worry. It doesn't find the correct minimum (and uses StableRNG, so seemingly meant to be deterministic). The package supports Threads (but the failing test seem to not use them), and given it's genetic search, so I'm thinking could it fail occasionally randomly? I tested locally and no failures, then its master and 4 failures... I see this option, but seems not used, thus serial:

parallelization::Symbol: allows parallelization of the population fitness evaluation if set to :thread using multiple threads (default: :serial)

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 28, 2022

SugarKelp.jl might have a performance regression, taking much longer. Might this be related to it:

[ Info: These tests check if the code has been broken but does not thoroughly check if it produces correct results (for now at least). They are also so monalithic you can't actually tell which bit you've broken.
[ Info: At level 1
┌ Warning: The special case of bisection over BigFloat with zero tolerance using `A42` is deprecated. Now bisection is used with non-zero tolerances.
│   caller = p(temp::Float64, irr::Float64, params::NamedTuple{(:A_0, :α, :C_min, :C_struct, :γ, :ϵ, :I_sat, :J_max, :K_A, :K_DW, :K_C, :K_N, :N_min, :N_max, :m_2, :m_1, :μ_max, :N_struct, :P_1, :P_2, :a_1, :a_2, :R_1, :R_2, :T_R1, :T_R2, :T_P1, :T_P2, :T_AP, :T_PL, :T_PH, :T_APH, :T_APL, :T_AR, :U_0p65, :K_X, :R_A, :R_B), Tuple{Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Int64, Int64, Int64, Int64, Float64, Int64, Int64, Float64, Float64, Float64, Float64, Int64, Float64, Float64}}) at SugarKelp.jl:132
└ @ SugarKelp ~/.julia/packages/SugarKelp/FymmY/src/SugarKelp.jl:132
┌ Warning: BisectionExact is deprecated; use Bisection
│   caller = #find_zero#27 at bisection.jl:191 [inlined]
└ @ Core ~/.julia/packages/Roots/6PvI4/src/Bracketing/bisection.jl:191

@PallHaraldsson
Copy link
Contributor

Why "wow"? Because mostly ok, or because "3167 failed", since I now realized that's with or without 1.8.3 PR, so not a worry? Or that is, a worry, just unrelated.

@KristofferC
Copy link
Sponsor Member Author

Wow because so few regressions :)

@vtjnash
Copy link
Sponsor Member

vtjnash commented Oct 28, 2022

@DilumAluthge
Copy link
Member

A quick note regarding the two pending (yellow) statuses of the form buildbot/tester_win*.

Windows has been migrated to Buildkite. To see the results of Windows CI, please click on the "Details" link next to the buildkite/julia-release-1.8 commit status.

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Oct 29, 2022

I noticed in the logs (also elsewhere for musl etc.)

Downloading Julia 1.6 for arch x86_64
Julia Version 1.6.7

Should CI be downloading 1.8.x or 1.9?

@KristofferC
Copy link
Sponsor Member Author

It's just using Julia 1.6 to do some "bootstrapping" to setup the environment for the CI to run. So it doesn't really matter what version that is.

@KristofferC
Copy link
Sponsor Member Author

The error in Evolutionary seems to come down to a change in the order in which things are put into a dictionary and the tests seem to rely on this being constant. Also, the tests for Evolutionary pass on this branch using the master branch of the package. So it doesn't seem anything is broken.

SugarKelp passed locally so probably just some slowness with the PkgEval setup which is expected.

@KristofferC
Copy link
Sponsor Member Author

@DilumAluthge, are the windows tests expected to pass on Buildkite?

@DilumAluthge
Copy link
Member

If I recall correctly, the failure in LinearAlgebra/special on i686 was a known failure. But apart from that, I would have expected all tests to pass on Windows on Buildkite.

@staticfloat Have we forgotten to backport something?

Worst case we can turn the Windows Buildbots back on temporarily, but it would be ideal if we could use Buildkite.

aviatesk and others added 4 commits November 3, 2022 23:32
…ease-julia-1.8`, but leave the commit (`f63732c`) unmodified
This warning was un-ironically introduced by "Fix or suppress some noisy tests".
```
┌ Error: mktemp cleanup
│   exception =
│    IOError: unlink("C:\\Windows\\TEMP\\jl_A49B.tmp"): resource busy or locked (EBUSY)
│    Stacktrace:
│      [1] uv_error
│        @ .\libuv.jl:100 [inlined]
│      [2] unlink(p::String)
│        @ Base.Filesystem .\file.jl:968
│      [3] rm(path::String; force::Bool, recursive::Bool)
│        @ Base.Filesystem .\file.jl:283
│      [4] rm
│        @ .\file.jl:274 [inlined]
│      [5] mktemp(fn::Main.Test49Main_misc.var"#110#113", parent::String)
│        @ Base.Filesystem .\file.jl:736
│      [6] mktemp(fn::Function)
│        @ Base.Filesystem .\file.jl:730
│      [7] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1065 [inlined]
│      [8] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
│      [9] top-level scope
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1060
│     [10] include
│        @ .\Base.jl:427 [inlined]
│     [11] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
│     [12] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
│     [13] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
│     [14] macro expansion
│        @ .\timing.jl:440 [inlined]
│     [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
│        @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
│     [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
│        @ Base .\essentials.jl:798
│     [17] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
│        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
│     [18] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
│        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
│     [19] macro expansion
│        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
│     [20] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
│        @ Distributed .\task.jl:490
└ @ Base.Filesystem file.jl:738
```

(cherry picked from commit d28a0dd)

Co-authored-by: Jameson Nash <[email protected]>
@staticfloat
Copy link
Sponsor Member

@KristofferC I have backported the missing piece for #44444 and merged it in. I also identified (with Jameson's help) the missing piece in Pkg that is causing the timeout failures on Windows, and I've submitted a backport for Pkg's release-1.8 branch. Once that is merged, we should bump Pkg to include that here, and that will hopefully get us a green flush on Windows.

@KristofferC
Copy link
Sponsor Member Author

Great!
Regarding cb1e40b, it looks like it just reverts the partial revert I just did of that PR. Before, it got an EBUSY error cleaning up the file there.

@quinnj
Copy link
Member

quinnj commented Nov 7, 2022

Can we include #47419? I think it should backport pretty easily. (happy to push it here if that doesn't mess up some regular flow)

@quinnj
Copy link
Member

quinnj commented Nov 7, 2022

Thanks @KristofferC

@KristofferC KristofferC merged commit 8b67441 into release-1.8 Nov 8, 2022
@KristofferC KristofferC deleted the backports-release-1.8 branch November 8, 2022 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release management and versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.