Skip to content

CI: remove dependency on cabal-plan (#8891) #8893

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
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 2 additions & 23 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,6 @@ jobs:
run: |
git config --global protocol.file.allow always

# The '+exe' constraint below is important, otherwise cabal-install
# might decide to build the library but not the executable which is
# what we need.
- name: Install cabal-plan
run: |
cd $(mktemp -d)
cabal install cabal-plan --allow-newer="base" --constraint='cabal-plan +exe'
echo "$HOME/.cabal/bin:$HOME/.local/bin" >> $GITHUB_PATH

# The tool is not essential to the rest of the test suite. If
# hackage-repo-tool is not present, any test that requires it will
# be skipped.
Expand Down Expand Up @@ -132,11 +123,11 @@ jobs:
- name: Tar cabal head executable
if: matrix.ghc == env.GHC_FOR_RELEASE
run: |
CABAL_EXEC=$(cabal-plan list-bin --builddir=dist-newstyle-validate-ghc-${{ matrix.ghc }} cabal-install:exe:cabal)
CABAL_EXEC=$(cabal list-bin --builddir=dist-newstyle-validate-ghc-${{ matrix.ghc }} --project-file=cabal.project.validate cabal-install:exe:cabal)
# We have to tar the executable to preserve executable permissions
# see https://github.com/actions/upload-artifact/issues/38
if [[ ${{ runner.os }} == 'Windows' ]]; then
# `cabal-plan` gives us a windows path but tar needs the posix one
# `cabal list-bin` gives us a windows path but tar needs the posix one
CABAL_EXEC=$(cygpath $CABAL_EXEC)
fi
if [[ "${{ runner.os }}" == "macOS" ]]; then
Expand Down Expand Up @@ -235,12 +226,6 @@ jobs:
key: ${{ runner.os }}-${{ matrix.ghc }}-${{ github.sha }}
restore-keys: ${{ runner.os }}-${{ matrix.ghc }}-

- name: Install cabal-plan
run: |
cd $(mktemp -d)
cabal install cabal-plan --allow-newer="base" --constraint='cabal-plan +exe'
echo "$HOME/.cabal/bin:$HOME/.local/bin" >> $GITHUB_PATH

- name: Validate build
run: sh validate.sh ${{ env.COMMON_FLAGS }} -s build

Expand Down Expand Up @@ -283,12 +268,6 @@ jobs:
ghc-version: ${{ matrix.ghc }}
cabal-version: latest # default, we are not using it in this job

- name: Install cabal-plan
run: |
cd $(mktemp -d)
cabal install cabal-plan --allow-newer="base" --constraint='cabal-plan +exe'
echo "$HOME/.cabal/bin:$HOME/.local/bin" >> $GITHUB_PATH

- name: Download cabal executable from workflow artifacts
uses: actions/download-artifact@v3
with:
Expand Down
10 changes: 4 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ $(TEMPLATE_PATHS) : templates/Paths_pkg.template.hs cabal-dev-scripts/src/GenPat

buildinfo-fields-reference : phony
cabal build --builddir=dist-newstyle-bi --project-file=cabal.project.buildinfo buildinfo-reference-generator
$$(cabal-plan list-bin --builddir=dist-newstyle-bi buildinfo-reference-generator) buildinfo-reference-generator/template.zinza | tee $@
$$(cabal list-bin --builddir=dist-newstyle-bi buildinfo-reference-generator) buildinfo-reference-generator/template.zinza | tee $@

# analyse-imports
analyse-imports : phony
Expand Down Expand Up @@ -113,24 +113,22 @@ hackage-roundtrip-tests :
$(CABALRUN) hackage-tests -- roundtrip +RTS -s -qg -I0 -A64M -N${THREADS} -RTS ${TEST}

cabal-install-test:
@which cabal-plan
$(CABALBUILD) -j3 cabal-tests cabal
rm -rf .ghc.environment.*
cd cabal-testsuite && `cabal-plan list-bin cabal-tests` --with-cabal=`cabal-plan list-bin cabal` --hide-successes -j3 ${TEST}
cd cabal-testsuite && `cabal list-bin cabal-tests` --with-cabal=`cabal list-bin cabal` --hide-successes -j3 ${TEST}

# hackage-benchmarks (solver)

hackage-benchmarks-run:
$(CABALBUILD) -j3 hackage-benchmark cabal
rm -rf .ghc.environment.*
$$(cabal-plan list-bin hackage-benchmark) --cabal1=cabal --cabal2=$$(cabal-plan list-bin cabal) --packages="hakyll servant-auth-server" --print-trials --concurrently
$$(cabal list-bin hackage-benchmark) --cabal1=cabal --cabal2=$$(cabal list-bin cabal) --packages="hakyll servant-auth-server" --print-trials --concurrently


# This doesn't run build, as you first need to test with cabal-install-test :)
cabal-install-test-accept:
@which cabal-plan
rm -rf .ghc.environment.*
cd cabal-testsuite && `cabal-plan list-bin cabal-tests` --with-cabal=`cabal-plan list-bin cabal` --hide-successes -j3 --accept ${TEST}
cd cabal-testsuite && `cabal list-bin cabal-tests` --with-cabal=`cabal list-bin cabal` --hide-successes -j3 --accept ${TEST}

# Docker validation

Expand Down
46 changes: 20 additions & 26 deletions validate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# See https://github.com/haskell/cabal/issues/8049
HC=ghc
CABAL=cabal
CABALPLAN=cabal-plan
JOBS=4
LIBTESTS=true
CLITESTS=true
Expand Down Expand Up @@ -50,7 +49,6 @@ Available options:
--(no-)run-cli-suite Run cabal-testsuite with client
-w, --with-compiler HC With compiler
--with-cabal CABAL With cabal-install
--with-cabal-plan CABALPLAN With cabal-plan
--extra-hc HC Extra compiler to run test-suite with
--(no-)doctest Run doctest on library
--(no-)solver-benchmarks Build and trial run solver-benchmarks
Expand Down Expand Up @@ -196,11 +194,6 @@ while [ $# -gt 0 ]; do
shift
shift
;;
--with-cabal-plan)
CABALPLAN=$2
shift
shift
;;
--extra-hc)
EXTRAHCS="$EXTRAHCS $2"
shift
Expand Down Expand Up @@ -315,7 +308,7 @@ BUILDDIR=dist-newstyle-validate-$BASEHC
CABAL_TESTSUITE_BDIR="$(pwd)/$BUILDDIR/build/$ARCH/$BASEHC/cabal-testsuite-3"

