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

makeBinaryWrapper: breaks downstream derivations which edit wrappers #172592

Closed
LunNova opened this issue May 11, 2022 · 7 comments
Closed

makeBinaryWrapper: breaks downstream derivations which edit wrappers #172592

LunNova opened this issue May 11, 2022 · 7 comments

Comments

@LunNova
Copy link
Member

LunNova commented May 11, 2022

Describe the bug

At least one derivation exists which copies and then edits a wrapper. This no longer works with makeBinaryWrapper.

https://github.com/LavaDesu/powercord-overlay/blob/61458f8d09b977fe4347ef68c1a29b4ac4ae3baf/drvs/discord.nix

Steps To Reproduce

Steps to reproduce the behavior:

  1. Build the linked derivation

Expected behavior

Succeeds

Actual behavior

substituteInPlace fails with:

error: builder for '/nix/store/hhifdwfj3265yq6kgk27dlp4v0i1rvv4-discord-plugged.drv' failed with exit code 1;
       last 1 log lines:
       > consumeEntire(): ERROR: Input null bytes, won't process
@LunNova
Copy link
Member Author

LunNova commented May 11, 2022

Made this issue as I wasn't sure how best to handle this downstream.

@LunNova
Copy link
Member Author

LunNova commented May 11, 2022

The issue with this particular derivation was fixed in #164163 by reverting makeBinaryWrapper, but it looks like in future binary wrappers will be back and it will break again (#172583 plans a fix for variable expansion).

@ncfavier
Copy link
Member

A similar issue came up with the firefox wrapper: #171985. Do you happen to know why exactly editing the wrapper is needed? How does discord decide where to look for its resources?

@ncfavier
Copy link
Member

Ah, forgot discord was closed source. Good luck with that.

@LunNova
Copy link
Member Author

LunNova commented May 11, 2022

Looks like the same fix for the firefox thing applies here.

Adding a generic way of extracting the original arguments to makeWrapper somewhere in pkgs/build-support sound like a good way of dealing with this for the general case? I could extract out the code you added in #171985.

The issue isn't specific to discord, it's just the first thing I ran into.

@ncfavier
Copy link
Member

Ah thanks for reminding me, I wanted to do that. I'll squeeze it into #172366

LunNova added a commit to LunNova/nixos-configs that referenced this issue May 14, 2022
@LunNova
Copy link
Member Author

LunNova commented May 14, 2022

Approach which works regardless of wrapper type:

    if grep '\0' $out/opt/DiscordCanary/DiscordCanary && wrapperCmd=$(${extractCmd} $out/opt/DiscordCanary/DiscordCanary) && [[ $wrapperCmd ]]; then
      # Binary wrapper
      parseMakeCWrapperCall() {
        shift # makeCWrapper
        oldExe=$1; shift
        oldWrapperArgs=("$@")
      }
      eval "parseMakeCWrapperCall ''${wrapperCmd//"${discord-canary.out}"/"$out"}"
      makeWrapper $oldExe $out/opt/DiscordCanary/DiscordCanary "''${oldWrapperArgs[@]}" --add-flags "${extraElectronArgs}"
    else
      # Normal wrapper
      substituteInPlace $out/opt/DiscordCanary/DiscordCanary \
      --replace '${discord-canary.out}' "$out" \
      --replace '"$@"' '${extraElectronArgs} "$@"'
    fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants