Skip to content

Conversation

m1dugh
Copy link
Contributor

@m1dugh m1dugh commented Feb 11, 2025

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@m1dugh m1dugh added the 8.has: package (new) This PR adds a new package label Feb 11, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 11, 2025
Copy link
Member

@ethancedwards8 ethancedwards8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM. Thanks!

@m1dugh m1dugh force-pushed the add-reflex-3 branch 2 times, most recently from dbaf7ae to bcf2ec6 Compare February 12, 2025 09:25
@m1dugh
Copy link
Contributor Author

m1dugh commented Feb 12, 2025

hello @ethancedwards8 , it should be fixed.

@m1dugh
Copy link
Contributor Author

m1dugh commented Feb 14, 2025

hello @Tert0 , it should be fixed.

@m1dugh m1dugh requested a review from Tert0 February 14, 2025 14:35
@Tert0
Copy link
Member

Tert0 commented Feb 14, 2025

Some suggestions:

diff --git a/pkgs/by-name/re/re-flex_3/package.nix b/pkgs/by-name/re/re-flex_3/package.nix
index 8b2a9eb787a9..491fc6ca74e2 100644
--- a/pkgs/by-name/re/re-flex_3/package.nix
+++ b/pkgs/by-name/re/re-flex_3/package.nix
@@ -1,7 +1,9 @@
 {
   lib,
-  fetchFromGitHub,
   stdenv,
+  fetchFromGitHub,
+  versionCheckHook,
+  nix-update-script,
 }:
 
 stdenv.mkDerivation (finalAttrs: {
@@ -23,9 +25,16 @@ stdenv.mkDerivation (finalAttrs: {
     "dev"
   ];
 
+  nativeInstallCheckInputs = [ versionCheckHook ];
+  doInstallCheck = true;
+  versionCheckProgram = "${placeholder "bin"}/bin/reflex";
+  versionCheckProgramArg = "--version";
+
+  passthru.updateScript = nix-update-script {};
+
   meta = {
     description = ''
-      High-performance C++ regex library and lexical analyzer generator with Unicode support.
+      High-performance C++ regex library and lexical analyzer generator with Unicode support
     '';
     longDescription = ''
       High-performance C++ regex library and lexical analyzer generator with Unicode support.
@@ -33,11 +42,11 @@ stdenv.mkDerivation (finalAttrs: {
       Seamlessly integrates with Bison and other parsers.
     '';
     homepage = "https://www.genivia.com/reflex.html";
+    changelog = "https://github.com/Genivia/RE-flex?tab=readme-ov-file#changelog";
     license = lib.licenses.bsd3;
     platforms = lib.platforms.all;
     maintainers = with lib.maintainers; [ m1dugh ];
     mainProgram = "reflex";
-    changelog = "https://github.com/Genivia/RE-flex?tab=readme-ov-file#changelog";
   };
 
 })

@m1dugh
Copy link
Contributor Author

m1dugh commented Feb 17, 2025

hello @Tert0 , If I recall well nix-update-script is unnecessary, and the default option if put in pkgs/by-name/.
The rest should be good.

@Tert0
Copy link
Member

Tert0 commented Feb 17, 2025

Does not seem to be the case:

nix-shell maintainers/scripts/update.nix --argstr path re-flex_3
Going to be running update for following packages:

Press Enter key to continue...

Running update for:

Packages updated!

After adding nix-update-script:

nix-shell maintainers/scripts/update.nix --argstr path re-flex_3
Going to be running update for following packages:
 - reflex-3.2.11

Press Enter key to continue...

Running update for:
 - reflex-3.2.11: UPDATING ...
 - reflex-3.2.11: DONE.

Packages updated!

The overlays does not change the packages:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/by-name-overlay.nix

@m1dugh
Copy link
Contributor Author

m1dugh commented Feb 17, 2025

Ok, seems strange as I already received nixpkgs update bot MR for other packages without this line.

@m1dugh
Copy link
Contributor Author

m1dugh commented Feb 17, 2025

Should be done @Tert0

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference to https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/re/re-flex/package.nix and why do we need an older version?

@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants