feat: C template #27

Merged
Sigmanificient merged 7 commits from c into main 2024-05-14 20:24:13 +00:00
Sigmanificient commented 2024-05-06 21:43:20 +00:00 (Migrated from github.com)

A small flake template for a C project using Makefile as build system.
The boilerplate code is a simple hello program.

Based of #26

  • built on x86-64_linux
  • ran nix flake check --all-systems
A small flake template for a C project using Makefile as build system. The boilerplate code is a simple hello program. Based of #26 - [x] built on `x86-64_linux` - [x] ran `nix flake check --all-systems`
TheCodedProf (Migrated from github.com) reviewed 2024-05-06 21:43:20 +00:00
getchoo (Migrated from github.com) reviewed 2024-05-06 21:43:20 +00:00
Austreelis (Migrated from github.com) reviewed 2024-05-06 21:43:20 +00:00
isabelroses (Migrated from github.com) reviewed 2024-05-06 22:17:10 +00:00
isabelroses (Migrated from github.com) commented 2024-05-06 22:13:05 +00:00

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses (Migrated from github.com) commented 2024-05-06 22:16:46 +00:00
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
@ -0,0 +17,4 @@
default = pkgs.mkShell { inputsFrom = [ packages.${pkgs.system}.hello ]; };
});
packages = forAllSystems (pkgs: rec {
isabelroses (Migrated from github.com) commented 2024-05-06 22:16:27 +00:00

Can we move the package defection to a file called default.nix

Can we move the package defection to a file called default.nix
@ -0,0 +21,4 @@
default = hello;
hello = pkgs.callPackage ./hello.nix { };
});
isabelroses (Migrated from github.com) commented 2024-05-06 22:16:50 +00:00

 overlays.default = final: prev: {
   hello = final.callPackage ./default.nix {};
 };

```suggestion overlays.default = final: prev: { hello = final.callPackage ./default.nix {}; }; ```
TheCodedProf (Migrated from github.com) reviewed 2024-05-06 22:18:25 +00:00
TheCodedProf (Migrated from github.com) commented 2024-05-06 22:15:23 +00:00

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient (Migrated from github.com) reviewed 2024-05-06 22:19:31 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-06 22:19:31 +00:00

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
Sigmanificient (Migrated from github.com) reviewed 2024-05-06 22:20:00 +00:00
@ -0,0 +17,4 @@
default = pkgs.mkShell { inputsFrom = [ packages.${pkgs.system}.hello ]; };
});
packages = forAllSystems (pkgs: rec {
Sigmanificient (Migrated from github.com) commented 2024-05-06 22:20:00 +00:00

Can we move the package defection to a file called default.nix

Is there a need to split the flake? it is quite small

> Can we move the package defection to a file called default.nix Is there a need to split the flake? it is quite small
Sigmanificient (Migrated from github.com) reviewed 2024-05-06 22:20:26 +00:00
@ -0,0 +21,4 @@
default = hello;
hello = pkgs.callPackage ./hello.nix { };
});
Sigmanificient (Migrated from github.com) commented 2024-05-06 22:20:26 +00:00

What does this do?

What does this do?
isabelroses (Migrated from github.com) reviewed 2024-05-06 22:36:00 +00:00
@ -0,0 +17,4 @@
default = pkgs.mkShell { inputsFrom = [ packages.${pkgs.system}.hello ]; };
});
packages = forAllSystems (pkgs: rec {
isabelroses (Migrated from github.com) commented 2024-05-06 22:36:00 +00:00

It allows it to have old nix compatibility. that’s the main thing for me. Also prevents redeclaring the package a few times.

It allows it to have old nix compatibility. that’s the main thing for me. Also prevents redeclaring the package a few times.
isabelroses (Migrated from github.com) reviewed 2024-05-06 22:37:19 +00:00
@ -0,0 +21,4 @@
default = hello;
hello = pkgs.callPackage ./hello.nix { };
});
isabelroses (Migrated from github.com) commented 2024-05-06 22:37:19 +00:00

this allows the user to consume the overlay and it will be added the nixpkgs list. so i could use enviroment.systemPackages = [ pkgs.hello ]; instead.

