Skip to content

Remove MSI #1666

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

Merged
merged 1 commit into from Mar 5, 2019
Merged

Remove MSI #1666

merged 1 commit into from Mar 5, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 20, 2019

This PR removes the rustup-win-installer directory, as suggested in #1611.

@ghost ghost closed this Feb 20, 2019
@ghost ghost reopened this Feb 20, 2019
@dwijnand
Copy link
Member

Looks like rustup's CI is broken with rust-lang/cargo#6192. Need to declare 'download' as a workspace member.

@ghost
Copy link
Author

ghost commented Feb 20, 2019

Thank you @dwijnand, for pointing me in the right direction.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Thanks!

@bors
Copy link
Contributor

bors commented Feb 26, 2019

☔ The latest upstream changes (presumably #1630) made this pull request unmergeable. Please resolve the merge conflicts.

@nrc
Copy link
Member

nrc commented Feb 26, 2019

@alexcrichton are we using this MSI code at all?

@emanuelbutacu could you rebase please?

@dwijnand
Copy link
Member

I also found out that, literally, the most commented issue in the issue tracker is "Add windows gui / msi installer" #253

So maybe we shouldn't remove this? (I'm just asking, I'm happy to remove it.)

@ghost
Copy link
Author

ghost commented Feb 27, 2019

In that case, we should wait for more people to chime in. I'll hold on to rebase until after we come to an agreement. How does that sound?

@alexcrichton
Copy link
Member

Looks like y'all may have found this, but the msi installer I believe is not currently used at all, but was in development historically to become the primary installation method on Windows. I'm not personally aware of what are the remaining blockers of it.

@kinnison
Copy link
Contributor

My gut feeling is that if we have no Windows-savvy developers on the WG right now, then removing it is the most sensible option. We can always retrieve it from git later, but for now it's just in the way and will be a bogon as @dwijnand goes about his restructuring of the codebase which we'd like to do after 1.17.0

@bors
Copy link
Contributor

bors commented Mar 5, 2019

☔ The latest upstream changes (presumably #1679) made this pull request unmergeable. Please resolve the merge conflicts.

@ghost
Copy link
Author

ghost commented Mar 5, 2019

I don't think the failing test has anything to do with this PR, though, I might be wrong. Can someone else confirm this?

@kinnison
Copy link
Contributor

kinnison commented Mar 5, 2019

Well, that's an interesting test failure.

@dwijnand
Copy link
Member

dwijnand commented Mar 5, 2019

Failure

failures:
---- rustup_stable_no_change stdout ----
running "/src/target/x86_64-unknown-linux-gnu/tests/rustup-exe.S9E4LlGpUVkM/rustup" "set" "default-host" "x86_64-unknown-linux-gnu"
status: exit code: 0
----- stdout
----- stderr
running "/src/target/x86_64-unknown-linux-gnu/tests/rustup-exe.S9E4LlGpUVkM/rustup" "update" "stable" "--no-self-update"
status: exit code: 0
----- stdout
  stable-x86_64-unknown-linux-gnu installed - 1.0.0 (hash-s-1)
----- stderr
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: latest update on 2015-01-01, rust version 1.0.0
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: downloading component 'cargo'
info: downloading component 'rust-docs'
info: installing component 'rust-std'
info: installing component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-docs'
running "/src/target/x86_64-unknown-linux-gnu/tests/rustup-exe.S9E4LlGpUVkM/rustup" "update" "--no-self-update"
status: signal: 11
----- stdout
----- stderr
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
> rustup update --no-self-update
out.ok: false
out.stdout (1 lines):
    
    
out.stderr (1 lines):
    info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
    
expected.ok: true
expected.stdout (3 lines):
    
      stable-x86_64-unknown-linux-gnu unchanged - 1.0.0 (hash-s-1)
    
    
expected.stderr (1 lines):
    info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
    
[src/rustup-mock/src/clitools.rs:228] out.stdout == stdout = false
[src/rustup-mock/src/clitools.rs:229] out.stderr == stderr = true
thread 'rustup_stable_no_change' panicked at 'explicit panic', src/rustup-mock/src/clitools.rs:230:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
failures:
    rustup_stable_no_change
test result: FAILED. 57 passed; 1 failed; 4 ignored; 0 measured; 0 filtered out

I'll restart the build.

@kinnison kinnison merged commit 64c8cbf into rust-lang:master Mar 5, 2019
@ghost ghost deleted the remove-msi branch March 6, 2019 04:33
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.

5 participants