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

Add buildkite project #3561

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Add buildkite project #3561

merged 1 commit into from
Jan 30, 2025

Conversation

charleskawczynski
Copy link
Member

@charleskawczynski charleskawczynski commented Jan 30, 2025

This PR adds the buildkite Project.toml (xref, xref).

I actually did this programmatically, and doing so allowed me to delete some direct dependencies:

function julia_files(root)
	all_files = String[]
  for (root, dirs, files) in walkdir(root)
      for file in files
          endswith(file, ".jl") && push!(all_files, joinpath(root, file))
      end
  end
  return all_files
end

function find_unecessary_deps(root, deps)
  found = Dict{String,Bool}()
  for dep in deps
    found[dep] = false
  end
  jl_files = julia_files(root)
  N = length(jl_files)
  @show N
  for (i, file) in enumerate(jl_files)
    percent = i / N
  	jl_contents = readlines(file)
  	for dep in deps
  		found[dep] = found[dep] || occursin(dep, join(jl_contents, "\n"))
  	end
  end
  unfound = String[]
  for k in keys(found)
  	if !found[k]
  		push!(unfound, k)
  	end
  end
  return unfound
end

contents = readlines(".buildkite/Project.toml");
_deps = deleteat!(map(x->first(split(x, " = ")), contents), 1);
compat_start = findfirst(x->x=="", _deps)
if !isnothing(compat_start)
  _deps = _deps[1:compat_start-1]
end
@show length(_deps)
ud = find_unecessary_deps(".", _deps)
isempty(ud) || (using Pkg; Pkg.rm(ud))

This unfortunately won't remove packages that are mentioned in julia file doc strings, but I think it was still helpful

@Sbozzolo
Copy link
Member

Thank you! I forgot to commit it.

Some depedencies are surprising. For example, how are we depending on Automa?

Also, some of the compats also seem wrong. For example, I don't think we are compatible with SciMLBase 1 or julia 1.7

@juliasloan25
Copy link
Member

Thank you! examples/Project.toml also still exists. Since this is meant to be replaced by the .buildkite environment, it seems like it should be removed

@juliasloan25 juliasloan25 mentioned this pull request Jan 30, 2025
@Sbozzolo
Copy link
Member

Thank you! examples/Project.toml also still exists. Since this is meant to be replaced by the .buildkite environment, it seems like it should be removed

I left it because people are used to using it, and there is no fundamental problem with having a Project.toml. You can still use it to instantiate your environment locally.

@charleskawczynski
Copy link
Member Author

Automa

No idea, this came along when I instantiated the .buildkite environment (without a .buildkite/Project.toml), which pulls all dependencies in. That's why I used that script to try and slim them back down to only the direct dependencies, but it's not air-tight.

@Sbozzolo
Copy link
Member

Automa

No idea, this came along when I instantiated the .buildkite environment (without a .buildkite/Project.toml), which pulls all dependencies in. That's why I used that script to try and slim them back down to only the direct dependencies, but it's not air-tight.

I was comparing with the Project.toml I have locally I think that there's something wrong with how you got the dependencies. I see a lot of packages that can be removed (but my Project.toml also has package that can be removed). I'll merge the two lists

@Sbozzolo
Copy link
Member

Oh, I see it. It's the examples/Project.toml that has a lot of stuff that is not needed.

@charleskawczynski
Copy link
Member Author

I took a closer look, Automa is an indirect dependency. So this PR looks fine as is.

@charleskawczynski
Copy link
Member Author

#3563 is failing. Let's tackle these issues incrementally. This PR fixes one. If there are more, let's open issues and tackle the remaining separately.

@charleskawczynski charleskawczynski merged commit d4ada46 into main Jan 30, 2025
15 of 19 checks passed
@charleskawczynski charleskawczynski deleted the ck/buildkite_project branch January 30, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants