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

RFC: Make include_dependency(path; track_content=true) the default #54965

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

fatteneder
Copy link
Member

By changing the default to true we make it easier to build relocatable packages from already existing ones when 1.11 lands.

This keyword was just added during 1.11, so its not yet too late to change its default.

@fatteneder fatteneder added the compiler:precompilation Precompilation of modules label Jun 27, 2024
@vchuravy vchuravy added the backport 1.11 Change should be backported to release-1.11 label Jun 27, 2024
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

I am in favor of this. But to double check if the file doesn't exist this will throw an error?
Previously it would not? Maybe add that to the compat note.

@fatteneder
Copy link
Member Author

I am in favor of this. But to double check if the file doesn't exist this will throw an error?
Previously it would not?

Well spotted. Although that was a consequence of #53286 and not changing the default.

I added a note.

@vchuravy
Copy link
Member

@KristofferC your thoughts since it is late in the cycle?

@vchuravy
Copy link
Member

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@fatteneder
Copy link
Member Author

PkgEval looks good. But just to be sure.

@nanosoldier runtests(["EarlyStopping", "BlockBandedMatrices", "FiniteDiff", "Pyehtim", "StatisticalProcessMonitoring", "OptimizationPRIMA", "DECAES", "OptimizationOptimJL", "PEPSKit", "LongwaveModePropagator", "ODEProblemLibrary", "GeneralizedSasakiNakamura", "NonconvexBayesian", "PGFPlots", "ConceptualClimateModels", "AdaptiveTestPlots", "SMLMSim", "TulipaPlots", "OVERT", "RetentionParameterEstimator"], vs = ":master")

@nanosoldier
Copy link
Collaborator

The package evaluation job you requested has completed - no new issues were detected.
The full report is available.

@JanisErdmanis
Copy link

JanisErdmanis commented Jul 2, 2024

Note that this fix is needed when Julia application is bundled into MSIX archive as makeappx sets mtime of all files when they are put into the archive. For my situation an alternative is submitting pull requests to ConstructionBase, CxxWrap, ExceptionUnwrapping, Infiltrator and RelocatableFolders.

I am also considering applying a regex for include_dependecy after bundling in AppBudler but I would prefer using a nightly 1.11 bundle with this change in JuliaCon presentation demo part.

@vchuravy
Copy link
Member

vchuravy commented Jul 2, 2024

I am going to merge this for now and defer the backport decision to Kristoffer.

@vchuravy vchuravy merged commit 6cf3a05 into master Jul 2, 2024
8 checks passed
@vchuravy vchuravy deleted the fatteneder-patch-1 branch July 2, 2024 17:22
@KristofferC KristofferC mentioned this pull request Jul 12, 2024
42 tasks
KristofferC pushed a commit that referenced this pull request Jul 23, 2024
…54965)

By changing the default to `true` we make it easier to build relocatable
packages from already existing ones when 1.11 lands.

This keyword was just added during 1.11, so its not yet too late to
change its default.

(cherry picked from commit 6cf3a05)
KristofferC added a commit that referenced this pull request Jul 26, 2024
Backported PRs:
- [x] #54201 <!-- Fix generic triangular solves with empty matrices -->
- [x] #54358 <!-- Create `jl_clear_coverage_data` to dynamically reset
coverage -->
- [x] #54908 <!-- LazyString in interpolated error messages in
threadingconstructs -->
- [x] #54952 <!-- LAPACK: Avoid repr call in `chkvalidparam` -->
- [x] #54898 <!-- fix concurrent module loading return value -->
- [x] #55082 <!-- Add fast method for copyto!(::Memory, ::Memory) -->
- [x] #55084 <!-- Use triple quotes in TOML.print when string contains
newline -->
- [x] #55141 <!-- Update the aarch64 devdocs to reflect the current
state of its support -->
- [x] #54955 <!-- don't throw EOFError from sleep -->
- [x] #54871 <!-- Make warn missed transformations pass optional -->
- [x] #55178 <!-- Compat for `Base.@nospecializeinfer` -->
- [x] #55197 <!-- compat notice for a[begin] indexing -->
- [x] #54917 <!-- Fix potential underrun with annotation merging -->
- [x] #55209 <!-- correction to compat notice for a[begin] -->
- [x] #55203 <!-- document mutable struct const fields -->
- [x] #54791 <!-- Bump libblastrampoline to v5.10.1 -->
- [x] #54950 <!-- SuiteSparse: Bump version -->
- [x] #54956 <!-- Fix accidental early evaluation of imported `using`
binding -->
- [x] #54996 <!-- inference: add missing `MustAlias` widening in
`_getfield_tfunc` -->
- [x] #55070 <!-- LinearAlgebra: LazyString in error messages for
Diagonal/Bidiagonal -->
- [x] #54574 <!-- Make ScopedValues public -->
- [x] #54739 <!-- finish implementation of upgradable stdlibs -->
- [x] #54965 <!-- RFC: Make `include_dependency(path;
track_content=true)` the default -->
- [x] #53286 <!-- Raise an error when using `include_dependency` with
non-existent file or directory -->
- [x] #55066 <!-- fix loading of repeated/concurrent modules -->
- [x] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [x] #55218 <!-- Artifacts: use a different way of getting the UUID of
a module -->
- [x] #54891 <!-- #54739-related fixes for loading stdlibs -->
- [x] #55072 <!-- trace-compile: don't generate `precompile` statements
for OpaqueClosure methods -->
- [x] #55188 <!-- Make Core.TypeofUnion use the type method table -->

Need manual backport:
- [ ] #55052 <!-- Fix `(l/r)mul!` with `Diagonal`/`Bidiagonal` -->


Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #55148 <!-- Random: Mark unexported public symbols as public -->
- [ ] #55017 <!-- TOML: Make `Dates` a type parameter -->
- [ ] #55013 <!-- [docs] change docstring to match code -->
- [ ] #54919 <!-- Fix annotated join with non-concrete eltype iters -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
- [ ] #41244 <!-- Fix shell `cd` error when working dir has been deleted
-->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:precompilation Precompilation of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants