这是indexloc提供的服务，不要输入任何密码
Skip to content

Replace menuinst subprocessing by ctypes win elevation #7426

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jul 9, 2018
Merged

Replace menuinst subprocessing by ctypes win elevation #7426

merged 9 commits into from
Jul 9, 2018

Conversation

@goanpeca
Copy link
Contributor

@goanpeca goanpeca commented Jun 18, 2018

No description provided.

@goanpeca goanpeca added this to the 4.6.0 milestone Jun 18, 2018
@goanpeca goanpeca self-assigned this Jun 18, 2018
@goanpeca goanpeca requested a review from a team as a code owner June 18, 2018 11:35
kalefranz
kalefranz reviewed Jun 18, 2018
View reviewed changes
conda/common/os/windows.py Outdated
"""
code = None
if _ctypes:
params = " ".join(['"%s"' % (x, ) for x in cmd_line[1:]])
Copy link
Contributor

@kalefranz kalefranz Jun 18, 2018

Choose a reason for hiding this comment

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

This line looks off to me. Is it really ok to quote every single parameter? And why the single-value tuple for the string interpolation?

Copy link
Contributor Author

@goanpeca goanpeca Jun 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is it really ok to quote every single parameter?

Well we could do it more conditionally based on the contents, but I think it is ok.

And why the single-value tuple for the string interpolation?

No real reason!

@kalefranz
Copy link
Contributor

kalefranz commented Jul 2, 2018 

INFO conda.core.initialize:run_plan(487): init_cmd_exe_registry: FileNotFoundError(2, 'The system cannot find the file specified', None, 2, None)
Traceback (most recent call last):
  File "C:\Users\admin\conda\conda\core\initialize.py", line 485, in run_plan
    result = globals()[step['function']](*step.get('args', ()), **step.get('kwargs', {}))
  File "C:\Users\admin\conda\conda\core\initialize.py", line 1064, in init_cmd_exe_registry
    prev_value, value_type = _read_windows_registry(target_path)
  File "C:\Users\admin\conda\conda\core\initialize.py", line 1036, in _read_windows_registry
    value_tuple = winreg.QueryValueEx(key, value_name)
FileNotFoundError: [WinError 2] The system cannot find the file specified

@goanpeca
Copy link
Contributor Author

goanpeca commented Jul 3, 2018

@kalefranz not sure about the error, as I have not change any code related to that.

Could you test on your VM?

kalefranz
kalefranz reviewed Jul 3, 2018
View reviewed changes
Copy link
Contributor

@kalefranz kalefranz left a comment

Choose a reason for hiding this comment

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

Looking good so far!

conda/cli/main_init.py
for_user = False

anaconda_prompt = on_win and args.anaconda_prompt
anaconda_prompt = on_win # TODO: probably remove and leave --anaconda-prompt as a flag
Copy link
Contributor

@kalefranz kalefranz Jul 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I must have put that in just for debug or something 😳

conda/core/initialize.py
stdin = json.dumps(plan)
result = subprocess_call(
'sudo %s -m conda.initialize' % sys.executable,
'sudo %s -m conda.core.initialize' % sys.executable,
Copy link
Contributor

@kalefranz kalefranz Jul 3, 2018

Choose a reason for hiding this comment

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

No wonder it wasn't working!

Copy link
Contributor Author

@goanpeca goanpeca Jul 3, 2018

Choose a reason for hiding this comment

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

yep I told you it was a silly bug 🤣

@goanpeca
Copy link
Contributor Author

goanpeca commented Jul 3, 2018 •
edited
Loading

@kalefranz :-p

=================================== FAILURES ===================================
__________ IntegrationTests.test_install_update_deps_only_deps_flags ___________
Traceback (most recent call last):
  File "/home/circleci/conda/tests/test_create.py", line 838, in test_install_update_deps_only_deps_flags
    raise NotImplementedError()
NotImplementedError
=============================== warnings summary ===============================
None
  pytest-catchlog plugin has been merged into the core, please remove it from your requirements.

-- Docs: http://doc.pytest.org/en/latest/warnings.html
============================= 492 tests deselected =============================
 1 failed, 113 passed, 28 skipped, 492 deselected, 2 xfailed, 1 warnings in 1707.25 seconds

@kalefranz
Copy link
Contributor

kalefranz commented Jul 7, 2018

I added a merge commit to this PR to merge in master. Want to see the tests run.

@goanpeca
Copy link
Contributor Author

goanpeca commented Jul 8, 2018 •
edited
Loading

@kalefranz tests pass now. This is ready AFAICT

@goanpeca goanpeca changed the title [WIP] Replace menuinst subprocessing by ctypes win elevation Replace menuinst subprocessing by ctypes win elevation Jul 8, 2018
@kalefranz
Copy link
Contributor

kalefranz commented Jul 9, 2018

Was there anything else you wanted to do here? Didn't you want to try to finish something out that you sort of started thinking through? Is that done, or not as high priority now? I don't remember exactly what that was...

@goanpeca
Copy link
Contributor Author

goanpeca commented Jul 9, 2018

I could not make it work ( reattach the elevated process to the current condole to view anyboutput)

kalefranz
kalefranz approved these changes Jul 9, 2018
View reviewed changes
Copy link
Contributor

@kalefranz kalefranz left a comment

Choose a reason for hiding this comment

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

LGTM

@kalefranz kalefranz merged commit b74cd81 into conda:master Jul 9, 2018
@goanpeca goanpeca mentioned this pull request Jul 17, 2018
Closed
@kalefranz kalefranz mentioned this pull request Aug 7, 2018
Closed
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Hi there, thank you for your contribution to Conda!

This pull request has been automatically locked since it has not had recent activity after it was closed.

Please open a new issue or pull request if needed.

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@kalefranz kalefranz kalefranz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@goanpeca goanpeca

Labels

locked [bot] locked due to inactivity

Projects

None yet

Milestone

4.6.0

Development

Successfully merging this pull request may close these issues.

2 participants

@goanpeca @kalefranz