transition Pkg.TOML to stdlib/TOML #1011

StefanKarpinski · 2019-01-23T23:47:46Z

The Pkg.TOML internal package (yes, it's in ext because it's an "external dependency") was originally based on @wildart's https://github.com/wildart/TOML.jl package. It was forked and vendored for a few of reasons: (1) @wildart got busy and wasn't merging pull requests for a while and (2) it wasn't feasible for the package manager to have an external package as a dependency and (3) I wasn't ready to deal with making it a stdlib package at the time.

It's pretty stable at this point and since the registered TOML package is long abandoned (last commit three years ago), it seems like it would be a good time to make Pkg.TOML an official stdlib package. Here is a plan to get there:

Get Pkg.TOML's tests running (Test TOML #1012)
Delete https://github.com/pygy/TOML.jl from the registry
Merge improvements from https://github.com/wildart/TOML.jl into Pkg.TOML
Review Pkg.TOML's API and make sure it's what we want—does it match JSON, for example?
Move stdlib/Pkg/ext/TOML to stdlib/TOML

For deleting https://github.com/pygy/TOML.jl from the registry, we can just special case it to get dropped in the sync process. Since it doesn't support Julia ≥ 0.7 (or even 0.6) this shouldn't bother anyone. We should technically give this TOML a new UUID but I'm not sure it matters. The sync script may assume that it can compute stdlib UUIDs from their names somewhere, which would be broken by given the new stdlib/TOML a different UUID from the one that the old TOML already has.

The text was updated successfully, but these errors were encountered:

tpapp · 2019-02-25T08:09:29Z

I am thinking about making some PRs to https://github.com/wildart/TOML.jl (mainly for some TOML v0.5 features I want to use), but I am wondering whether I should wait with that until the merge is done. Is there an approximate timeline?

I could also help with the merge (if the issue is lack of hands, not decisions).

StefanKarpinski · 2019-02-25T16:53:55Z

I don't think any further progress towards the merge has happened. It's on my todo list but not near the top, unfortunately, so any help would be greatly appreciated, as would new features :)

tpapp · 2019-02-28T13:43:20Z

@wildart, @StefanKarpinski : what would be the preferred way of contributing towards a merge?

Eg PRs to Pkg.jl, or to TOML.jl?

Small incremental ones (which is what I would prefer) or larger WIP ones?

StefanKarpinski · 2019-02-28T16:45:48Z

Small incremental ones

👍

tamasgal · 2019-03-03T08:47:40Z

Ahm, I am a bit confused to be honest. So PRs should be make towards this repository or to Pkg.jl? I need TOML as well and consider to help out if I manage to get some free time...

StefanKarpinski · 2019-03-03T14:50:59Z

Either direction is fine, just to get them to converge. You could ask for a particular change. The first step would be to look at a diff of the two packages and try to see what has even diverged.

lewisl · 2019-07-18T17:43:27Z

Where does this stand at the moment? I see only the first task checked above. This looks like it is within my moderate abilities so I could contribute.

How do you suggest I start, having not worked on official packages?

By forking Pkg.TOML
Diffing with @wildart's latest? (seeing what's interesting...)
Merging what appear to be meaningful changes?

Seems like item 2 could be done at any time--now? Who can do this
Item 3 is my item 2.
How important is it that the TOML api match the JSON api? I suppose that is anyone's reasonable opinion. At a start, why not leave as is? Both packages have really simple APIs.

Let me know: while not hugely important, this is manageable work that could get done soon.

lewisl · 2019-07-18T19:25:54Z

I note that Pkg.TOML generates errors on any/all floating point values.

StefanKarpinski · 2019-07-18T19:45:01Z

That would be great. I think no further work has been done on this but we still very much want it.

lewisl · 2019-07-24T19:40:51Z

Update.

I have used Pkg/ext/TOML as a base rather than @wildart.TOML. This includes Kristoffer's performance work, bug fixes, updates for Julia 1.0, and previous incorporation of several of @wildart's fixes.

I diffed the 3 working files: parser.jl, print.jl, TOML.jl with the latest of @wildart. There are no functionality differences. Most changes in the Pkg version were fixes for Julia > 0.7 and improvements such as replacing abstract types with concrete types.

I reviewed the issues in Pkg pertaining to TOML and @wildart's open issues, added a couple of my own--see below.

I checked if Pkg.TOML supports TOML 0.4. Thanks to @wildart and Kristoffer's improvements, it does.

I made slight changes to the code to simplify it with no functional changes except one bug fix (floating point numbers work). All tests pass, noting that the Pkg tests and @wildart tests might not have been converged yet.

Is this worth a pull request and moving to stdlib? Look at the outstanding issues and tell me if they should be addressed.

Issues review:

