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

fix(lib/ops): ensure the getName contract is uphold for operables #331

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

blaggacao
Copy link
Collaborator

@blaggacao blaggacao commented Jul 13, 2023

fix: #318

Context

Operables wrap packages, and then they are symlinked via lib.getExe into
the container.

  /* Get the path to the main program of a derivation with either
     meta.mainProgram or pname or name

     Type: getExe :: derivation -> string

     Example:
       getExe pkgs.hello
       => "/nix/store/g124820p9hlv4lj8qplzxw1c44dxaw1k-hello-2.12/bin/hello"
       getExe pkgs.mustache-go
       => "/nix/store/am9ml4f4ywvivxnkiaqwr0hyxka1xjsf-mustache-go-1.3.0/bin/mustache"
  */
  getExe = x:
    "${lib.getBin x}/bin/${x.meta.mainProgram or (lib.getName x)}";

Prior to this commit, using package.name (which is the wrong attribute, anyways),
we where furnishing that name to /bin/<name>.
While lib.getBin reads the store path verbatim and thereby honors that name,
lib.getName resolves differently via the following implementation resulting
in naming mismatches for the call to getExe which ultimately linked
/bin/entrypoint in mkOCI to a non-existing exectuable:

  /* This function takes an argument that's either a derivation or a
     derivation's "name" attribute and extracts the name part from that
     argument.

     Example:
       getName "youtube-dl-2016.01.01"
       => "youtube-dl"
       getName pkgs.youtube-dl
       => "youtube-dl"
  */
  getName = x:
   let
     parse = drv: (parseDrvName drv).name;
   in if isString x
      then parse x
      else x.pname or (parse x.name);

Solution

Feed to the consumer (getExe) it's very own contract (getName)


@Pegasust would you have the ability to test this fix in your particular situation?

@Pegasust
Copy link
Contributor

Will look into this

@Pegasust
Copy link
Contributor

Confirmed that this solves my use case for #318

@blaggacao blaggacao merged commit 17dc4eb into main Jul 14, 2023
4 checks passed
@blaggacao blaggacao deleted the towards-318 branch July 14, 2023 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Std ops.mkStandardOCI compatibility problem with ops.mkOperable to wrap
2 participants