Cleanup `.bat` scripts and ensure `dev/start.bat` uses source code's `conda_hook.bat` #13837

kenodegard · 2024-04-23T17:05:01Z

Description

Many small improvements:

Replace rem/REM/@rem/@REM with the tidier ::
Format all commands as uppercase
Standardize %ERRORLEVEL% checking
Standardize @IF clauses to use DEFINED when possible and to otherwise use [%VAR%]==[value]
Check for DOSKEY conda= instead of DEFINED CONDA_SHLVL
Update dev\start.bat to use source code's condabin & conda_hook.bat (so we can test bat script edits via our devenv)

Checklist - did you ...

~~Add a file to the news directory (using the template) for the next release's release notes?~~
~~Add / update necessary tests?~~
~~Add / update outdated documentation?~~

codspeed-hq · 2024-04-23T17:44:50Z

CodSpeed Performance Report

Merging #13837 will not alter performance

_{Comparing cleanup.bat (e4a388e) with 24.5.x (04308fe)}

Summary

✅ 21 untouched benchmarks

conda-bot · 2024-04-24T02:09:51Z

Review build status

The workflow for building and uploading a canary release for conda with the label conda-conda-pr-13837 in channel conda-canary was started by @kenodegard!

Once it's done, use this command to try it out in a new conda environment:

conda install -c conda-canary/label/conda-conda-pr-13837 conda

build 8979808571 succeeded on Linux (X64)
build 8979808571 succeeded on macOS (ARM64)
build 8979808571 succeeded on macOS (X64)
build 8979808571 succeeded on Windows (X64)

travishathaway · 2024-04-25T05:00:56Z

conda/shell/Library/bin/conda.bat

-@REM SPDX-License-Identifier: BSD-3-Clause
-@CALL "%~dp0..\..\condabin\conda.bat" %*
+:: Copyright (C) 2012 Anaconda, Inc
+:: SPDX-License-Identifier: BSD-3-Clause


Yes much tidier ❤️

travishathaway

Looks good.

One thing I can't help but wonder is if there are any linting tools for batch scripts? Maybe this would be something to add to pre-commit? Not necessary for this PR but just a thought.

dbast · 2024-04-25T08:56:33Z

Nice. This can be maybe made even tidier (and better to read) via the pattern cmd || goto :error instead of the many duplicated lines with @IF NOT [%ERRORLEVEL%]==[0] @EXIT /B %ERRORLEVEL% ... see e.g. where this is applied https://github.com/conda-forge/ripgrep-feedstock/blob/main/recipe/bld.bat

ifitchet · 2024-04-25T09:02:32Z

Nice. This can be maybe made even tidier (and better to read) via the pattern cmd || goto :error instead of the many duplicated lines with @IF NOT [%ERRORLEVEL%]==[0] @EXIT /B %ERRORLEVEL% ... see e.g. where this is applied https://github.com/conda-forge/ripgrep-feedstock/blob/main/recipe/bld.bat

If you're going to do this then Line 13, or an equivalent, is critical, https://github.com/conda-forge/ripgrep-feedstock/blob/main/recipe/bld.bat#L13. exit 0 is an equivalent, of course.

Without it, all your code paths will dutifully run through the :error code.

jezdez

This does look better, but OTOH I wish we'd have something like black for batch files.

conda/shell/Scripts/activate.bat

jezdez · 2024-04-29T19:55:26Z

conda/shell/condabin/_conda_activate.bat

+IF DEFINED CONDA_PS1_BACKUP (
+    :: Handle transition from shell activated with conda 4.3 to a subsequent activation
+    :: after conda updated to 4.4. See issue #6173.
+    SET "PROMPT=%CONDA_PS1_BACKUP%"
+    SET CONDA_PS1_BACKUP=
 )


Should we... remove this?

I would love to, if someone updates from an older conda that breaks their prompt they can just restart their shell and should be good to go

Let's leave this as is for now and work to remove it in the future

conda/shell/condabin/activate.bat

dev/start.bat

