close
Skip to content

Do not overwrite users environment variables - #115455

Merged
Ladislau Szomoru (lszomoru) merged 1 commit into
microsoft:mainfrom
Backfighter:patch-1
Mar 21, 2022
Merged

Do not overwrite users environment variables#115455
Ladislau Szomoru (lszomoru) merged 1 commit into
microsoft:mainfrom
Backfighter:patch-1

Conversation

@Backfighter

@Backfighter Backfighter commented Jan 30, 2021

Copy link
Copy Markdown
Contributor

These environment variables are currently enforced by vscode and there is no way to overwrite them. Turning this arround allows the user to use a custom GIT_ASKPASS inside vscode (e.g. /usr/bin/ksshaskpass for kde wallet).

This PR fixes #111839

@joaomoreno

João Moreno (joaomoreno) commented Feb 1, 2021

Copy link
Copy Markdown
Contributor

The current code prevents accidental usage of that environment variable and breaking auth. So I'd rather we put this behind a configuration setting, given it's such a niche scenario. I would also only move the GIT_ASKPASS env around, not the other two.

@ghost

Deleted user (ghost) commented Feb 1, 2021 •
edited by ghost
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@Backfighter

Backfighter commented Feb 1, 2021

Copy link
Copy Markdown
Contributor Author

João Moreno (@joaomoreno) I have adjusted my changes accordingly.

@joaomoreno
João Moreno (joaomoreno) changed the base branch from master to main February 15, 2021 08:51
@Backfighter

Backfighter commented Mar 7, 2021

Copy link
Copy Markdown
Contributor Author

Eric Amodio (@eamodio) What is the status of the merge? Is there anything I can do to speed up the process?

@MCterra10

James (MCterra10) commented Dec 22, 2021

Copy link
Copy Markdown

Can we have this merged? It's incredibly frustrating

MCterra10
James (MCterra10) reviewed Dec 28, 2021
View reviewed changes
Comment thread extensions/git/package.nls.json Outdated
@Backfighter

Backfighter commented Dec 28, 2021

Copy link
Copy Markdown
Contributor Author

James (@MCterra10) thanks for spotting the typo. I have rebased my changes onto the new mainline so it can be merged easily.

Ladislau Szomoru (@lszomoru) any chance this can be merged?

@MCterra10

James (MCterra10) commented Feb 1, 2022

Copy link
Copy Markdown

Backfighter conflicts?

@Backfighter
Allow disabling integrated askpass
01ffd21 
The GIT_ASKPASS variable is currently enforced by vscode and there is no way to overwrite it.
This commit adds an options to disable the integrated askpass and use
your own.

Fixes microsoft#111839
@MCterra10

James (MCterra10) commented Mar 16, 2022 •
edited
Loading

Copy link
Copy Markdown

are the maintainers always this slow for such simple changes?
it's not like there's the "they are volunteers" argument, this is literally a paid job to maintain the vscode repo

@lszomoru
Ladislau Szomoru (lszomoru) merged commit b8a5e18 into microsoft:main Mar 21, 2022
@lszomoru

Ladislau Szomoru (lszomoru) commented Mar 21, 2022

Copy link
Copy Markdown
Member

Apologies for the delay on getting this in. This should be in tomorrow's Insiders release and the next Stable release.

@Backfighter
Backfighter deleted the patch-1 branch March 21, 2022 21:46
@MCterra10

James (MCterra10) commented Apr 1, 2022

Copy link
Copy Markdown

sweeeeet

@github-actions github-actions Bot locked and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@MCterra10 James (MCterra10) MCterra10 left review comments
@joaomoreno João Moreno (joaomoreno) joaomoreno approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lszomoru Ladislau Szomoru (lszomoru)

Labels

None yet

Projects

None yet

Milestone

March 2022

Development

Successfully merging this pull request may close these issues.

GIT_ASKPASS is set unconditionally

5 participants

@Backfighter @joaomoreno @MCterra10 @lszomoru @eamodio