Opened 2 years ago

Closed 2 years ago

#28721 closed enhancement (fixed)

Document sage-dist-helpers

Reported by: jhpalmieri Owned by:
Priority: minor Milestone: sage-9.0
Component: documentation Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 33abf8f (Commits, GitHub, GitLab) Commit: 33abf8f4c01a7c2f5e2ca0ae4abefbad6267b5bc
Dependencies: Stopgaps:

Status badges

Description

Document the bash functions sdh_make and friends in the Sage developer's guide. (Currently they are only documented in their source file, build/bin/sage-dist-helpers.)

Change History (13)

comment:1 Changed 2 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/sdh-docs

comment:2 Changed 2 years ago by jhpalmieri

  • Commit set to bbd044a85231ed4834c3aeccf3c69c38cc95ff20
  • Status changed from new to needs_review

New commits:

bbd044atrac 28721: document the functions in sage-dist-helpers.

comment:3 follow-up: Changed 2 years ago by dimpase

The pattern sdh_XYZ || sdh_die ... one sees in few places is not necessary, and sdh_XYZ alone suffices, isn't it?

By looking at the code of e.g. sdh_configure one sees that sdh_die is actually called in sdh_configure, and so chaining them as above is pointless (the part after || won't be called).

This should be mentioned in the docs.

Last edited 2 years ago by dimpase (previous) (diff)

comment:4 follow-up: Changed 2 years ago by embray

By the way, there was already a ticket for this task at #24023.

Something I wanted to do for that was to have a way to automatically include the in-line docs in sage-dist-helpers into the Sphinx docs, but I would have needed to write a plugin, and didn't get to do it.

comment:5 Changed 2 years ago by embray

In any case, this is definitely good enough for now, thank you for taking care of it.

comment:6 in reply to: ↑ 4 ; follow-up: Changed 2 years ago by jhpalmieri

Replying to embray:

By the way, there was already a ticket for this task at #24023.

Oops, sorry.

Something I wanted to do for that was to have a way to automatically include the in-line docs in sage-dist-helpers into the Sphinx docs, but I would have needed to write a plugin, and didn't get to do it.

Maybe we can leave #24023 open for that.

comment:7 in reply to: ↑ 6 Changed 2 years ago by embray

Replying to jhpalmieri:

Replying to embray:

By the way, there was already a ticket for this task at #24023.

Oops, sorry.

No problem. Personally I sometimes even prefer opening separate tickets for specific solutions to a problem from the ticket for the problem itself.

Something I wanted to do for that was to have a way to automatically include the in-line docs in sage-dist-helpers into the Sphinx docs, but I would have needed to write a plugin, and didn't get to do it.

Maybe we can leave #24023 open for that.

That makes sense.

comment:8 Changed 2 years ago by git

  • Commit changed from bbd044a85231ed4834c3aeccf3c69c38cc95ff20 to 33abf8f4c01a7c2f5e2ca0ae4abefbad6267b5bc

Branch pushed to git repo; I updated commit sha1. New commits:

33abf8ftrac 28721: do not combine sdh_BLAH with sdh_die

comment:9 in reply to: ↑ 3 Changed 2 years ago by jhpalmieri

Replying to dimpase:

The pattern sdh_XYZ || sdh_die ... one sees in few places is not necessary, and sdh_XYZ alone suffices, isn't it?

By looking at the code of e.g. sdh_configure one sees that sdh_die is actually called in sdh_configure, and so chaining them as above is pointless (the part after || won't be called).

This should be mentioned in the docs.

I added a note about this.

comment:10 Changed 2 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

Thanks. Looks good to me; the still missing part is package de-installation, and the related JSON files, but it can be on another ticket.

comment:11 Changed 2 years ago by dimpase

specifically about JSON files, de-installation is handled by build/sage_bootstrap/uninstall.py, and there are good docs there, but I don't know where these JSON files are written.

I have a vague recollection I knew it once, but it's not clear to me now, without digging through the whole command chain and makefiles... :-(

comment:12 Changed 2 years ago by jhpalmieri

Seems to be written by build/bin/sage-spkg, starting at line 1000:

PKG_NAME_INSTALLED="$SAGE_SPKG_INST/$PKG_NAME"
cat > "$PKG_NAME_INSTALLED" << __EOF__
{
    "package_name": "$PKG_BASE",
    "package_version": "$PKG_VER",
    "install_date": "$(date)",
    "system_uname": "$(uname -a)",
    "sage_version": "$(cat "${SAGE_ROOT}/VERSION.txt")",
    "test_result": "$TEST_SUITE_RESULT",
    "files": [
        $FILE_LIST
    ]
}
__EOF__

The file manifest is constructed at line 899-900:

    # Generate installed file manifest
    FILE_LIST="$(cd "$PREFIX" && find . -type f -o -type l | sed 's|^\./||' | sort)"

comment:13 Changed 2 years ago by vbraun

  • Branch changed from u/jhpalmieri/sdh-docs to 33abf8f4c01a7c2f5e2ca0ae4abefbad6267b5bc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.