Opened 3 years ago

Closed 3 years ago

#26660 closed defect (fixed)

Fix changes from #25857 that were accidentally reverted by #25188

Reported by: embray Owned by:
Priority: blocker Milestone: sage-8.5
Component: build Keywords:
Cc: dimpase, vbraun Merged in:
Authors: Erik Bray Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 8fec43d (Commits, GitHub, GitLab) Commit: 8fec43df1fd2816432b763166361c1836e579e6e
Dependencies: Stopgaps:

Status badges


As I explained a bit here, #25857 ad #25188 had some interdependency, but were merged in the wrong order.

#25857 was merged for 8.5.beta1, then #25188 was merged in 8.5.beta2. However, the latter contained in it an old version of the former, which ended up taking precedence in the merge resolution.

This resulted in some changes being undone:

$ git diff 8.5.beta1..8.5.beta2 -- build/make/deps
  • build/make/deps

    diff --git a/build/make/deps b/build/make/deps
    index a3abca3..bbe3277 100644
    a b toolchain: $(foreach pkgname,$(TOOLCHAIN),$(inst_$(pkgname))) 
    7575# unconditionally. We still use the dependency checking from $(MAKE),
    7676# so this will not trigger useless rebuilds.
    7777# See #14168 and #14232.
    78 #
    79 # Note: This list consists of only the *runtime* dependencies of the toolchain.
    80 TOOLCHAIN_DEPS = zlib $(MP_LIBRARY) mpfr mpc
    82        $(foreach pkgname,$(TOOLCHAIN_DEPS),$(inst_$(pkgname)))
     78# Note: This list is determined from the dependencies of the TOOLCHAIN
     79# packages which include gcc, and optionally ccache; in principle this
     80# list is redundant.
     81TOOLCHAIN_DEPS = zlib xz $(MP_LIBRARY) mpfr mpc
    85        for target in $(TOOLCHAIN_DEP_INSTS); do \
    86            $(MAKE) $$target; \
     83       for pkg in $(TOOLCHAIN_DEPS); do \
     84           $(MAKE) $$pkg; \
    8785       done
    8987all-toolchain: base-toolchain

Change History (7)

comment:1 Changed 3 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/build/ticket-25188-fix
  • Commit set to 8fec43df1fd2816432b763166361c1836e579e6e
  • Status changed from new to needs_review

New commits:

8fec43dTrac #26660: restore changes originally from #25857 that were accidentally

comment:2 Changed 3 years ago by embray

  • Cc dimpase vbraun added

comment:3 Changed 3 years ago by dimpase

  • Status changed from needs_review to positive_review

I'd remove from this branch, as there you just add whitespace. Otherwise, looks good and works.

comment:4 Changed 3 years ago by chapoton

reviewer name missing

comment:5 Changed 3 years ago by dimpase

  • Reviewers set to Dima Pasechnik

thanks for the reminder!

comment:6 Changed 3 years ago by embray

The whitespace added there was intentional--it was unintentionally removed same as the other changes added back in by this ticket. In this case the whitespace is meaningful--it makes debug output more readable.

comment:7 Changed 3 years ago by vbraun

  • Branch changed from u/embray/build/ticket-25188-fix to 8fec43df1fd2816432b763166361c1836e579e6e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.