CABALNEWBUILD="${CABAL} v2-build $JOBS -w $HC --builddir=$BUILDDIR --project-file=$PROJECTFILE"
CABALPLANLISTBIN="${CABALPLAN} list-bin --builddir=$BUILDDIR"
CABALLISTBIN="${CABAL} list-bin --builddir=$BUILDDIR --project-file=$PROJECTFILE"

# header
#######################################################################
Expand All @@ -327,7 +320,6 @@ cat <<EOF
compiler: $HC
runhaskell: $RUNHASKELL
cabal-install: $CABAL
cabal-plan: $CABALPLAN
jobs: $JOBS
Cabal tests: $LIBTESTS
cabal-install tests: $CLITESTS
Expand All @@ -347,7 +339,6 @@ print_header print-tool-versions

timed $HC --version
timed $CABAL --version
timed $CABALPLAN --version

for EXTRAHC in $EXTRAHCS; do
timed $EXTRAHC --version
Expand All @@ -368,8 +359,11 @@ step_time_summary() {

step_build() {
print_header "build"
print_header "Step Build: dry run"
timed $CABALNEWBUILD $TARGETS --dry-run || exit 1
$CABALPLAN topo --builddir=$BUILDDIR || exit 1
Copy link
Member

@fgaz fgaz Apr 3, 2023

Choose a reason for hiding this comment

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

If we still want a list of packages in the plan (though not topologically sorted) we can do it with a jq command:

jq -r '."install-plan" | map(."pkg-name" + "-" + ."pkg-version" + " " + ."component-name") | join("\n")' "$BUILDDIR/cache/plan.json"

iirc it's installed by default on github runners

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH I don't see a big value in this list. If someone feels strongly about it, I could add this oneliner, perhaps. But I'm not a fan of this solution either: it looks rather fragile to my uniformed taste.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I guess, it was requested here: #8440 so there are users for it...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since #8440 was opened by @andreasabel, paging him in... Andreas, maybe you have a preference between:

  1. keeping cabal-plan; possibly, switching to binary distribution to save time and dependency management;
  2. removing cabal-plan and print the list of dependencies to be built using the on-liner provided above by fgaz;
  3. removing cabal-plan and keeping the list of dependencies to be built that is already produced by cabal build [--dry-run] actually; (see e.g. here)

I personally lean towards (3) as I fail to see much advantage of anything else. One difference between cabal build --dry-run and cabal-plan topo that I noticed today (and you can check it using the CI link above) is that the latter seems to print nearly twice as much as the former. I'm not sure why that is...

Copy link
Member

@fgaz fgaz Apr 6, 2023

Choose a reason for hiding this comment

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

One difference between cabal build --dry-run and cabal-plan topo that I noticed today (and you can check it using the CI link above) is that the latter seems to print nearly twice as much as the former. I'm not sure why that is...

cabal build does not print cached stuff, it only prints what it's going to download/build

If we want the whole build plan we need 1. or 2. (or cabal freeze I guess)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good to know, thank you!

I don't see much emotion one way or another, so I'm just leaving the current version (with your oneliner) for the review...

Copy link
Member

Choose a reason for hiding this comment

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

Where is the one-liner replacement for the cabal-plan printing? I cannot see it, I only see the deletion of the cabal-plan step.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andreasabel oops: forgot to commit. Fixed.

print_header "Step Build: full build plan (cached and to-be-built dependencies):"
jq -r '."install-plan" | map(."pkg-name" + "-" + ."pkg-version" + " " + ."component-name") | join("\n")' "$BUILDDIR/cache/plan.json"
print_header "Step Build: actual build"
timed $CABALNEWBUILD $TARGETS || exit 1
}

Expand All @@ -386,22 +380,22 @@ timed doctest -package-env=doctest-Cabal --fast Cabal/Distribution Cabal/Languag
step_lib_tests() {
print_header "Cabal: tests"

CMD="$($CABALPLANLISTBIN Cabal-tests:test:unit-tests) $TESTSUITEJOBS --hide-successes --with-ghc=$HC"
CMD="$($CABALLISTBIN Cabal-tests:test:unit-tests) $TESTSUITEJOBS --hide-successes --with-ghc=$HC"
(cd Cabal-tests && timed $CMD) || exit 1

CMD="$($CABALPLANLISTBIN Cabal-tests:test:check-tests) $TESTSUITEJOBS --hide-successes"
CMD="$($CABALLISTBIN Cabal-tests:test:check-tests) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1

CMD="$($CABALPLANLISTBIN Cabal-tests:test:parser-tests) $TESTSUITEJOBS --hide-successes"
CMD="$($CABALLISTBIN Cabal-tests:test:parser-tests) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1

CMD="$($CABALPLANLISTBIN Cabal-tests:test:rpmvercmp) $TESTSUITEJOBS --hide-successes"
CMD="$($CABALLISTBIN Cabal-tests:test:rpmvercmp) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1

CMD="$($CABALPLANLISTBIN Cabal-tests:test:no-thunks-test) $TESTSUITEJOBS --hide-successes"
CMD="$($CABALLISTBIN Cabal-tests:test:no-thunks-test) $TESTSUITEJOBS --hide-successes"
(cd Cabal-tests && timed $CMD) || exit 1

CMD=$($CABALPLANLISTBIN Cabal-tests:test:hackage-tests)
CMD=$($CABALLISTBIN Cabal-tests:test:hackage-tests)
(cd Cabal-tests && timed $CMD read-fields) || exit 1
if $HACKAGETESTSALL; then
(cd Cabal-tests && timed $CMD parsec) || exit 1
Expand All @@ -418,14 +412,14 @@ fi
step_lib_suite() {
print_header "Cabal: cabal-testsuite"

CMD="$($CABALPLANLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR $TESTSUITEJOBS --with-ghc=$HC --hide-successes"
CMD="$($CABALLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR $TESTSUITEJOBS --with-ghc=$HC --hide-successes"
(cd cabal-testsuite && timed $CMD) || exit 1
}

step_lib_suite_extras() {
for EXTRAHC in $EXTRAHCS; do

CMD="$($CABALPLANLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR $TESTSUITEJOBS --with-ghc=$EXTRAHC --hide-successes"
CMD="$($CABALLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR $TESTSUITEJOBS --with-ghc=$EXTRAHC --hide-successes"
(cd cabal-testsuite && timed $CMD) || exit 1

done
Expand All @@ -438,19 +432,19 @@ step_cli_tests() {
print_header "cabal-install: tests"

# this are sorted in asc time used, quicker tests first.
CMD="$($CABALPLANLISTBIN cabal-install:test:long-tests) $TESTSUITEJOBS --hide-successes"
CMD="$($CABALLISTBIN cabal-install:test:long-tests) $TESTSUITEJOBS --hide-successes"
(cd cabal-install && timed $CMD) || exit 1

# This doesn't work in parallel either
CMD="$($CABALPLANLISTBIN cabal-install:test:unit-tests) -j1 --hide-successes"
CMD="$($CABALLISTBIN cabal-install:test:unit-tests) -j1 --hide-successes"
(cd cabal-install && timed $CMD) || exit 1

# Only single job, otherwise we fail with "Heap exhausted"
CMD="$($CABALPLANLISTBIN cabal-install:test:mem-use-tests) -j1 --hide-successes"
CMD="$($CABALLISTBIN cabal-install:test:mem-use-tests) -j1 --hide-successes"
(cd cabal-install && timed $CMD) || exit 1

# This test-suite doesn't like concurrency
CMD="$($CABALPLANLISTBIN cabal-install:test:integration-tests2) -j1 --hide-successes --with-ghc=$HC"
CMD="$($CABALLISTBIN cabal-install:test:integration-tests2) -j1 --hide-successes --with-ghc=$HC"
(cd cabal-install && timed $CMD) || exit 1
}

Expand All @@ -460,7 +454,7 @@ CMD="$($CABALPLANLISTBIN cabal-install:test:integration-tests2) -j1 --hide-succe
step_cli_suite() {
print_header "cabal-install: cabal-testsuite"

CMD="$($CABALPLANLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR --with-cabal=$($CABALPLANLISTBIN cabal-install:exe:cabal) $TESTSUITEJOBS --with-ghc=$HC --hide-successes"
CMD="$($CABALLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TESTSUITE_BDIR --with-cabal=$($CABALLISTBIN cabal-install:exe:cabal) $TESTSUITEJOBS --with-ghc=$HC --hide-successes"
(cd cabal-testsuite && timed $CMD) || exit 1
}

Expand All @@ -470,15 +464,15 @@ CMD="$($CABALPLANLISTBIN cabal-testsuite:exe:cabal-tests) --builddir=$CABAL_TEST
step_solver_benchmarks_tests() {
print_header "solver-benchmarks: test"

CMD="$($CABALPLANLISTBIN solver-benchmarks:test:unit-tests)"
CMD="$($CABALLISTBIN solver-benchmarks:test:unit-tests)"
(cd Cabal && timed $CMD) || exit 1
}

step_solver_benchmarks_run() {
print_header "solver-benchmarks: run"

SOLVEPKG=Chart-diagrams
CMD="$($CABALPLANLISTBIN solver-benchmarks:exe:hackage-benchmark) --cabal1=$CABAL --cabal2=$($CABALPLANLISTBIN cabal-install:exe:cabal) --trials=5 --packages=$SOLVEPKG --print-trials"
CMD="$($CABALLISTBIN solver-benchmarks:exe:hackage-benchmark) --cabal1=$CABAL --cabal2=$($CABALLISTBIN cabal-install:exe:cabal) --trials=5 --packages=$SOLVEPKG --print-trials"
(cd Cabal && timed $CMD) || exit 1
}

Expand Down