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.

Sign up for GitHub

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

Jump to bottom

Add Artifacts to Pkg #1277

Merged
merged 13 commits into from
Aug 15, 2019
Merged

Add Artifacts to Pkg #1277

merged 13 commits into from
Aug 15, 2019

Conversation

staticfloat
Copy link
Sponsor Member

@staticfloat staticfloat commented Aug 1, 2019 •
edited
Loading

This adds the artifacts subsystem to Pkg, read this WIP blog post for more details. Closes #841 and #1234.

This PR still needs:

  • A pkg> gc hook that looks at the list of projects that we know about, examines which artifacts are bound, and marks all that are unbound. Unbound artifacts that have been continuously unbound for a certain time period (e.g. one month, or something like that) will be automatically reaped.
  • Greater test coverage (even without seeing the codecov report, I am certain of this), especially as related to the installation of platform-specific binaries.
  • Overrides.toml support for global overrides of artifact locations

@staticfloat
Copy link
Sponsor Member Author

bors try

bors bot added a commit that referenced this pull request Aug 1, 2019
@bors
Try #1277:
47da955
@bors
Copy link
Contributor

bors bot commented Aug 1, 2019

try

Build failed

KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/API.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
KristofferC
KristofferC reviewed Aug 2, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl Outdated Show resolved Hide resolved
oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl
cxxstring_abi = nosym(get(entry, "cxxstring_abi", nothing))

# Construct the actual Platform object
return P(arch;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the

 os_map = Dict(
        "windows" => Windows,
        "macos" => MacOS,
        "freebsd" => FreeBSD,
        "linux" => Linux,
    )
    P = get(os_map, lowercase(entry["os"]), UnknownPlatform)

Stuff to here?

So it is clearer where P comes from?

oxinabox
oxinabox reviewed Aug 6, 2019
View reviewed changes
src/Artifacts.jl
FreeBSD => "freebsd",
Linux => "linux",
)
meta["os"] = os_map[typeof(p)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe break the above into very short functions?
name(p::Windows)="windows" etc

@StefanKarpinski
Copy link
Sponsor Member

bors r+

bors bot added a commit that referenced this pull request Aug 15, 2019
@bors @staticfloat
Merge #1277
1ad655c 
1277: Add Artifacts to Pkg r=StefanKarpinski a=staticfloat

This adds the artifacts subsystem to Pkg, [read this WIP blog post](https://github.com/JuliaLang/www.julialang.org/pull/417/files?short_path=514f74c#diff-514f74c34d50677638b76f65d910ad17) for more details.  Closes #841 and #1234.

This PR still needs:

- [x] A `pkg> gc` hook that looks at the list of projects that we know about, examines which artifacts are bound, and marks all that are unbound.  Unbound artifacts that have been continuously unbound for a certain time period (e.g. one month, or something like that) will be automatically reaped.
- [x] Greater test coverage (even without seeing the codecov report, I am certain of this), especially as related to the installation of platform-specific binaries.
- [x] `Overrides.toml` support for global overrides of artifact locations

Co-authored-by: Elliot Saba <staticfloat@gmail.com>
@StefanKarpinski
Copy link
Sponsor Member

bors r-

@bors
Copy link
Contributor

bors bot commented Aug 15, 2019

Canceled

@StefanKarpinski
Copy link
Sponsor Member

bors try

bors bot added a commit that referenced this pull request Aug 15, 2019
@bors
Try #1277:
43f164b
@StefanKarpinski
Copy link
Sponsor Member

bors try

@bors
Copy link
Contributor

bors bot commented Aug 15, 2019

try

Already running a review

@StefanKarpinski
Copy link
Sponsor Member

bors try

@bors
Copy link
Contributor

bors bot commented Aug 15, 2019

try

Already running a review

@StefanKarpinski
Copy link
Sponsor Member

Jesus Christ, bors, just run the f*@&ing tests.

@KristofferC KristofferC mentioned this pull request Aug 15, 2019
Closed
@bors
Copy link
Contributor

bors bot commented Aug 15, 2019

try

Timed out

@fredrikekre
Copy link
Member

Probably was running. But running on 4 Julia versions on AppVeyor takes 2 hours.

fredrikekre
fredrikekre reviewed Aug 15, 2019
View reviewed changes
docs/src/api.md Outdated Show resolved Hide resolved
docs/src/managing-packages.md Outdated Show resolved Hide resolved
docs/src/managing-packages.md Outdated Show resolved Hide resolved
appveyor.yml
- julia_version: 1
- julia_version: 1.1
- julia_version: 1.2
- julia_version: 1.3
Copy link
Member

Choose a reason for hiding this comment

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

Should be enough to run on 1.3 and nightly, we will not ship this version of Pkg with older Julia anyway.

docs/src/artifacts.md Show resolved Hide resolved
docs/src/artifacts.md Show resolved Hide resolved
src/API.jl
finding artifacts and packages that are thereafter not used by any other projects. This
method will only remove package versions and artifacts that have been continually un-used
for a period of `collect_delay`; which defaults to thirty days.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Should this docstring be attached to the binding in Pkg.jl?

src/Artifacts.jl Outdated Show resolved Hide resolved
staticfloat and others added 4 commits August 15, 2019 14:09
@staticfloat @fredrikekre
Apply Frederik's doc suggestions
8fbb6aa 
Co-Authored-By: Fredrik Ekre <ekrefredrik@gmail.com>
@staticfloat @fredrikekre
Update src/Artifacts.jl
e3d1911 
Co-Authored-By: Fredrik Ekre <ekrefredrik@gmail.com>
@staticfloat
@warn -> @error
93be0de
@staticfloat
Add tests for invalid Overrides.toml files
8197e22
@staticfloat
Copy link
Sponsor Member Author

bors r+

@staticfloat
Change collect_delay to be a Dates.Period
3d5bec2 
This gives greater clarity on the meaning behind `collect_delay`.
@bors
Copy link
Contributor

bors bot commented Aug 15, 2019

Canceled

@staticfloat
Copy link
Sponsor Member Author

I'm sorry bors, but you have failed me for the last time. Here we go, squashing and merging in one fell swoop. Next up, PR to base Julia.

@staticfloat staticfloat merged commit 8631324 into master Aug 15, 2019
@bors bors bot deleted the sf/artifactory branch August 15, 2019 21:57
@staticfloat staticfloat mentioned this pull request Aug 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KristofferC KristofferC KristofferC left review comments

@oxinabox oxinabox oxinabox left review comments

@fredrikekre fredrikekre fredrikekre left review comments

@StefanKarpinski StefanKarpinski StefanKarpinski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pkg + BinaryProvider
5 participants
@staticfloat @StefanKarpinski @fredrikekre @KristofferC @oxinabox