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

Update manifests by default #102

Closed
wants to merge 1 commit into from
Closed

Update manifests by default #102

wants to merge 1 commit into from

Conversation

christopher-dG
Copy link
Member

@christopher-dG christopher-dG commented Nov 10, 2019

This finds all Manifest.toml files, activates their environments, and runs Pkg.update.
There is a platform-specific find invocation in there, I suppose it should probably go away if we have something like #85. It could be implemented instead with walkdir or Glob.jl (although vtjnash/Glob.jl#19 is needed).

@DilumAluthge
Copy link
Member

FYI we should do the package operations in a separate Julia process.

So for example, you'd have something like this:

function update_manifests()
    list_of_manifest_files = blah blah blah # use walkdir
    for f in list_of_manifest_files
        @info "Updating manifest: $f"
        run(`$(Base.julia_cmd()) -e 'import Pkg; Pkg.activate("$(f)")'; Pkg.update()`)
    end
end

@DilumAluthge
Copy link
Member

Also, we'll need at least some kind of test

@christopher-dG
Copy link
Member Author

Can you explain why this needs to happen in a new process?

@DilumAluthge
Copy link
Member

Here are some snippets from Kristoffer in a Slack conversation in #pkg-usage on Thursday:

Sounds like you should spawn a new julia process with --project=example/A/ instead. Because if you load stuff in one environment and then change environment and load other things, strange stuff can happen.

Loading stuff from different environments in one process is just not great.

Well, you shouldn't really load things from different subprojects in the same process either.

@christopher-dG
Copy link
Member Author

Works for me 😄

@DilumAluthge
Copy link
Member

DilumAluthge commented Nov 10, 2019

Here's a question for you.

If both path/Project.toml and path/Manifest.toml exist, which is a better idea?

  1. cd into path and run Pkg.instantiate(); Pkg.update()
  2. cd into path, delete the path/Manifest.toml, and then run Pkg.instantiate(); Pkg.update()

@christopher-dG
Copy link
Member Author

Hm, I'm not sure if there's a difference. But AFAIK, just doing update will update the manifest in the right way.

bors bot pushed a commit that referenced this pull request Nov 10, 2019
104: Automatically update all Manifest files r=DilumAluthge,christopher-dG a=DilumAluthge

Closes #102 

This is an expanded version of the excellent work by @christopher-dG in #102. This PR adds the following on top of the work in #102:
1. Adds tests.
2. Makes sure that we support custom registries.
3. Run jobs in a "sandbox" that does not have access to most environment variables. That way, if a package has a malicious build step, for example, they cannot steal your GitHub token.

Co-authored-by: Dilum Aluthge <[email protected]>
@bors bors bot closed this in 461b050 Nov 10, 2019
@bors bors bot closed this in #104 Nov 10, 2019
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