kenodegard · 2024-05-01T14:09:23Z

While testing canary builds I discovered issues with some of these changes, still debugging

The base branch was changed.

kenodegard · 2024-05-07T05:15:29Z

conda/shell/Scripts/activate.bat

+:: skip checking for quotes if there are no arguments
+IF [%1]==[] GOTO :SKIP_CHECK
+
+:: enter localized variable scope
+SETLOCAL
+
+:: Test first character and last character of %1 to see if first character is a "
+::   but the last character isn't.
+:: This was a bug as described in https://github.com/ContinuumIO/menuinst/issues/60
+:: When Anaconda Prompt has the form
+::   %windir%\system32\cmd.exe "/K" "C:\Users\builder\Miniconda3\Scripts\activate.bat" "C:\Users\builder\Miniconda3"
+:: Rather than the correct
+::    %windir%\system32\cmd.exe /K ""C:\Users\builder\Miniconda3\Scripts\activate.bat" "C:\Users\builder\Miniconda3""
+:: this solution taken from https://stackoverflow.com/a/31359867
+
+SET "_ARG=%1"
+SET _ARG_FIRST=%_ARG:~0,1%
+SET _ARG_LAST=%_ARG:~-1%
+SET _ARG_FIRST=%_ARG_FIRST:"=+%
+SET _ARG_LAST=%_ARG_LAST:"=+%
+
+:: if the first character is not a quote we can skip further quote checking
+IF NOT [%_ARG_FIRST%]==[+] ENDLOCAL & GOTO :SKIP_CHECK
+
+:: the first and last characters appear to be matching quotes so we can activate normally
+IF [%_ARG_LAST%]==[+] ENDLOCAL & GOTO :SKIP_CHECK
+
+:: found a quote mismatch, we assume this is a bug in the downstream code
+ENDLOCAL & CALL "%~dp0\..\condabin\conda.bat" activate
+GOTO :EOF
+
+:SKIP_CHECK


I would prefer to remove this special bug handling for Anaconda Prompt/Menuinst

Thoughts @jaimergp @marcoesters

Sorry, I missed the tag. Right now, the target of the shortcut looks like this

%windir%\System32\cmd.exe "/K" C:\Users\<user>\miniconda3\Scripts\activate.bat C:\Users\<user>\miniconda3

So, maybe install console_shortcut_miniconda or console_shortcut with the dev version of conda and see if the workaround is still necessary?

if it is, removing it is tricky because that would render that shortcut unusable when a user updates conda.

kenodegard added 7 commits April 23, 2024 12:00

@REM → ::

f642cd8

Cleanup & format

04999b6

Use DEFINED, improve comments, use [] over ""

89eb679

Update start.bat

3198fff

Check for DOSKEY, not CONDA_SHLVL

67496d1

Use source conda_hook.bat

59cc06d

Update conda_hook.bat

6528215

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 23, 2024

kenodegard added 5 commits April 23, 2024 14:37

Merge branch 'main' into cleanup.bat

28262f2

Cleanup

0669c08

Update start.bat

5d6ac4f

Update cmd.exe init_command

996ae1b

Update activate.bat

09f8a64

kenodegard marked this pull request as ready for review April 24, 2024 02:09

kenodegard requested a review from a team as a code owner April 24, 2024 02:09

kenodegard added the build::review trigger a build for this PR label Apr 24, 2024

conda-bot removed the build::review trigger a build for this PR label Apr 24, 2024

travishathaway reviewed Apr 25, 2024

View reviewed changes

travishathaway previously approved these changes Apr 25, 2024

View reviewed changes

More cleanup

1a6dcd8

kenodegard dismissed travishathaway’s stale review via 1a6dcd8 April 26, 2024 02:15

kenodegard added 5 commits April 26, 2024 09:41

Merge remote-tracking branch 'upstream/main' into cleanup.bat

0965fc5

Fix errorlevel exits

3016276

Cleanup exits

acf62da

Don't return errorlevel

839a812

Merge branch 'main' into cleanup.bat

f4b156d

