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

proposal: instead of build.zig.ini, use build.zig.zon #14290

Closed
Tracked by #14265
andrewrk opened this issue Jan 13, 2023 · 55 comments · Fixed by #14523
Closed
Tracked by #14265

proposal: instead of build.zig.ini, use build.zig.zon #14290

andrewrk opened this issue Jan 13, 2023 · 55 comments · Fixed by #14523
Assignees
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Jan 13, 2023

Extracted from #14265.

Zig Object Notation

Instead of...

[package]
name=libffmpeg
version=5.1.2

[dependency]
name=libz
url=https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz
hash=c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae

[dependency]
name=libmp3lame
url=https://github.com/andrewrk/libmp3lame/archive/497568e670bfeb14ab6ef47fb6459a2251358e43.tar.gz
hash=9ba4f49895b174a3f918d489238acbc146bd393575062b2e3be33488b688e36f

It would look like this:

.{
    .name = "libffmpeg",
    .version = "5.1.2",
    .dependencies = .{
        .libz = .{
            .url = "https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz",
            .hash = "c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae",
        },
        .libmp3lame = .{
            .url = "https://github.com/andrewrk/libmp3lame/archive/497568e670bfeb14ab6ef47fb6459a2251358e43.tar.gz",
            .hash = "9ba4f49895b174a3f918d489238acbc146bd393575062b2e3be33488b688e36f",
        },
    },
}

It also should be renamed with the .zon extension, and furthermore, [package] should be renamed to [project] to match the correct terminology.

Perhaps the . in Zig language itself should be removed, and then it could be like this:

{
    .name = "libffmpeg",
    .version = "5.1.2",
    .dependencies = {
        .libz = {
            .url = "https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz",
            .hash = "c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae",
        },
        .libmp3lame = {
            .url = "https://github.com/andrewrk/libmp3lame/archive/497568e670bfeb14ab6ef47fb6459a2251358e43.tar.gz",
            .hash = "9ba4f49895b174a3f918d489238acbc146bd393575062b2e3be33488b688e36f",
        },
    },
}

which is pretty unoffensive if you ask me. Lots of people hate that dot anyway. I don't know if the parsing would be problematic or not though, would need to investigate.

See competing proposal:

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jan 13, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Jan 13, 2023
@ghost
Copy link

ghost commented Jan 13, 2023

Ok, this isn't quite as bad as I was expecting. Though of course we'd have to provide a reference parser to be taken seriously, and if we write that in Zig, we'd have to make sure it itself has no dependencies.

Also I think removing the dot from dotbrace really would cause parsing problems. Is {} an empty tuple or a void constant? Is { key = "value" } a struct-like tuple or an assignment?

@deflock
Copy link

deflock commented Jan 13, 2023

Maybe I missed something but what is the main profit of using this own DSL (not supported by any tool)?

In yaml:

name: libffmpeg
version: 5.1.2
dependencies:
  libz:
    url: https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz
    hash: c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae
  libmp3lame:
    url: https://github.com/andrewrk/libmp3lame/archive/497568e670bfeb14ab6ef47fb6459a2251358e43.tar.gz
    hash: 9ba4f49895b174a3f918d489238acbc146bd393575062b2e3be33488b688e36f

@ghost
Copy link

ghost commented Jan 13, 2023

@deflock In short, no existing tool satisfies all our requirements. For more detail, see the linked PR.

@andrewrk
Copy link
Member Author

YAML is too complicated and has unacceptable footguns.

@deflock
Copy link

deflock commented Jan 13, 2023

@EleanorNB I've scrolled one more time and I see why json/csv do not satisfy, s-expressions in imperative language is a bit weird :) So remaining candidates are toml, yaml, own format. I see only one profit of using own format - if it will be used directly within build.zig so you don't need additional file for declaring dependencies, but it's not possible bc of declarative reasons.

@batiati
Copy link

batiati commented Jan 13, 2023

image

@andrewrk
Copy link
Member Author

andrewrk commented Jan 13, 2023

@deflock that's a nice point in favor of this proposal that I hadn't considered - similar to how in node.js you can require("foo.json"), it could make sense to support @import("foo.zon") in Zig which would give you comptime access to the data.

@andrewrk
Copy link
Member Author

Another point in favor of this proposal over the TOML one: #14291. Note how line diffs against mirrors field (an array) would be annoying in the TOML case due to both elements being on the same line.

@kuon
Copy link
Contributor

kuon commented Jan 13, 2023

I think this is much better than TOML subset.

Upsides:

  • can be @import as it is valid zig
  • no new format that is TOML but not really
  • allow for reuse of editor plugins
  • could itself support @import, like this
.{
            .url = "https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz",
            .hash = "c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae",
        }
