-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
RFC/WIP: Allow Windows users to specify JULIA_SHELL for REPL shell mode #20776
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
Conversation
RFC/WIP: Opt-in to shell for REPL shell mode on Windows
|
Don't we also have busybox (#7446), which would offer the possibility of using Unix shell programs? This way the basic shell functions would be somewhat platform-independent. |
|
That has been suggested (iirc) but Windows folks seem to want their shell prompt to work like cmd. |
|
It seems like 99% of what people want is |
|
I'm totally fine with it, but then we get complaints that |
|
cmd is needlessly complicated to implement correct quoting and escaping for, the last time this came up busybox is GPL so not ideal, and I wouldn't really call a posix compatibility shim for a non-posix os "platform-independent" |
I guess there is toybox, though I don't know if anyone has ported it to mingw? But if we are already including busybox...?
I don't understand this comment. Platform-independent behaviors are always implemented on top of platform-specific shims. If you want a platform-independent shell mode, this is how you would do it, no? (Or we could implement our own shell that is incompatible with everything, of course.) |
Yes, I agree. I guess the purpose of this proposal was to give Windows users the ability to select their shell (just as Linux users can). I have changed the title accordingly. Under this proposal, users could choose: no shell, cmd or powershell (maybe busybox could be added) At the moment none of these are ideal and each has it's shortcomings, but maybe that's a separate issue. My concern is that an ideal single solution might be far off. In the meantime Windows users might be able to experiment, and maybe even submit PRs to improve one or more options. On the other hand, I do understand proceeding with non-ideal changes is not quite right. |
|
yes, I like this.
I can't speak for other Windows users, but I'd be perfectly happy with unix-style commands. I'm learning Julia, so I think I can learn a few commands like I've updated this PR to add support for
Well it's a simple change to make |
|
the version of |
base/client.jl
Outdated
| elseif shell_name == "powershell" | ||
| run(ignorestatus(`$shell -Command $(shell_escape(cmd))`)) | ||
| elseif shell_name == "busybox" | ||
| run(ignorestatus(`$shell $cmd`)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to do any special escaping for busybox on Windows?
If Julia bundles busybox on Windows, can we default to that (with the correct path if it is buried somewhere in the julia directory)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to do any special escaping for busybox on Windows?
Yes, for busybox now using full sh applet with shell_escape()
If Julia bundles busybox on Windows, can we default to that (with the correct path if it is buried somewhere in the julia directory)?
Default is now set to busybox using full path to julia bin sub-directory.
(Full path might not be necessary since bin sub-directory is added to PATH)
|
Summary of updates
Would some users still want old behavior (bypass shell and directly execute commands)? |
| shell = shell_split(get(ENV,"JULIA_SHELL",get(ENV,"SHELL","/bin/sh"))) | ||
| shell_name = Base.basename(shell[1]) | ||
| if is_windows() | ||
| default_shell = string("\"", joinpath(JULIA_HOME, "busybox"), "\"") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spaces, not tabs
|
Shouldn't default to busybox in a no-gpl build where we don't include it. I'm also not so sure Windows users want to default to unix shell behavior either. |
Oh I see.
I see your point. And also default for no-gpl build would be different from normal build. Perhaps it's better to revert to original opt-in approach? i.e. Keep the current "no-shell" default. |
* fix regressions due to replacing too many slots with SSAValues * Work around bug in tar 1.29 with --no-recursion and -T (#20942) causing duplicate files and including directories it shouldn't be * Fix printing of indices in the depwarn for to_indexes (#20960) * Use proper alignment when copying data to a variable. Using align=1 is very expensive on hardware without unaligned ld/st (eg. GPUs). * Lower `x^n` (literal n) to `Base.literal_pow(^, x, Val{n})` (#20889) Fixes #20882 * Add section on type piracy to style guide (#20967) * Add section on type piracy. * Add examples of type piracy. * Resolve ambiguity between * methods associated with RowVectors. * use explicit checks instead of try/catch to detect BLAS vendor * Nix ambiguity between \ methods. * fix broken link to xterm 256 colors in NEWS * Resolve ambiguities between checkbounds_indices methods. * Resolve ambiguity between tuple constructors. * Fix handling of `allow_bottom_tparams` in `isambiguous` The former version failed to detect that certain type parameters were Bottom (for example, in the case in the added docstring for `isambiguous`). * Resolve ambiguity between colon methods. * Fix broadcast with RowVectors of matrices (#20980) Fix #20979. Amusingly, this bug is a direct result of `transpose` being recursive. * pkg: factor url_or_pkg logic into its own function and test it * `abstract` -> `abstract type` (#21002) * Upgrade Documenter to 0.9.1 (#20997) fixes some windows path issues * Fix `typeintersect` involving `TypeVar`s bound to non-types Modify `var_gt` and `intersect_var` to cope with cases where a `TypeVar` has been deduced to be a non-type constant as is compared or intersected with another `TypeVar` or vice versa. * fix #20998 This was a case where an unknown typevar was incorrectly inferred to have the value `Any`. * fix #20992, compiler hang This was due to type intersection returning a type containing a free variable. * Make broadcast over sparse and rowvecs sparse * fix #20999, allow type redefinition with UnionAll-typed fields * Clarify data sharing behavior of vec (#21030) [ci skip] * NEWS.md: various fixes (#20996) * fix #21028, static parameter value incorrectly set to typevar This issue was caused by 6fb2c67. * Improve printing of manually thrown `MethodError`s Make sure the world age is only mentioned for valid exception world age and when exception world has no matching method. * Update NEWS.md w.r.t. removal of localize_vars [ci skip] (#21034) Changes related to PR #19594 * Correct error message when adding/removing processes from workers (#21033) * Clarify some license exceptions FFTW wrappers were moved and mostly rewritten - the Julia wrappers are probably not GPL? ref 481f351 and #8248 and #12201 make note of grisu derived license in top-level LICENSE.md and update links, repo was moved from github.com/floitsch to github.com/google umfpack and cholmod code were moved from base/linalg to base/sparse * RFC: Added recursive transpose to `Diagonal` (#21019) * Added recursive transpose to `Diagonal` `Diagonal` might be used to represent a block-diagonal matrix structure. * Fixes plus expm, logm, sqrtm * fix strtod indentation and mention it in LICENSE.md ref #5988 * Change default proxy settings to auto (#21044) * documentation: change `immutable` to `struct` (#21056) * Fix errors in, and run, add_license_to_files.jl * Fix line numbers and schurfact `show` output in doctests * Fix links in documentation smaller version of 0cc7d79 use --color=yes when running linkcheck * Extensible and fast date parsing (#20952) * Refactor date parsing to be fast and extensible * Deprecate parse(::AbstractString, ::DateFormat) * fixup * Switch to datatype_name * Move towards consistent terminology * Rename parse(::Vector, ...) to parse_components * Internal parse funcs now take and return position * Documentation for internal functions * Corrections to documentation * Corrections from review * More details about raise * Remove outdated comment * Cleanup function signatures using DateLocale (#21011) * fix `return_type_tfunc` in the case where no methods match also fix `return_type` on Builtins; this was another disparity with `return_type_tfunc` * Introduce temporary if necessary to inline `_apply` * use `newvar!` instead of `add_slot!` * add test * Download failure with curl no longer create a file * fix #21054, parse/lowering problem with `<:T` syntax * use newest world callback on_done (#21027) * use newest world callback on_done * use QuoteNode to prevent parameters being expanded * also quote on_done * fix #20611, type intersection inferring wrong parameter value * fix #21081, quadratic slowdown in `remove_redundant_temp_vars!` * ensure the gc-rooting pass does not discard derived roots fix #21015 * Carry openblas power assembly fixes from OpenMathLib/OpenBLAS#1098 * Revert "Add patch to set `-O2` for suitesparse build (#20165)" This reverts commit 145eae8. This was actually a bug in the power kernels in OpenBLAS, not a GCC or SuiteSparse problem * Respect format width for milliseconds (#21070) Fixes #21001 * wrap long lines in base/linalg/lu.jl line breaks in definitions of stridedarray aliases and a couple lines in test/parse.jl * wrap long lines in base/abstractarray.jl * 4 space indent in go_benchmark.jl * put comments at the same indentation level as code in base/linalg/givens.jl * empty line before jldoctest openers in base/reflection.jl * Throw an exception for invalid hostnames in addprocs. (#21021) * throw an exception for invalid hostnames in addprocs. * clean up some one-liners in string functions remove unnecessary spaces inside [] in contains doctest fix nonstandard indentation in utf8proc tests top level testset in a file under test/ should be redundant now fix some typos and grammar in a few comments add some spaces in arnoldi tests combine sentences instead of starting one with And change bitstype to primitive bits type in simd-types docs fix plural subtypes fix plural or other types fix retrhow and unsatifiable typos in base/pkg/resolve.jl code highlighting in types manual section * rename oid to oid_ptr in fetchhead_foreach_callback add a few spaces after # comment openers spell out acronyms in compiler devdocs make spacing more conventional in test/replutil.jl * use === for comparison to nothing * Use `typejoin` of the field types for `eltype` of heterogeneous `Tuple`s * Move `rewrap_unionall` in `eltype(::Type{<:Tuple})` * Improve task switching times (#21111) * Only create file upon wget success * Fix typo in inference * Improve performance of skipchars for small inputs This mostly solves the problem, now the function is only significantly (2x) slower when there are no characters to skip, otherwise it's mostly the same for 1 or 2 characters and faster for 3 or more. Also improve the tests a little. * fix #21088, missing `widenconst` in `return_type` (#21115) * Handle `Union` of `Tuple`s in `eltype(::Type{<:Tuple})` * fix #21074, constant fold more expressions * also for #21074, improve `getfield_elim_pass!` to handle more kinds of values * fix a bug in the t-func for `nfields` * fix a bug in `getfield` t-func on invalid calls * improve constant folding of SimpleVector length and `nfields` * fix some issues around inlining constants avoid inlining large constants * fix unary `<:` and `>:` call syntax * fix FFTW crash after set_num_threads * cosmetic: add a couple of newlines at file ends * rename allow_bottom(_tparams)? => ambiguous_bottom * isambiguous: make ambiguous_bottom a keyword argument This matches detect_ambiguities – yes, isambiguous is unexported, but we know people will still use it and we may as well have consistent signatures here. * deprecate old arguments to isambiguous, detect_ambiguities * test ambiguous_bottom options to isambiguous, detect_ambiguities * swap default for ambiguous_bottom from true to false * fix #21118, subtyping issue with diagonal variable with abstract lower bound In `Tuple{Vararg{T}} where T`, `T` is diagonal. In the case in this issue, type intersection was giving such a variable an abstract lower bound. That was wrong, but it also created a type that could not be subtyped properly since the given bound on the variable contradicted the requirement that it be concrete. So for now at least, given e.g. `Tuple{T,T} where T>:Real`, simply drop the diagonal-concrete rule. * OSX: Add both RPATH entries to `julia`, don't overwrite See 30ba746#commitcomment-21430419 for more details * Add tests for download * fix #21132, intersection hang due to x == Pair{x} constraint * Fix stride for vectors in BLAS.ger! and BLAS.trsv! (#21125) Fixes #21122 * add devdocs on "diagonal" types [ci skip] * test/socket: wait_with_timeout to provide more debugging info * Immediately expand JULIAHOME in Makefile (#21140) * fix whitelist of expression heads in `lambda-optimize-vars!` `decl` and `ssavalue` were omitted by mistake. This helps #15276. * compress AST in memory, not just on disk sysimg size will be exactly the same, but seems to be pretty significant savings in memory and initial GC time (about 10% on each) * Improve task switching performance on unbuffered channels (#21120) Improve task switching performance on unbuffered channels * some fixes for compiling the runtime code as c++ * reenable Core+Base ambiguity detection (#20986) * Resolve ambiguity between cat (git-cat-file and other) methods. * Reenable the Core+Base ambiguity detection test. * improve `precompile` mechanism a bit - improve static_show to readably print a much larger set of types - add TRACE_COMPILE option to print `precompile` calls * Restrict mapslices to only reuse StridedArray slices (#21133) Fix #21123. * fix #21147, lowering of keyword args with chained static parameters * rename Core.BottomType => Core.TypeofBottom (#21057) The old name sounds like an alias for `Bottom = Union{}` rather than `Type{Bottom} == typeof(Bottom)`. (This last should hold but currently doesn't – see #21016.) * fix part of #21016, specificity of `TypeofBottom` * add `serialize` for `Bottom` to ensure no `Type{<:T}` methods override it * fix a method cache matching issue with Type{T} This is needed to avoid confusing `Type{A{B}}` and `Type{A{B} where B}`. In the future, this can be improved by not using `Type{ }` to dispatch types with free variables. * preserve identities of type wrappers in serializer. fixes #20324 * Run test/ambiguous.jl first works around #21160, hang in test that started happening when #20986 was merged * fix other part of #21147, optional instead of keyword args * Fix mapreduce_impl() for 1-element range (#19325) * fix mapreduce_impl() for 1-element range * mapreduce_impl() type stability tests * wrap long lines * mapreduce_impl(): add @inbounds ifirst:ilast should be a valid A indices range (checked by the caller) * reduce code duplication in _mapreduce() - let mapreduce_impl() handle all but empty collection cases - don't test mapreduce_impl() directly - extend mapreduce() tests with empty/long collections * mapreduce_impl(f, min/max): don't skip NaNs - update mapreduce_impl(f, min/max) to 0.6 behaviour of treating NaNs, also stop scanning the array once NaN is detected - add NaN minimum/maximum() tests for 2-element and long arrays * mapreduce(f, op): don't apply @inbounds for `f` * clarify how mapreduce_impl() should be used * DefaultTestSet tests for test_broken and test_skip (#21086) * DefaultTestSet tests for test_broken and test_skip Check that test_broken and test_skip do not result in an exception. Already fixed in 8982605 - "Rework test framework", but present in julia-0.5 (see #21008). * Cleanup: use isa rather than typeof * Correct anchors in README.md (#21166) * Move manual anchors in README.md below headlines. Alternative approach to make GitHub correctly render the `README.md` headlines by simply moving the manual anchors to below the headlines. Since current manual anchors are uppercase, this keeps external links valid. * Remove manual HTML anchors from README.md. The `README.md` is currently rendered incorrectly on GitHub since it includes manual HTML anchors for headlines. However, since several years, GitHub automatically includes these anchors. Considering the anchors in the README.md have been added some six years ago, the GitHub feature is probably younger. This removes all the manual anchors, for which all match the exact headline pattern anyway, except the following which differ slightly: - `<a name="Editor-Terminal-Setup"/>` - `<a name="Platform-Specific-Notes"/>` - `<a name="Required-Build-Tools-External-Libraries"/>` If referenced from somewher externally, these would need to be updated. Links within this README to anchors are also corrected. The automatically generated anchors on GitHub follow the pattern of replacing all spaces with `-` and everything being lowercase, e.g. `## This is a Title` automatically gets the anchor equivalent to `<a name="this-is-a-title"/>` to be referenced in markdown with `[Link to a title](#this-is-a-title)`. * handle small (<16) arrays by _mapreduce() (#21167) * deprecate use of cat with LibGit2 objects * Augment sort.jl documentation with doctests (#18923) * Fix doc system part of #21016 (#21036) * Breakage. * Teach doc system to handle 'where'. * Add parameters to signature. * Add test and continue to use Union. * Fix missing docstrings. * Address comments. * use newest Documenter, DocStringExtensions * Use separate build dirs for mbedtls depending on USE_GPL_LIBS prevents cmake from complaining when you switch sources for the same build dir * Fix doctest line numbers * run code signing script in contrib/prepare_release.sh if present * fix #21178, lowering of static params with `<:T` syntax * fix #20671, slowdown loading SIUnits This adds a fast path to intersection that helps when we have a type with lots of parameters that just need to be matched up 1-to-1 with parameters of another type. Also fixes a bug in intersection uncovered by this test case. * Various missing git tests * Add missing test for mapslices (#21181) Followup from #21133 * Update arrayops.jl * ensure ShellExecuteW only gets compiled on windows rather than depending on DCE to delete this code after inference * dump: ensure jlcall_api is cleared if we don't load the fptr from the sysimg * Fix `⊑(a::Const, b::PartialTypeVar)` For `isa(a, Const) && !isa(b, Const)`, `⊑` assumed `b` to be a type, leading to an error if `b` was a `PartialTypeVar`. Fixed by using `widenconst(b)`. * Use `TypeVar`s in `limit_type_depth` in `Union`s in invariant position Using `Any` for depth-limited `Union` members is only correct in covariant position, otherwise a `TypeVar` has be be introduced. * document x % T (#20759) * add support for AbstractGitHash * updated fix for #19892 (FFTW threads initialization) (#21169) * updated fix for #19892; initialize FFTW threads the first time the planner is called (#21127 incorrectly prevented threads from being used at all) * add test for #21163 * fix #21172, `a = f(x) = 1` * handle unicode space characters in `parse` for `Bool` * fix #21180, regression in `readcsv` with quoted unicode fields * don't make corrupted copies of singleton objects during AST deserialization this corruption will now be detected when finalizing the sysimg with an assertion * Update datafmt.jl Use `endof` instead of `length` as `colval` expects byte index. * Add test to for fix * Correct notation used in the rem/mod docstring (#21198) * Correct notation used in rem/mod docstring * Fix build warning about overwritten docstring for rem * More tests for GitConfig * improve codegen of `setfield!` with constant integer index * Fix bug in merge! and add test (#21213) * Add a test for aborting a rebase (#21209) * Support Rational and Irrational arguments for exp10 (#21199) * null pointer checks are unnecessary: API should never return valid pointers with an error (#21174) * Revert "Add a test for aborting a rebase (#21209)" (#21220) This reverts commit 46b6099. * Fix rem/mod doc (#21223) * Two minitests for show * Make cor work again for complex input (#21205) Also fix inconsistency in `cov` of vectors. Fixes #21093 * Fix build with LLVM master. (#21194) In LLVMFPtoInt signature of convertToInteger now expects a MutableArrayRef. AttributeSet -> AttributeList * Add tests for internal fastforward merge methods * Fix bug in backsolve of sparse hermitian matrices (#21165) * fix #21168, `f.[1,2,3]` should be an error * Add methods to add refspecs and tests to view fetchspecs (#21227) * Add methods to add refspecs and tests to view fetchspecs * Add docs * fix explicity typo and very minor wording tweak * Tag 0.6.0-pre.beta (#21232) * Non-ff merge test * Add a test for GitRemote ctor with refspec * Tests for cleanup and branch deletion * More fixes for non-1 arrays * Remove extraneous x^Val methods and add tests * allow TTYTerminal streams to be any IO types * ensure that AST of a constant return function is correct fix #21175 * Allow newline or semicolon in abstract and primitive defs * Fix exports of undefined/missing/conditionally defined symbols (#21217) * Remove Base.Parallel export. * Remove 'big_str' export from mpfr.jl * Remove 'dense' from export in sparse.jl * deprecate unintended methods of zeros, ones (#21183) * fix error handling for ccall without a concrete layout fix #21104 * Test that `convert(Tuple{...}, (...))` throws for mismatched numbers of elements * fix #20614, need error for `Vararg` on non-final argument * Ensure that `collect(::AbstractArray)` always returns an `Array` (#21257) The docstring specifies that the return type is `Array`, but `similar` isn't guaranteed to return an `Array`. (Exceptions include AxisArrays, OffsetArrays, etc.) * fix #21271, codegen of typeassert involving `Tuple{Type{T}}` * [docathon] Add examples for diff_files * Add a test for aborting a rebase (#21222) * Add a test for aborting a rebase * Fix bad merge tests corrupting test_repo
There has been much discussion regarding REPL shell mode for Windows:
In my reading, I think there seems to be 2 main issues:
cmdis difficult to work with, especially when escaping special characterspowershellis better but is quite slow to startupThis proposal is to enable users to opt-in (via JULIA_SHELL) to using
cmdorpowershellas the command interpreter.The default shell mode for Windows users is unchanged.
If
JULIA_SHELLenvironment variable is not explicity set, then shell mode executes commands directly, as is the case now.There are probably many cases where shell escaping does not work properly, but simple commands including the built-ins should work. For anything more complex it would probably be easier to launch the shell (via
;cmdor;powershell) and run commands directly from there.I haven't done extensive testing but the following commands and paths work:
cmdpowershellIf there is no real appetite for progressing with this proposal, please feel free to close.
I just think that minimal opt-in support for simple commands on Windows would be better than what is available now.
Unfortunately I cannot build Julia and test this directly.
My tests have been on similar changes to v0.5.0.