jezdez reviewed Apr 29, 2024

View reviewed changes

kenodegard added 2 commits April 30, 2024 09:29

Update dev/start.bat

e62a924

Update conda/shell/condabin/activate.bat

75a66e9

kenodegard requested review from marcoesters, dbast, ifitchet, travishathaway and jezdez April 30, 2024 14:55

Use 4 space indentations

96ecc01

jezdez previously approved these changes Apr 30, 2024

View reviewed changes

kenodegard added the build::review trigger a build for this PR label Apr 30, 2024

conda-bot removed the build::review trigger a build for this PR label Apr 30, 2024

kenodegard added this to the 24.5.x milestone Apr 30, 2024

kenodegard marked this pull request as draft May 1, 2024 14:09

kenodegard changed the base branch from main to 24.5.x May 1, 2024 14:59

kenodegard force-pushed the cleanup.bat branch from 95ed9d1 to dd4b506 Compare May 7, 2024 03:46

SETLOCAL

70b0920

kenodegard force-pushed the cleanup.bat branch from dd4b506 to 70b0920 Compare May 7, 2024 03:49

Update start.bat

185623a

kenodegard added the build::review trigger a build for this PR label May 7, 2024

conda-bot removed the build::review trigger a build for this PR label May 7, 2024

Flatten

e4a388e

kenodegard added the build::review trigger a build for this PR label May 7, 2024

conda-bot removed the build::review trigger a build for this PR label May 7, 2024

kenodegard commented May 7, 2024

View reviewed changes

kenodegard marked this pull request as ready for review May 7, 2024 14:26

kenodegard modified the milestones: 24.5.x, 24.7.x May 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup `.bat` scripts and ensure `dev/start.bat` uses source code's `conda_hook.bat` #13837

Cleanup `.bat` scripts and ensure `dev/start.bat` uses source code's `conda_hook.bat` #13837

kenodegard commented Apr 23, 2024 •

edited

codspeed-hq bot commented Apr 23, 2024 •

edited

conda-bot commented Apr 24, 2024 •

edited

travishathaway Apr 25, 2024

travishathaway left a comment

dbast commented Apr 25, 2024

ifitchet commented Apr 25, 2024

jezdez left a comment

jezdez Apr 29, 2024

kenodegard Apr 30, 2024

kenodegard Apr 30, 2024

kenodegard commented May 1, 2024

kenodegard May 7, 2024

marcoesters May 21, 2024

Cleanup .bat scripts and ensure dev/start.bat uses source code's conda_hook.bat #13837

Are you sure you want to change the base?

Cleanup .bat scripts and ensure dev/start.bat uses source code's conda_hook.bat #13837

Conversation

kenodegard commented Apr 23, 2024 • edited

Description

Checklist - did you ...

codspeed-hq bot commented Apr 23, 2024 • edited

CodSpeed Performance Report

Merging #13837 will not alter performance

Summary

conda-bot commented Apr 24, 2024 • edited

Review build status

travishathaway Apr 25, 2024

Choose a reason for hiding this comment

travishathaway left a comment

Choose a reason for hiding this comment

dbast commented Apr 25, 2024

ifitchet commented Apr 25, 2024

jezdez left a comment

Choose a reason for hiding this comment

jezdez Apr 29, 2024

Choose a reason for hiding this comment

kenodegard Apr 30, 2024

Choose a reason for hiding this comment

kenodegard Apr 30, 2024

Choose a reason for hiding this comment

kenodegard commented May 1, 2024

kenodegard May 7, 2024

Choose a reason for hiding this comment

marcoesters May 21, 2024

Choose a reason for hiding this comment

Cleanup `.bat` scripts and ensure `dev/start.bat` uses source code's `conda_hook.bat` #13837

Cleanup `.bat` scripts and ensure `dev/start.bat` uses source code's `conda_hook.bat` #13837

kenodegard commented Apr 23, 2024 •

edited

codspeed-hq bot commented Apr 23, 2024 •

edited

conda-bot commented Apr 24, 2024 •

edited