Pkg#1010, #1012   floating point numbers not read--errors out.  FIXED
?           can't print TOML with dict keys that are symbols:   SEEMS WORTH FIXING
Pkg#934     write TOML files with \r\n line endings on Windows:  NEED TO TEST
                    note: parse works now with \n or \r\n (TOML v 0.4 feature)
      test on Windows:  MILD NUISANCE BUT I WILL DO
      test on Julia = 0.7 <= version < 1.1.0: YOU GUYS WOULD KNOW IF SAFE
           TO RELY ONLY ON TESTING ON V 1.1.0
?     TOML.parse reads empty sections (tables)->creates Dict{String, Any}()
       which is valid;  TOML.print ignores empty Dict and creates NO 
       section--also valid;  do we care this is slightly inconsistent?   I'd say not...
?      did not converge test cases between Pkg.TOML and @wildart: WILL LOOK
other   requests for TOML version 0.5 features: per @StefanKarpinski's inclination
            to proceed in small increments:                 DEFER

lewisl · 2019-07-24T20:17:09Z

Fixed: use symbols as keys in a Dict and TOML.print outputs unquoted keys in the TOML output.
Added suitable test case.

fredrikekre · 2019-07-24T21:16:59Z

Nice!
I git-filtered ext/TOML to this repo: https://github.com/JuliaStdlibs/TOML.jl so maybe we can work there and then transfer that to be a real stdlib.

lewisl · 2019-07-26T16:44:24Z

I will submit a pull request for the changes I've made. I added a test for Dict with symbol keys printed to TOML. Existing tests already covered floating point values. As noted, all the tests pass.

This is the first time I've done this: DO point out what I've done wrong. I am sure I made a mistake in runtests.jl. I was loading into the REPL and had the expected name space problem so I changed both the way I use Pkg.TOML and changed a test that functionally passed, but was tripped up by name space collision between Pkg.TOML and base.TOML. This may prevent the revised tests from running. Send me the correction and suggestions.

lewisl · 2019-07-27T23:48:10Z

Sorry for the past mess. Much progress.
Latest pull request should be much cleaner.