this allows the user to consume the overlay and it will be added the nixpkgs list. so i could use `enviroment.systemPackages = [ pkgs.hello ];` instead.
Sigmanificient (Migrated from github.com) reviewed 2024-05-06 22:38:42 +00:00
@ -0,0 +21,4 @@
default = hello;
hello = pkgs.callPackage ./hello.nix { };
});
Sigmanificient (Migrated from github.com) commented 2024-05-06 22:38:42 +00:00

doesn't this cause conflict with the nixpkgs's hello pkgs?

doesn't this cause conflict with the nixpkgs's [hello](https://github.com/NixOS/nixpkgs/blob/nixos-23.11/pkgs/by-name/he/hello/package.nix#L34) pkgs?
isabelroses (Migrated from github.com) approved these changes 2024-05-06 22:53:03 +00:00
getchoo (Migrated from github.com) requested changes 2024-05-07 06:38:40 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 06:16:38 +00:00

this can be commited, as long as it doesn't assume nix is installed. this can work for example

if has nix_direnv_version; then
  use flake
fi
```suggestion ``` this can be commited, as long as it doesn't assume nix is installed. this can work for example ``` if has nix_direnv_version; then use flake fi ```
getchoo (Migrated from github.com) commented 2024-05-07 06:16:48 +00:00
result*
repl-result-*
```suggestion result* repl-result-* ```
getchoo (Migrated from github.com) commented 2024-05-07 06:37:19 +00:00

these are all unused

```suggestion ``` these are all unused
getchoo (Migrated from github.com) commented 2024-05-07 06:38:00 +00:00

when using cmake or meson with github's template, these are not necessary as we have a standard build directory and file extensions are already matched

```suggestion ``` when using cmake or meson with github's template, these are not necessary as we have a standard build directory and file extensions are already matched
getchoo (Migrated from github.com) commented 2024-05-07 06:20:12 +00:00

why assume this?

```suggestion ``` why assume this?
getchoo (Migrated from github.com) commented 2024-05-07 06:21:18 +00:00

there's no reason to use these by default. it will only be confusing if anyone shares a locally built binary

```suggestion ``` there's no reason to use these by default. it will only be confusing if anyone shares a locally built binary
getchoo (Migrated from github.com) commented 2024-05-07 06:31:09 +00:00

make is in stdenv, so i'm not sure we need this? also if you were to switch to meson/cmake, this would need to be replaced with one of these instead

nativeBuildInputs = [ cmake ];
nativeBuildInputs = [ meson ninja ];
```suggestion ``` `make` is in stdenv, so i'm not sure we need this? also if you were to switch to meson/cmake, this would need to be replaced with one of these instead ```nix nativeBuildInputs = [ cmake ]; ``` ```nix nativeBuildInputs = [ meson ninja ]; ```
getchoo (Migrated from github.com) commented 2024-05-07 06:33:08 +00:00
  installPhase = ''
    runHook preInstall

    install -Dm755 hello $out/bin/hello
    
    runHook postInstall
  '';

this is also another reason to use cmake or meson: this phase wouldn't be required

```suggestion installPhase = '' runHook preInstall install -Dm755 hello $out/bin/hello runHook postInstall ''; ``` this is also another reason to use cmake or meson: this phase wouldn't be required
getchoo (Migrated from github.com) commented 2024-05-07 06:24:08 +00:00
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo (Migrated from github.com) commented 2024-05-07 06:24:50 +00:00

im not really sure why this is set?

im not really sure why this is set?
getchoo (Migrated from github.com) commented 2024-05-07 06:25:15 +00:00
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo (Migrated from github.com) commented 2024-05-07 06:26:34 +00:00
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo (Migrated from github.com) commented 2024-05-07 06:27:21 +00:00

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
@ -0,0 +1,27 @@
{
description = "Aux template for C project";
inputs.nixpkgs.url = "github:auxolotl/nixpkgs/nixos-unstable";
getchoo (Migrated from github.com) commented 2024-05-07 06:35:48 +00:00
  inputs.nixpkgs.url = "github:auxolotl/nixpkgs";

we aren't actually updating branches besides master currently

```suggestion inputs.nixpkgs.url = "github:auxolotl/nixpkgs"; ``` we aren't actually updating branches besides master currently
getchoo (Migrated from github.com) commented 2024-05-07 06:29:54 +00:00
#include <stdio.h>

int main(int argc, char *argv[]) {
    printf("Hello from Aux!\n");
    return 0;
}

we really don't need to overcomplicate this

```suggestion #include <stdio.h> int main(int argc, char *argv[]) { printf("Hello from Aux!\n"); return 0; } ``` we really don't need to overcomplicate this
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 08:05:34 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-07 07:18:21 +00:00

Only the .pre-commit-config.yaml is unused so I'll remove it.

I'm not sure why do you want to include entries unrelated to this template in the gitignore but also removing important ignore like .cache or compile_commands.json that are useful, it seems like a contradiction?

Only the `.pre-commit-config.yaml` is unused so I'll remove it. I'm not sure why do you want to include entries unrelated to this template in the gitignore but also removing important ignore like `.cache` or `compile_commands.json` that are useful, it seems like a contradiction?
Sigmanificient (Migrated from github.com) commented 2024-05-07 07:20:22 +00:00

Committing a .envrc doesn't seems like a good practice to me.
I can remove that entry tho, so it's up to the user to decide was to do i they use direnv

Committing a `.envrc` doesn't seems like a good practice to me. I can remove that entry tho, so it's up to the user to decide was to do i they use direnv
Sigmanificient (Migrated from github.com) commented 2024-05-07 07:22:29 +00:00

I cant find any public gitignore using repl-result-*, so it sound like something we may not need

I cant find any public gitignore using `repl-result-*`, so it sound like something we may not need
Sigmanificient (Migrated from github.com) commented 2024-05-07 07:55:13 +00:00

I think that C99 is a very good standard to based a template on

I think that C99 is a very good standard to based a template on
@ -0,0 +1,27 @@
{
description = "Aux template for C project";
inputs.nixpkgs.url = "github:auxolotl/nixpkgs/nixos-unstable";
Sigmanificient (Migrated from github.com) commented 2024-05-07 08:02:20 +00:00

master template use it too.

master template use it too.
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 08:11:04 +00:00
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 08:20:21 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-07 08:20:21 +00:00

write is lighter than printf. I think it is better to use it, and i am not sure by what means it is overcomplicated?

`write` is lighter than `printf`. I think it is better to use it, and i am not sure by what means it is overcomplicated?
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 08:25:22 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-07 08:25:22 +00:00

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 08:35:26 +00:00
Sigmanificient commented 2024-05-07 08:49:22 +00:00 (Migrated from github.com)

I went through your suggestion and made what i think is a good tradeoff between our two positions

I went through your suggestion and made what i think is a good tradeoff between our two positions
Austreelis (Migrated from github.com) reviewed 2024-05-07 09:17:17 +00:00
Austreelis (Migrated from github.com) commented 2024-05-07 09:17:17 +00:00

I agree with getchoo.
I think defining macros when it's arguably not needed is overcomplicating it. Also, I think we really don't care about the performance of printf vs write here. Getchoo's suggestion is the canonical hello world C program, it doesn't bring in any header other than stdlib, it doesn't define any item besides main (which you probably need), and it's recognizable so people know they should scrap it anyway.

I feel like the template should focus on nix code, and completely disregard C code (unless it demonstrates something on the nix side). I'd even be fine with an empty source file if that compiled.

I agree with getchoo. I think defining macros when it's arguably not needed is overcomplicating it. Also, I think we really don't care about the performance of printf vs write here. Getchoo's suggestion is the canonical hello world C program, it doesn't bring in any header other than stdlib, it doesn't define any item besides main (which you probably need), and it's recognizable so people know they should scrap it anyway. I feel like the template should focus on nix code, and completely disregard C code (unless it demonstrates something on the nix side). I'd even be fine with an empty source file if that compiled.
getchoo (Migrated from github.com) reviewed 2024-05-07 09:21:23 +00:00
getchoo (Migrated from github.com) reviewed 2024-05-07 09:22:13 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:22:12 +00:00

yeah this would be better removed

yeah this would be better removed
getchoo (Migrated from github.com) reviewed 2024-05-07 09:22:43 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:22:43 +00:00

I cant find any public gitignore using repl-result-*, so it sound like something we may not need

if you use :bl in a repl, you do. this is a known nix artifact, same as result*

> I cant find any public gitignore using repl-result-*, so it sound like something we may not need if you use `:bl` in a repl, you do. this is a known nix artifact, same as `result*`
getchoo (Migrated from github.com) reviewed 2024-05-07 09:38:09 +00:00
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 09:43:45 +00:00
getchoo (Migrated from github.com) reviewed 2024-05-07 09:44:51 +00:00
getchoo (Migrated from github.com) reviewed 2024-05-07 09:45:21 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:45:20 +00:00

i feel this is once again putting a lot of focus on what is really just throwaway example code. i would really take a look at https://github.com/NixOS/templates/tree/master/c-hello as a reference

most people also aren't using c99 anymore. i feel this is just your personal taste here

> i feel this is once again putting a lot of focus on what is really just throwaway example code. i would really take a look at https://github.com/NixOS/templates/tree/master/c-hello as a reference most people also aren't using c99 anymore. i feel this is just your personal taste here
getchoo (Migrated from github.com) reviewed 2024-05-07 09:49:10 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:49:10 +00:00

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
getchoo (Migrated from github.com) reviewed 2024-05-07 09:51:14 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:51:14 +00:00

write is lighter than printf

i feel this is once again putting a lot of focus on what is really just throwaway example code. i would really take a look at https://github.com/NixOS/templates/tree/master/c-hello as a reference

and i am not sure by what means it is overcomplicated?

i don't think we're really going to get anywhere with this question being asked, so i would rather not continue with this

> write is lighter than printf >> i feel this is once again putting a lot of focus on what is really just throwaway example code. i would really take a look at https://github.com/NixOS/templates/tree/master/c-hello as a reference > and i am not sure by what means it is overcomplicated? i don't think we're really going to get anywhere with this question being asked, so i would rather not continue with this
getchoo (Migrated from github.com) reviewed 2024-05-07 09:51:25 +00:00
@ -0,0 +1,27 @@
{
description = "Aux template for C project";
inputs.nixpkgs.url = "github:auxolotl/nixpkgs/nixos-unstable";
getchoo (Migrated from github.com) commented 2024-05-07 09:51:25 +00:00

this should changed as well

this should changed as well
getchoo (Migrated from github.com) reviewed 2024-05-07 09:53:53 +00:00
getchoo (Migrated from github.com) commented 2024-05-07 09:53:53 +00:00

important ignore like .cache or compile_commands.json

the only reason they are important is because of the specific way you have this set up. other projects are very unlikely to use these, and i would say they shouldn't be used here either

> important ignore like .cache or compile_commands.json the only reason they are important is because of the specific way you have this set up. other projects are very unlikely to use these, and i would say they shouldn't be used here either
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 09:56:23 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-07 09:56:23 +00:00

i genuinely don't get it, but i changed the implementation to use a regular printf

i genuinely don't get it, but i changed the implementation to use a regular `printf`
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 10:06:12 +00:00
Sigmanificient (Migrated from github.com) commented 2024-05-07 10:06:12 +00:00

This is the same scenario with the other GitHub ignore you wanted me to include, I think either both should be put or none, because it doesn't seem logic to add one and not the other

This is the same scenario with the other GitHub ignore you wanted me to include, I think either both should be put or none, because it doesn't seem logic to add one and not the other
Sigmanificient (Migrated from github.com) reviewed 2024-05-07 21:30:30 +00:00
@ -0,0 +1,27 @@
{
description = "Aux template for C project";
inputs.nixpkgs.url = "github:auxolotl/nixpkgs/nixos-unstable";
Sigmanificient (Migrated from github.com) commented 2024-05-07 21:30:30 +00:00

This might need it's own dedicated issue

This might need it's own dedicated issue
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: auxolotl/templates#27
No description provided.