Wow, I'm the worst. I doubt anyone wants it to be foo. I don't rely on it, and if nobody else is using it in nixpkgs, it seems like a safe bet to just take out the default. I feel bad for leaving that in by accident!

@copumpkin what do you think of using baseNameOf s3url as the default instead? I want to keep a default to avoid breaking back-compat. IMO it's being used outside of nixpkgs in private repositories.

Sure, baseNameOf sounds good. If you do think it's being used widely though, changing the default is pretty awkward. Not really sure what to do about it though...

lmao. It happens! No worries :). But LOL 😁

Nice! TIL that fetchs3 is in nixpkgs. At LumiGuide I've been using the following for some time:

{
  fetchS3 = {path, sha256, accessKeyId, secretAccessKey, region}:
    previous.runCommand (baseNameOf path) {
      inherit path;
      AWS_ACCESS_KEY_ID     = accessKeyId;
      AWS_SECRET_ACCESS_KEY = secretAccessKey;
      AWS_DEFAULT_REGION    = region;
      outputHashMode = "flat";
      outputHashAlgo = "sha256";
      outputHash     = sha256;
    } ''${final.awscli}/bin/aws s3 cp "s3://$path" "$out"'';
}

I'll be replacing that with fetchs3. But first a few issues need to be fixed:

• If I specify credentials but don't specify session_token I get the following error:

error: attribute 'session_token' missing, at 
nixpkgs/pkgs/build-support/fetchs3/default.nix:16:25

we should probably define credentialAttrs as follows:

let
  credentialAttrs = stdenvNoCC.lib.optionalAttrs (credentials != null) (with credentials; {
    AWS_ACCESS_KEY_ID = access_key_id;
    AWS_SECRET_ACCESS_KEY = secret_access_key;
    AWS_SESSION_TOKEN = session_token ? null;
  });
in 

• I see that the region argument is not used. Shouldn't the following environment variable be set: AWS_DEFAULT_REGION = region;?

@basvandijk thanks, integrated your feedback. Wanna try it out?

Thanks. I successfully executed a command like:

$ nix-build -E 'let pkgs = import ./. {}; in pkgs.fetchs3 {s3url = "s3://xxx"; sha256="xxx"; region = "eu-central-1"; credentials = {access_key_id=xxx; secret_access_key=xxx;};}'  

Nice, thanks! I only use temporary credentials which is how I missed the accidentally required session token 😄

Also, I think the region argument here just dictates which regional endpoint you hit with GetBucketLocation, right? After it knows what region your bucket is in, it'll just use that regardless of what you specify.

One last thing though: in my use case, I don't have a multi-user store so I don't care about the .drvs containing my credentials (and as I said, they're temporary, so it doesn't even matter), but some might find it unexpected that their credentials are in the .drv. Some might prefer impureEnvVars for that reason. Do any of you care or shall we just put this in like this for now?

I don't know about impureEnvVars or how it works but it sounds like a good idea to avoid storing credentials in .drvs.

sounds good, threw in a preferLocalBuild for the good form as well

hmm, getting "fatal error: An error occurred (400) when calling the HeadObject operation: Bad Request" when using the impureEnvVars

I will just get rid of it for now. If you pass them as attributes then it's already in the nix store as build inputs anyways.

Oh, one thing I noticed with impureEnvVars in the past is that they always set an env var even if it's unset in the parent context (in which case it's set to empty). An empty session token will probably screw you up. In the past I've had to explicitly unset empty variables. Should probably be fixed in Nix 😄

yeah that's a weird design decision, I don't expect impureEnvVars to touch the environment :)

also pushed as 7a37ed5 in release-18.03