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

Prepare 0.14 #709

Closed
wants to merge 11 commits into from
Closed

Prepare 0.14 #709

wants to merge 11 commits into from

Conversation

JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Sep 26, 2020

This merge was done by checking each repo's Network page on GitHub to determine which version of the project should be used.

  • If the project had a branch that could be merged into master, it was stated there.
  • If it didn't, but had a fork with a branch that could be merged into master, this was stated.

All other projects compiled on ps-0.14 in my original attempt back in February or so. I will need to check each repo again after this commit.

This merge was done by checking each repo's Network page on GitHub to 
determine which version of the project should be used. If the project 
had a branch that could be merged into `master`, it was stated there. If 
it didn't, but had a fork with a branch that could be merged into 
master, this was stated. All other projects compiled on ps-0.14 in 
Jordan's original attempt and will need to be checked again after this 
commit.
@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Sep 26, 2020

I installed all packages in the package set using Harry's bash script
I then adjusted my purescript-module-graph project to output the following content:

<number of dependencies>-<package name>

It's sorted first by the number of dependencies (lowest first) and then by the package name (alphabetical)

That gets us this list:
ordered-content.txt

From here, I can output a single spago.dhall file for each package that only installs its necessary dependencies. I think that will help me get through all of the compiler warning noise as I update packages.

@thomashoneyman
Copy link
Member

Nice work! Are you going filter to just core / contrib to start, or try and do the whole set? That seems like a lot of work!

@JordanMartinez
Copy link
Contributor Author

Just core and contrib. However, I'm running into various issues with tooling that complicates things for me. For example, spago does not provide a way to use a local purs binary that's specified via path.

@f-f
Copy link
Member

f-f commented Sep 27, 2020

@JordanMartinez I'm not sure what you mean? Spago should just pick up whatever purs is in the PATH?

@JordanMartinez
Copy link
Contributor Author

Ah... I didn't know that. So, I could so something like this?

export purs=./purs-v0.14-rc2 spago -x prelude.dhall build

Since there are so many compiler warnings, it's harder to know what needs to be updated on an individual project basis.

@JordanMartinez
Copy link
Contributor Author

So.... this didn't work:

export purs=./purs-v0.14-rc2 spago -x prelude.dhall build

I think it's because I'm using nvm.

Regardless, here's another update on that ordered-content.txt file I attached above. I discovered that it only sometimes included transitive dependencies. My algorithm was based only on the actual modules the library's files imported via the output of purs graph. I reworked the algorithm to parse the output of dhall-to-json --compact --file ./packages.dhall --output ./packageSet.json, and then calculate the full transitive dependencies of each package.

Let me state that again, the algorithm now calculates both the direct and transitive dependencies of every package in the package set. (side note: we once talked about updating the package sets to require libraries to specify direct and transitive dependencies. Is now a good time to do that?)

After finishing that, I updated ordered-content.txt to do the same as above, but also output the names of all the packages' direct and transitive dependencies.

Attached is the updated file: ordered-content.txt
To build the tool yourself, see purescript-package-graph

Let me explain why I did this.

  • There are a whole lot of compiler warnings if I compile the entire package set via purs compile ".spago/*/*/src/**/*.purs". It's difficult to sift through this noise when I'm focusing on only updating one library.
  • During this update, we will be making breaking changes to some libraries.
  • To update the ecosystem efficiently, we will need to parallelize the work as much as possible. This tool that anyone can build (and modify) would help one answer the following questions:
    • Contributors: which libraries can I update so that Library X can get updated?
    • Authors/Maintainers: which libraries do I need to wait on before I can update Library X?

This tool enables my ideal workflow:

  • I pass only the necessary source globs for compiling a given package into purs
  • I only see errors and warnings for a given package
  • I update the package to compile on PS v0.14 and account for breaking changes in my dependencies
  • I submit a PR
  • Author/Maintainer merges my PR
  • I update the package set to refer to the updated branch/release
  • I reinstall that library via spago, so that my local code uses the updated branch/release
  • I move on to the next library per the ordered-content.txt file above
  • Loop until the package set is fully updated, starting with the purescript and purescript-contrib libraries.

@f-f f-f changed the title Merge my fork's 'fix-warnings-ps-compatible' branch into 'prepare-0.14' Prepare 0.14 Sep 28, 2020
@f-f
Copy link
Member

f-f commented Sep 28, 2020

@JordanMartinez yeah sorry I didn't clarify well: Spago will not literally read from the purs environment variable, but will pick up whatever purs executable you have in your PATH variable. So if you have a purs binary (it needs to have that exact name) in a folder called purs-0.14, then you can have Spago use it in this way:

PATH="$(pwd)/purs-0.14:${PATH}" spago build

@JordanMartinez
Copy link
Contributor Author

@f-f Oh... duh! Thanks for the help!

@f-f
Copy link
Member

f-f commented Sep 29, 2020

You're welcome 🙂

@JordanMartinez
Copy link
Contributor Author

I'd like to propose making my fork's prepare-0.14 branch this repo's prepare-0.14 branch. That would require one of us to force-push to overwrite this repo's current prepare-0.14 branch, so I thought I'd ask before doing that.

Other than that, the purescript-package-graph project is ready for use. I've already tested it out on purescript/purescript-math and it compiles without error.

@f-f
Copy link
Member

f-f commented Sep 30, 2020

@JordanMartinez the point of that would be to start from a set of packages that work rather than trying to work backwards from the big set?

My preference goes to whatever is less work for you, as long as we're careful to not downgrade package versions once we are ready to merge to master

@JordanMartinez
Copy link
Contributor Author

There's a few reasons for this:

  • master will continue to accumulate more commits. If I keep merging those commits into the prepare-0.14 branch, it will cause merge conflicts as we update the package set. I'd rather handle that after most of the package set is updated.
  • once I get the initial repos done/updated, we need a centralized place for others to submit their updates. That could be my fork of this repo or this repo's prepare-0.14 branch. I would prefer the latter because more people would be notified when their libraries' dependencies have been updated. It also means some library ecosystems (e.g. halogen) could be updated independently of other library ecosystems (e.g. react)
  • I imagine that at some point, the v0.14.0 package set will have a lot of the packages currently in master but not all of them. At some point, we'll need to release the v0.14.0 package set, but it will not include all the packages in master because some packages never got updated. I think this would incentivize authors to update them without blocking the v0.14.0 package set from becoming the master branch.

@JordanMartinez
Copy link
Contributor Author

Ok, I will be force-pushing my fork's prepare-0.14 branch to this repo by the end of the day.

@JordanMartinez
Copy link
Contributor Author

This PR was force-pushed to the prepare-0.14 branch of this repo.

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.

3 participants