My local environment installed @wildart.TOML. When I ran tests, I included my fork of Pkg/ext/TOML, and the import imported @wildart. That is why I saw the tests all pass: I was testing a horrible blend of both modules. I rm'ed @wildart.TOML.
I removed TOML.get(... from the tests as I remove that method from the code.
I added the fork to my LOAD_PATH so I could drop the include. The name of the module is no longer mangled with a dot, which had caused tests on type TOML.Table to fail (I had hacked the tests). I set these back as the original file defined these tests. isa(foo, TOML.Table). These pass.
I added 4 tests that @wildart included which were not in Pkg/ext/TOML/test.
I run the tests as you prescribed with these results:

julia> import Pkg; Pkg.test()
   Testing TOML
 Resolving package versions...
Test Summary: | Pass  Total
TOML          |  252    252
   Testing TOML tests passed

Note: there are now 252 tests. I could not locate the test file you also ran containing 130 or so tests. This must be from the old version of TOML carried around as part of Base??? I think this is accessible in Travis, but I could not find it (even though I enrolled in Travis).

I hope that Pkg/ext/TOML/test/runtests.jl should do much better in Travis now.

If you point me to the other test file I can either add it as a second test file (which might break the workflow) or--better--add those tests which are not duplicates. Note that the older tests will still reference the method TOML.get.

Note there were 2 ways to fix the floating point value bug: either import Base: get so that TOML could add a method. Or get rid of the TOML.get method and then the import is unnecessary. I did the latter.

00vareladavid · 2019-08-30T16:52:36Z

I think #1302 should take care of the second TODO.

And does #1272 satisfy the third TODO?

KristofferC · 2019-09-02T06:50:18Z

I think so, yes.

scottstanie · 2019-09-21T16:35:09Z

If I were to try to add a small feature to the TOML parser (the ability to parse simply mydate = 2019-01-01), should I add that here? to https://github.com/wildart/TOML.jl ? or https://github.com/JuliaStdlibs/TOML.jl ?

wildart · 2019-09-24T16:34:39Z

PR to JuliaStdlibs/TOML. Added the deprecation message to wildart/TOML.

Nosferican · 2019-10-11T01:00:58Z

Is this Julia v1.4 milestone or when should the move be expected?

StefanKarpinski · 2019-10-11T04:20:33Z

When someone does it. Unfortunately, no one has had the bandwidth to work on it.

Nosferican · 2019-10-11T04:26:56Z

The second item has already been ✅ and can be checked off.
I am not sure about the status of three... @wildart, are there any improvements that haven't been ported to the Pkg/ext/TOML?
I can check if there are API issues that might be desirable to address before porting it out.

ziotom78 · 2020-01-17T07:35:41Z

PR to JuliaStdlibs/TOML. Added the deprecation message to wildart/TOML.

Has the repo https://github.com/JuliaStdlibs/TOML.jl been moved elsewhere? I get 404.

fredrikekre · 2020-01-17T07:55:25Z

Yea, I deleted it.

ekmecic · 2020-01-18T18:21:42Z

Where can it be found now?

tpapp · 2020-01-25T07:45:55Z

So PRs for TOML.jl this should go to this repository?

lewisl · 2020-01-27T04:22:50Z

The second item has already been ✅ and can be checked off.

I am not sure about the status of three... @wildart, are there any improvements that haven't been ported to the Pkg/ext/TOML?

I can check if there are API issues that might be desirable to address before porting it out.

When I updated Pkg/ext/TOML I diffed with @wildart version and there were no functional differences. Seems like the meaningful improvements would be moving to TOML 5.0. One of the official maintainers should decide if it will move to stdlib or stay with Pkg. I am willing then to start on TOML 5.0 with anyone who would like to chip in.

tpapp · 2020-01-27T06:26:18Z

I would still like to help out. I hope that TOML will be excised to stdlib for 1.5, would be easier to contribute. The full test suite suite ensure that it keeps working for Pkg.

KristofferC · 2020-01-27T12:14:36Z

There is nothing technical blocking this from happening. The wildart TOML and the TOML here should be merged (they might already be pretty much identical), the API should be audited to make sure it looks good (look at how JSON.jl) do it, and then make it an stdlib and then make a PR to Pkg to use the TOML stdlib instead.

StefanKarpinski · 2020-01-27T15:10:35Z

Since we're moving towards vendoring things that Pkg depends on, we should do that here as well. I.e. set it up so that Pkg depends on a snapshot of the official TOML package rather than making TOML a stdlib. I'm happy to do that if someone else gets things to the point where wildart/TOML.jl and this TOML are identical.

lewisl · 2020-01-27T15:50:40Z

What’s the official TOML package? Should we just start a new one? - Lewis

StefanKarpinski · 2020-01-27T16:42:02Z

@wildart/TOML.jl is the registered TOML package. Pkg.TOML is strictly internal to Pkg and should not be depended on by external code, although I have no doubt that it is being used. There's no reason to create a new TOML package, just converge these two and then we can start using a snapshot of @wildart/TOML.jl from Pkg.

tpapp · 2020-01-27T17:27:15Z

OTOH https://github.com/wildart/TOML.jl says that

This package is no longer maintained. It is superseded by a standard library package JuliaStdlibs/TOML.

which was created, then deleted for reasons I that are not clear to me.

This is somewhat confusing.

StefanKarpinski · 2020-01-27T18:06:53Z

@wildart, could you make me an owner of your TOML.jl repo so I can transfer it to an appropriate org and we can reinstate it as the official and maintained TOML package?

wildart · 2020-01-29T15:33:47Z

@StefanKarpinski I added you as a collaborator. I tried to transfer ownership to you, but couldn't because you already have your own TOML.jl repo. Do you want me to transfer this repo to other organization?

fredrikekre · 2020-01-29T15:43:35Z

We could just push your repo to JuliaLang/TOML.jl. Edit: Unless we care about GitHub automatic redirects etc.

StefanKarpinski · 2020-01-29T16:21:09Z

@wildart, I think I need to be an admin on the repo in order to transfer it.

fredrikekre · 2020-01-29T16:30:28Z

You can't be an admin on a personal account, you are either the owner (wildart in this case) or a collaborator (https://help.github.com/en/github/setting-up-and-managing-your-github-user-account/permission-levels-for-a-user-account-repository).

To make this transfer possible I believe wildart needs elevated permissions in the JuliaLang org.

StefanKarpinski · 2020-01-29T16:35:44Z

@wildart, you should already have permission to create repos on JuliaLang and therefore be able to transfer the repo.

wildart · 2020-01-29T18:16:46Z

Done. It's JuliaLang/TOML.jl now.

lewisl · 2020-01-29T18:21:24Z

What do we need to do to replace the src and test folders with the version at Pkg/ext? This has bug fixes and improvements from mid-2019 contributed by me and Kristoffer?

* Use [`SafeTestsets.jl`](https://github.com/YingboMa/SafeTestsets.jl)to put all `@testset`s into a separate module, with a name generated by `gensym()`. Added as an `extra` dep, only downloaded when `test`ing * Added [`ArgParse.jl`](https://github.com/carlobaldassi/ArgParse.jl) and the [(currently internal)](JuliaLang/Pkg.jl#1011) [`TOML.jl`](https://github.com/JuliaLang/TOML.jl). Showcasing trivial functionality and project structure

fredrikekre · 2020-08-27T16:41:15Z

Done.

lewisl · 2020-08-27T18:00:42Z

Does this include the fixes for values that are floating point and for generating TOML from a Julia dict using a symbol key? The Pkg.TOML version has these.

KristofferC · 2020-08-27T18:23:22Z

Does this include the fixes for values that are floating point

I don't know which one that is.

generating TOML from a Julia dict using a symbol key?

Yes.

This was referenced Jan 28, 2019

Advertise TOML.jl (and Julia itself) in TOML Wiki? JuliaAttic/TOML_old.jl#16

Closed

Register TOML.jl with METADATA.jl JuliaAttic/TOML_old.jl#17

Closed

00vareladavid mentioned this issue Feb 4, 2019

excise ext/* #118

Closed

StefanKarpinski mentioned this issue Feb 4, 2019

improvements to downstream to Pkg.TOML? JuliaAttic/TOML_old.jl#18

Closed

wildart mentioned this issue Feb 22, 2019

Julia v1.0 compatible release? JuliaAttic/TOML_old.jl#19

Closed

tpapp mentioned this issue Feb 25, 2019

Update to TOML 0.5.0 JuliaAttic/TOML_old.jl#11

Closed

lewisl mentioned this issue Jul 26, 2019

Pull request per issue #1011 #1267

Closed

00vareladavid added the TOML label Sep 6, 2019

fkastner mentioned this issue Aug 11, 2020

url to superseded version is broken JuliaAttic/TOML_old.jl#22

Closed

fredrikekre closed this as completed Aug 27, 2020

transition Pkg.TOML to stdlib/TOML #1011

transition Pkg.TOML to stdlib/TOML #1011

Comments

StefanKarpinski commented Jan 23, 2019 • edited by KristofferC Loading

tpapp commented Feb 25, 2019

StefanKarpinski commented Feb 25, 2019

tpapp commented Feb 28, 2019

StefanKarpinski commented Feb 28, 2019

tamasgal commented Mar 3, 2019

StefanKarpinski commented Mar 3, 2019

lewisl commented Jul 18, 2019

lewisl commented Jul 18, 2019

StefanKarpinski commented Jul 18, 2019

lewisl commented Jul 24, 2019

lewisl commented Jul 24, 2019 • edited Loading

fredrikekre commented Jul 24, 2019

lewisl commented Jul 26, 2019

lewisl commented Jul 27, 2019

00vareladavid commented Aug 30, 2019

KristofferC commented Sep 2, 2019

scottstanie commented Sep 21, 2019

wildart commented Sep 24, 2019 • edited Loading

Nosferican commented Oct 11, 2019

StefanKarpinski commented Oct 11, 2019

Nosferican commented Oct 11, 2019

ziotom78 commented Jan 17, 2020

fredrikekre commented Jan 17, 2020

ekmecic commented Jan 18, 2020

tpapp commented Jan 25, 2020

lewisl commented Jan 27, 2020

tpapp commented Jan 27, 2020

KristofferC commented Jan 27, 2020 • edited Loading

StefanKarpinski commented Jan 27, 2020 • edited Loading

lewisl commented Jan 27, 2020 via email • edited by StefanKarpinski Loading

StefanKarpinski commented Jan 27, 2020

tpapp commented Jan 27, 2020

StefanKarpinski commented Jan 27, 2020

wildart commented Jan 29, 2020 • edited Loading

fredrikekre commented Jan 29, 2020 • edited Loading

StefanKarpinski commented Jan 29, 2020

fredrikekre commented Jan 29, 2020 • edited Loading

StefanKarpinski commented Jan 29, 2020

wildart commented Jan 29, 2020

lewisl commented Jan 29, 2020 via email • edited by StefanKarpinski Loading

fredrikekre commented Aug 27, 2020

lewisl commented Aug 27, 2020 via email • edited by StefanKarpinski Loading

KristofferC commented Aug 27, 2020

StefanKarpinski commented Jan 23, 2019 •

edited by KristofferC

Loading

lewisl commented Jul 24, 2019 •

edited

Loading

wildart commented Sep 24, 2019 •

edited

Loading

KristofferC commented Jan 27, 2020 •

edited

Loading

StefanKarpinski commented Jan 27, 2020 •

edited

Loading

lewisl commented Jan 27, 2020 via email •

edited by StefanKarpinski

Loading

wildart commented Jan 29, 2020 •

edited

Loading

fredrikekre commented Jan 29, 2020 •

edited

Loading

fredrikekre commented Jan 29, 2020 •

edited

Loading

lewisl commented Jan 29, 2020 via email •

edited by StefanKarpinski

Loading

lewisl commented Aug 27, 2020 via email •

edited by StefanKarpinski

Loading