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

No temp nuget.config when NuGet 4.8 or higher #7997

Merged
merged 3 commits into from
Aug 14, 2018

Conversation

satbai
Copy link
Contributor

@satbai satbai commented Aug 9, 2018

No description provided.

@satbai satbai requested a review from aldoms August 9, 2018 17:41
@satbai satbai force-pushed the users/satbai/externalEndpointJson branch from 2666827 to 959db5c Compare August 9, 2018 17:44
@@ -30,6 +37,7 @@ function prepareNuGetExeEnvironment(
let env: EnvironmentDictionary = {};
let originalCredProviderPath: string = null;
let envVarCredProviderPathV2: string = null;
let noTempConfigFile: string = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove


const externalCredentials: string = JSON.stringify(enpointCredentialsJson);
return externalCredentials;
}

This comment was marked as resolved.

@@ -163,19 +170,30 @@ export async function run(nuGetPath: string): Promise<void> {
}
}

// Setting creds in the temp NuGet.config if needed
await nuGetConfigHelper.setAuthForSourcesInTempNuGetConfigAsync();
tl.debug('nuGetConfigPath'); // satus debugging
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove

env["VSS_NUGET_URI_PREFIXES"] = authInfo.internalAuthInfo.uriPrefixes.join(";")
}

let externalCredentials = buildCredentialJson(authInfo);
Copy link
Contributor Author

@satbai satbai Aug 9, 2018

Choose a reason for hiding this comment

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

move this inside the V2 cred provider check

await nuGetConfigHelper.setAuthForSourcesInTempNuGetConfigAsync();
tl.debug('Setting nuget.config auth');
} else {
tl.debug('Not setting temp nuget.config auth');
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a message saying the new provider will handle auth, or something similar. So we know why we're not adding it.

Copy link
Contributor

@aldoms aldoms left a comment

Choose a reason for hiding this comment

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

Left some comments, looks good overall!

@satbai satbai merged commit a6b3db8 into master Aug 14, 2018
@satbai satbai deleted the users/satbai/externalEndpointJson branch August 14, 2018 15:47
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.

2 participants