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

remind: 05.01.01 -> 05.02.01 #368389

Closed
wants to merge 1 commit into from
Closed

Conversation

r-ryantm
Copy link
Contributor

Automatic update generated by nixpkgs-update tools. This update was made based on information from passthru.updateScript.

meta.description for remind is: Sophisticated calendar and alarm program for the console

meta.homepage for remind is: https://dianne.skoll.ca/projects/remind/

Updates performed
  • Ran passthru.UpdateScript
To inspect upstream changes
Impact

Checks done


  • built on NixOS
  • The tests defined in passthru.tests, if any, passed
  • found 05.02.01 with grep in /nix/store/nhx7hv5gnpaxfbplnxgw706v33w79p3h-remind-05.02.01
  • found 05.02.01 in filename of file in /nix/store/nhx7hv5gnpaxfbplnxgw706v33w79p3h-remind-05.02.01

Rebuild report (if merged into master) (click to expand)
3 total rebuild path(s)

3 package rebuild(s)

First fifty rebuilds by attrpath

remind
wyrd
Instructions to test this update (click to expand)

Either download from the cache:

nix-store -r /nix/store/nhx7hv5gnpaxfbplnxgw706v33w79p3h-remind-05.02.01 \
  --option binary-caches 'https://cache.nixos.org/ https://nixpkgs-update-cache.nix-community.org/' \
  --option trusted-public-keys '
  nixpkgs-update-cache.nix-community.org-1:U8d6wiQecHUPJFSqHN9GSSmNkmdiFW7GW7WNAnHW0SM=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The nixpkgs-update cache is only trusted for this store-path realization.)
For the cached download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.

Or, build yourself:

nix-build -A remind https://github.com/r-ryantm/nixpkgs/archive/513f038405191d1cba6333016d258e07127bbd35.tar.gz

Or:

nix build github:r-ryantm/nixpkgs/513f038405191d1cba6333016d258e07127bbd35#remind

After you've downloaded or built it, look at the files and if there are any, run the binaries:

ls -la /nix/store/nhx7hv5gnpaxfbplnxgw706v33w79p3h-remind-05.02.01
ls -la /nix/store/nhx7hv5gnpaxfbplnxgw706v33w79p3h-remind-05.02.01/bin


Pre-merge build results

We have automatically built all packages that will get rebuilt due to
this change.

This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review


x86_64-linux

✅ 2 packages built:
  • remind
  • wyrd

Maintainer pings

cc @afh @7c6f434c @KoviRobi for testing.

Tip

As a maintainer, if your package is located under pkgs/by-name/*, you can comment @NixOS/nixpkgs-merge-bot merge to automatically merge this update using the nixpkgs-merge-bot.


Add a 👍 reaction to pull requests you find important.

@afh
Copy link
Member

afh commented Dec 26, 2024

Building this PR locally fails for me with the error:

> clang: error: unsupported option '-ffat-lto-objects' for target 'arm64-apple-darwin'

While upstream is aware of the issue (see this commit), it seems the fix does not work for nixpkgs…

A workaround I can think of is:

diff --git a/pkgs/by-name/re/remind/package.nix b/pkgs/by-name/re/remind/package.nix
index 751b418c59d2..b62a18a0c5e7 100644
--- a/pkgs/by-name/re/remind/package.nix
+++ b/pkgs/by-name/re/remind/package.nix
@@ -40,6 +40,8 @@ tcl.mkTclDerivation rec {
       --replace-fail 'set Remind "remind"' "set Remind \"$out/bin/remind\"" \
       --replace-fail 'set Rem2PS "rem2ps"' "set Rem2PS \"$out/bin/rem2ps\"" \
       --replace-fail 'set Rem2PDF "rem2pdf"' "set Rem2PDF \"$out/bin/rem2pdf\""
+    substituteInPlace configure \
+      --replace-fail 'f=-ffat-lto-objects' ""
   '';
 
   env.NIX_CFLAGS_COMPILE = lib.optionalString stdenv.hostPlatform.isDarwin (toString [

Can someone think of a more elegant way to solve this or is aware of a known underlying issue within nixpkgs?

@KoviRobi
Copy link
Contributor

I think the upstream fix should have worked -- can you give more details on how you tried to apply it/log output? Because AFAIK (but not an autoconf expert), it should have changed the test of -ffat-lto-objects to fail on error during the ./configure phase. If you build with --keep-failed, does the env-vars contain -ffat-lto-objects? I suspect not, if your substituteInPlace works

@ofborg ofborg bot requested review from 7c6f434c, afh and KoviRobi December 26, 2024 21:40
@afh
Copy link
Member

afh commented Jan 8, 2025

@KoviRobi given that the upstream fix is from 2 years ago I did not try to apply it as it is already contained in the source archive.

I'm logging build output using: nix build -L .#remind and see

…
remind> checking whether clang supports -flto=auto... yes
remind> checking whether clang supports -ffat-lto-objects... yes
…

I'm leaning towards creating a new PR that contains the proposed workaround above and updates remind to the recently published version 5.02.02. Would you agree that that is a viable path forward?

@afh
Copy link
Member

afh commented Jan 12, 2025

Closing in favor of #373286

@afh afh closed this Jan 12, 2025
@r-ryantm r-ryantm deleted the auto-update/remind branch January 13, 2025 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants