sagemath · vbraun · Jul 20, 2023 · Jun 27, 2023 · Jun 27, 2023 · Jul 12, 2023
diff --git a/.github/workflows/ci-conda.yml b/.github/workflows/ci-conda.yml
@@ -34,7 +34,9 @@ jobs:
       matrix:
         os: [ubuntu-latest, macos-latest]
         python: ['3.9', '3.10', '3.11']
-        conda-env: [environment, environment-optional]
+        # Optional environment is disabled for now as its not yet working
+        # environment: [environment, environment-optional]
+        conda-env: [environment]
 
     steps:
       - uses: actions/checkout@v3
@@ -100,19 +102,20 @@ jobs:
           SAGE_NUM_THREADS: 2
 
       - name: Verify dependencies
-        if: always()
+        if: success() || failure()
         shell: bash -l {0}
         run: pip check
 
       - name: Test
+        if: success() || failure()
         shell: bash -l {0}
         run: ./sage -t --all -p0
 
       - name: Print logs
+        if: always()
         run: |
           for file in $(find . -type f -name "*.log"); do
               echo "::group::$file"
               cat "$file"
               echo "::endgroup::"
           done
-        if: always()
diff --git a/build/pkgs/sphinx/distros/conda.txt b/build/pkgs/sphinx/distros/conda.txt
@@ -1 +1 @@
-sphinx<6,>=5.2
+sphinx<8,>=5.2
diff --git a/build/pkgs/sphinx/install-requires.txt b/build/pkgs/sphinx/install-requires.txt
@@ -1 +1 @@
-sphinx >=5.2, <6
+sphinx >=5.2, <8
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		sphinx<6,>=5.2
Copy link Contributor mkoeppe Jun 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. We need to keep an upper bound. `sphinx<8` would be appropriate as the fixes for 7 have been done. Copy link Contributor Author tobiasdiez Jun 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. why? I would only add a version constraint if there are really known issues. Copy link Contributor mkoeppe Jun 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Because we know that in every Sphinx upgrade we had to fix something critical. Copy link Contributor Author tobiasdiez Jun 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. That probably applies to many dependencies, but almost none of them have upper constraints in the conda info. Copy link Contributor mkoeppe Jun 27, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. No, I am saying that it specifically applies to Sphinx. Copy link Contributor mkoeppe Jun 28, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. all of this applies to cython as well in an even more extreme way, and there we don't specify any upper bound. Please check the facts before making such claims; it really makes no sense to discuss at this level. We do set an upper bound for Cython in install-requires.txt. (There we know already that the next major will break things. That's #29863. Copy link Contributor mkoeppe Jun 28, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. finally, its not conda specific. We agree here -- I of course also object to removing the upper bound from `build/pkgs/sphinx/install-requires.txt`. Copy link Contributor mkoeppe Jun 28, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. conda is marked experimental, so a few rough edges are acceptable. That's a terrible argument; certainly we don't want to keep it "experimental". Copy link Contributor Author tobiasdiez Jun 28, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Thanks for your input, I see your point but don't agree with it. Copy link Contributor mkoeppe Jun 28, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Which point? I've made many.
		sphinx<8,>=5.2