.{
    .name = "libffmpeg",
    .version = "5.1.2",
    .dependencies = .{
        .libz = @import("libz.lulz"),
        .libmp3lame = .{
            .url = "https://github.com/andrewrk/libmp3lame/archive/497568e670bfeb14ab6ef47fb6459a2251358e43.tar.gz",
            .hash = "9ba4f49895b174a3f918d489238acbc146bd393575062b2e3be33488b688e36f",
        },
    },
}
  • perhaps a few functions could be whitelisted (but this needs to be investigated)
  • can be extended without having to make the format or the parser more complicated
  • build.zig could have a package function which would return a struct like this, this can be used to generate the .lulz file with zig code, which leads to the next point
  • this would put some effort into an object notation, which is really a nice supporting tool for a language, it can provide tooling for things like configurations, object format (game state...), rust has ron. I think this would be better than working on a pseudo TOML parser. To say it more plainly, work put into that would be directly put into zig.

Downsides:

  • a bit harder to write
  • further from the left margin
  • people don't like .{ but maybe that dot could be removed, even if I personally like it

Please, don't use YAML, every time I work with that, I lose my sanity over some weird thing

@thejoshwolfe
Copy link
Contributor

why do we have this

    .dependencies = .{
        .libz = .{
            .url = "https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz",
            .hash = "c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae",
        },

instead of this?

    .dependencies = .{
        .{
            .name = "libz",
            .url = "https://github.com/andrewrk/libz/archive/f0e53cc2391741034b144a2c2076ed8a9937b29b.tar.gz",
            .hash = "c9b30cffc40999d2c078ff350cbcee642970a224fe123c756d0892f876cf1aae",
        },

does the former actually work in Zig if there's no struct with a libz: Dependency field defined in it? We don't have hashmap literals in Zig, right?

I would also like to vote against the name "lulz", as that word is often associated with mob harassment and racism.

@andrewrk andrewrk changed the title proposal: instead of build.zig.ini, use build.zig.lulz proposal: instead of build.zig.ini, use build.zig.zon Jan 13, 2023
@andrewrk
Copy link
Member Author

I would also like to vote against the name "lulz", as that word is often associated with mob harassment and racism.

Alright fine, I changed the proposal to the much more conservative and obvious choice of .zon.

@andrewrk
Copy link
Member Author

Alright, enough deliberation. Moving forward with this one; rejecting #14289.

@andrewrk andrewrk added accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. labels Jan 13, 2023
@suhr
Copy link

suhr commented Jan 13, 2023

This reminds me of https://github.com/ron-rs/ron.

@ikskuh
Copy link
Contributor

ikskuh commented Jan 13, 2023

I don't know if the parsing would be problematic or not though, would need to investigate.

That wouldn't be a problem as ZON is a data format, and we only need .{} (a struct/tuple) to be distinguished from a {} (block) in Zig, as we can execute code that might break data from the block.

We don't need to execute that here. But imho, for the sake of consistency, i'd prefer .{ over {, so i can copy-paste the code into Zig.

@thejoshwolfe: I actually prefer .libz = .{ over .{ .name = "libz", as the key syntax implies that the names have to be unique, due to fields have to be unique. The second syntax doesn't convey that information.

@likern
Copy link

likern commented Jan 13, 2023

I proposed #14265 (comment), this proposal alignes to my needs, more or less idea I envisioned. I upvote for them.

The only thing I dislike is build.zig.zon. If we already have custom format called Zig Object Notation just build.zon is better.

build.zig and build.zon ❤️ perfect match.

It looks and feels very much like improved JSON format. Very structered and understandable, but with comments.

@cryptocode
Copy link
Contributor

cryptocode commented Jan 13, 2023

Zon of a gun, this is brilliant ❤️

Will comments be // ?

@17dec
Copy link

17dec commented Jan 13, 2023

Another point in favor of this proposal over the TOML one: #14291. Note how line diffs against mirrors field (an array) would be annoying in the TOML case due to both elements being on the same line.

With the ini format you could have duplicate url keys though, which would achieve the same.

@thejoshwolfe: I actually prefer .libz = .{ over .{ .name = "libz", as the key syntax implies that the names have to be unique, due to fields have to be unique. The second syntax doesn't convey that information.

While I agree that the syntax is nicer, I also believe that if a Zig data format is chosen, it makes sense that the schema can be described by a Zig type and that the data can be loaded directly into said type. Using the key syntax to describe a dynamic map kinda loses both of those properties

@wooster0
Copy link
Contributor

At first I thought this was a new format with a subset of features of normal Zig but as far as I understand it it's just a regular Zig file with a single struct expression that can be imported, which I like very much.
But why do we have to call it build.zig.zon or build.zon? It looks kind of convoluted when we could just call it packages.zig? This could be the standard name for the file containing a struct expression with the packages listed.
build.zig.zon or more recently build.zon indicates to me it is a new separate format (Zig Object Notation) where I can't expect all features from Zig to work, even though that's not the case because it is in fact a normal Zig file (or did I get that wrong?).

build.zon sounds like build.zig; a file that builds my project, but written in "Zig Object Notation"...? Seems rather confusing.

IMO build.zig and packages.zig would be a much clearer and simpler naming.


If however this is actually supposed to be a new format with only a subset of features of normal Zig, why? Why do we need to create a new format when a normal Zig file could also contain this struct expression listing the packages?
It seems like this way there will always be new requests to port features from normal Zig over to this format, duplicating a lot of work.

@tato
Copy link

tato commented Jan 17, 2023

An advantage of build.zig.zon is that the file would always show up next to build.zig in listings sorted by file name.

@likern
Copy link

likern commented Jan 17, 2023

package.zon for defining package and direct (top-level) dependencies, package.lock.zon for fixed transitive (non-direct) dependencies, generated and maintained by package manager only.

Let's reuse knowledge of other package managers (npm as most advanced) which already faced with all kind of subtle bugs and errors.

@silversquirl
Copy link
Contributor

@likern we shouldn't blindly copy other package managers. Looking at their decisions and why they were made is a good idea, but package managers like cargo, npm, etc. were not designed for Zig and differ quite a lot from Zig's package manager.

@michaelbartnett
Copy link
Contributor

Possibly cursed question:

If .zon has the leading .{, should that imply that it could support a leading type annotation? Maybe in a .zton file? (Zig Typed Object Notation). Nice implications for if you need to do polymorphic serialization (thinking about use cases totally unrelated to packaging).

@silversquirl
Copy link
Contributor

@michaelbartnett I think for that you'd use a union-style .{ .foo = .{ ... } } or .{ .bar = .{ ... } }

@Vexu Vexu self-assigned this Jan 18, 2023
@tpmoney
Copy link

tpmoney commented Jan 20, 2023

@likern we shouldn't blindly copy other package managers. Looking at their decisions and why they were made is a good idea, but package managers like cargo, npm, etc. were not designed for Zig and differ quite a lot from Zig's package manager.

Probably most similar in design to what Zig attempts to do (both with the build.zig file and the proposal for .zon to be a zig-ish (if not actually valid zig) is the Swift Package Manager might be some interesting ideas in there.

@lin72h
Copy link

lin72h commented Jan 20, 2023

@tpmoney I feel the same way about the Swift Package Manager format, and Erlang's configuration file also has very similar design, which is a subset of erlang syntax

@likern
Copy link

likern commented Jan 20, 2023

With all that functionality sooner or later we might need some if else branches, different sources for packages (with priority) and more. Seems sooner or later we will need full language support and .zon will not be enough.

I think like for 95-99% of packages .zon is Ok, but for that 1% we need ad-hoc solution with full language support (running code in 100% isolated light and fast VM).

@silversquirl
Copy link
Contributor

silversquirl commented Jan 21, 2023

@likern The whole point of zon is to not execute arbitrary code. See #14290 (comment) and #14290 (comment)

@Vexu Vexu removed their assignment Jan 27, 2023
@andrewrk andrewrk self-assigned this Feb 2, 2023
andrewrk added a commit that referenced this issue Feb 3, 2023
 * improve error message when build manifest file is missing
 * update std.zig.Ast to support ZON
 * Compilation.AllErrors.Message: make the notes field a const slice
 * move build manifest parsing logic into src/Manifest.zig and add more
   checks, and make the checks integrate into the standard error
   reporting code so that reported errors look sexy

closes #14290
andrewrk added a commit that referenced this issue Feb 3, 2023
 * improve error message when build manifest file is missing
 * update std.zig.Ast to support ZON
 * Compilation.AllErrors.Message: make the notes field a const slice
 * move build manifest parsing logic into src/Manifest.zig and add more
   checks, and make the checks integrate into the standard error
   reporting code so that reported errors look sexy

closes #14290
@expikr
Copy link
Contributor

expikr commented Sep 24, 2023

What about “Zig Attribute Grammar”?

alex-courtis added a commit to nvim-tree/nvim-web-devicons that referenced this issue Feb 9, 2024
XI2vGMst added a commit to XI2vGMst/lsd that referenced this issue Aug 21, 2024
ziglang/zig#14290

This is the official file ending of zon files, at the time of writing this only used to specify dependencies, versions and files of zig packages
@mlugg mlugg moved this to Urgent Enhancements in Package Manager Aug 22, 2024
zwpaper pushed a commit to lsd-rs/lsd that referenced this issue Sep 28, 2024
ziglang/zig#14290

This is the official file ending of zon files, at the time of writing this only used to specify dependencies, versions and files of zig packages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.