Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#16048 closed defect (fixed)

dev docs: Inclusion Procedure for New and Updated Packages

Reported by: rws Owned by:
Priority: major Milestone: sage-6.2
Component: documentation Keywords:
Cc: Merged in:
Authors: Ralf Stephan, Vincent Delecroix Reviewers: R. Andrew Ohana, Jeroen Demeyer, Vincent Delecroix, Ralf Stephan
Report Upstream: N/A Work issues:
Branch: ecba6b6 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

As discussed in

https://groups.google.com/forum/?hl=en#!topic/sage-devel/jopuoWO1Fvk

clear documentation on the procedure is missing.

Change History (21)

comment:1 Changed 8 years ago by rws

  • Branch set to public/spkg-devdoc

comment:2 Changed 8 years ago by git

  • Commit set to bf1291ad35edb65eb87a45a61d2515f6f614c838

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

bf1291aTrac 16048: Inclusion Procedure for New and Updated Packages

comment:3 Changed 8 years ago by rws

  • Status changed from new to needs_review

comment:4 Changed 8 years ago by ohanar

There is a bit of redundancy in your first paragraph and the 3rd about a package requiring a year before it could be considered for a standard package. Additionally, experimental packages aren't really ever eligible to become standard packages since standard packages need to support all of Sage's supported platforms (and most things that fall into the experimental category are things that break on various platforms).

Also, I don't know if it is mentioned in any of the surrounding documentation, but standard packages need to be GPLv3+ compatible, so including that somewhere would be good.

comment:5 Changed 8 years ago by git

  • Commit changed from bf1291ad35edb65eb87a45a61d2515f6f614c838 to 5e72e74e1cf869f391d2a53a07cb4c58489767a6

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

5e72e7416048: minor adaptations

comment:6 Changed 8 years ago by git

  • Commit changed from 5e72e74e1cf869f391d2a53a07cb4c58489767a6 to a05ec467e50d8a5d7d49e056e03f40ad3b3460fe

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

a5f8a33Merge branch 'develop' into t/16048/public/spkg-devdoc
a05ec4616048: add paragraph about ready to use spkgs

comment:7 Changed 8 years ago by git

  • Commit changed from a05ec467e50d8a5d7d49e056e03f40ad3b3460fe to 77ce76e58c464472313c9836f08c31a8e1836c82

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

77ce76e16048: fixes

comment:8 Changed 8 years ago by vbraun

Does having both a SPKG and a build/pkg/foo directory work? Which spkg-install script is being used? I don't even know if this works, but it shouldn't. You can either have a legacy spkg or make a git branch with an upstream tarball (gz/bz2).

comment:9 Changed 8 years ago by rws

  • Branch changed from public/spkg-devdoc to public/spkg-devdoc-1
  • Commit changed from 77ce76e58c464472313c9836f08c31a8e1836c82 to 469f971e9bd04ddf19ad8e2e42afb0dea643d72d

OK, but what to do if the supporting files in the published SPKG are defect, see #15813?


New commits:

d993164Trac 16048: Inclusion Procedure for New and Updated Packages
1db8cd716048: minor adaptations
b23109216048: add paragraph about ready to use spkgs
bec1ced16048: fixes
469f97116048: improve previous change

comment:10 Changed 8 years ago by jdemeyer

  • Status changed from needs_review to needs_work

I really don't understand what you're trying to say with

A special case where no patch would be necessary is when an author
provides an already fine SPKG on the net which includes all files
needed for ``SAGE_ROOT/build/pkgs/foo`` and the source in its ``src/``
subdirectory. Here it suffices to put the web link to the package
into the ticket. Alternatively, you would have to ask the author to
make a tarball of the ``src/`` subdirectory, and upload a branch with
``SAGE_ROOT/build/pkgs/foo``.

The first part of the patch is good for me.

comment:11 follow-up: Changed 8 years ago by vdelecroix

Hi,

0) It would be nice to start with a section explaining how the installation procedure works ("how does this work" or "the big picture") and make appropriate pointers to the install scripts. Part of it is already explained in "Directory structure".

1) There is a lot of intersection between your new section "Procedure for ..." and the "Prerequisites". It would make more sense to have only one.

2) About "Testing". Firstly there is a typo ``SAGE_ROOT/upstream/` should be closed with two backquotes. Secondly, the name of the section is not very appropriate as it intersects with "Self-test". It would also be better if the content of this section appear inside "how this work" mentioned in 0).

Vincent

Last edited 8 years ago by vdelecroix (previous) (diff)

comment:12 Changed 8 years ago by git

  • Commit changed from 469f971e9bd04ddf19ad8e2e42afb0dea643d72d to 42e05b1e7a1655e8c7b29712340e922029be05ba

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

42e05b116048: enhancements; typos fixed

comment:13 in reply to: ↑ 11 Changed 8 years ago by rws

  • Reviewers set to R. Andrew Ohanar, Jeroen Demeyer, Vincent Delecroix
  • Status changed from needs_work to needs_review

Thanks to all reviewers so far.

Replying to vdelecroix:

1) There is a lot of intersection between your new section "Procedure for ..." and the "Prerequisites". It would make more sense to have only one.

I will take the liberty to disagree here, for educational reasons.

Everything else was fixed. Anything amiss in this section?

comment:14 follow-up: Changed 8 years ago by vdelecroix

  • Status changed from needs_review to needs_work

I am very sorry to be very picky but I definitely think that the documentation is an important piece of a software. Please for each of my comment, say explicitly if you agree or not (you did it for my 1) before but you mostly eluded the other two). I am happy to make the changes you accept by myself if you prefer.

section "Inclusion Procedure for New and Updated Packages"

As it is the case in all other sections, it would be nice if you add a line break after the title.

For the politics about tickets, I thought that:

  • experimental = does not build on all platforms officially supported => never be optional
  • optional = does build on all platforms => can possibly become standard after some time

The sentence "In short, packages consist of a link to the original tarball with additional code under ``SAGE_ROOT/build/pkgs``" is not meaningful at this stage of the document. It is not clear what is a "package" nor what is an "original tarball". I would simply remove the sentence as this section is purely about "how do you submit" and not "how do you make your own package". Moreover, I would would better put this section near the end of the document because before submitting a package you need to build one ;-).

section "Directory structure"

In the sentence "to package it in Sage" you must uppercase the "t". At the very same place I suggest to put a comment in parenthesis "... distribute a tarball ``foo-1.3.tar.gz`` (that will be automatically placed in ``SAGE_ROOT/upstream`` during the installation process)" or something similar.

section "Manual package build and installation"

I think that the shell command

$ SAGE_CHECK=yes sage -f package_name

will not do what you expect. You might either put a column between the SAGE_CHECK=yes and the sage -f package_name or put it in two lines.

There are at least 5 important missing things:

  • sage -i PACKAGE_NAME is the standard command to install a package
  • if a package is already installed, we can perform the tests only with sage -i -c PACKAGE_NAME (what the -c option does is that it set SAGE_CHECK to yes + some extra mystery about SAGE_CHECK_PACKAGES)
  • to force the reinstallation of a package we can do sage -i -f PACKAGE_NAME (and sage -f PACKAGE_NAME is a shortcut for it)
  • the install script is in $SAGE_ROOT/local/bin/install (and Sage reinvented the wheel here!!). It is relatively well documented and is worth to points its existence.

comment:15 Changed 8 years ago by git

  • Commit changed from 42e05b1e7a1655e8c7b29712340e922029be05ba to b0a7ee5d74544e793db3c907de8900895b2513c9

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

b0a7ee516048: more changes

comment:16 in reply to: ↑ 14 Changed 8 years ago by rws

Thanks for your review.

Replying to vdelecroix:

I am very sorry to be very picky

Don't.

As it is the case in all other sections, it would be nice if you add a line break after the title.

Done.

  • experimental = does not build on all platforms officially supported => never be optional

Yes, done.

The sentence "In short, packages consist of a link to the original tarball with additional code under ``SAGE_ROOT/build/pkgs``" is not meaningful at this stage of the document. It is not clear what is a "package" nor what is an "original tarball". I would simply remove the sentence as this section is purely about "how do you submit" and not "how do you make your own package".

Done.

Moreover, I would would better put this section near the end of the document because before submitting a package you need to build one ;-).

I was not sure how to do this.

In the sentence "to package it in Sage" you must uppercase the "t". At the very same place I suggest to put a comment in parenthesis "... distribute a tarball ``foo-1.3.tar.gz`` (that will be automatically placed in ``SAGE_ROOT/upstream`` during the installation process)" or something similar.

Both done.

$ SAGE_CHECK=yes sage -f package_name

will not do what you expect.

I replaced it with sage -i -c PACKAGE_NAME.

There are at least 5 important missing things:

I was unsure how to include this. Please feel free to add it and the other issue above yourself.

comment:17 Changed 8 years ago by vdelecroix

  • Branch changed from public/spkg-devdoc-1 to public/spkg-devdoc-2
  • Commit changed from b0a7ee5d74544e793db3c907de8900895b2513c9 to ecba6b6283c17800eb5bb7d9e8f55a79428b67ec
  • Status changed from needs_work to needs_review

Hi Ralf,

I added few sentences in the introduction about the install script and the organization of the document. I also reorganize the sections (not changing them).

In my second commit I just set the line width to 72.

Tell me if you think it is better (as I am not a native english speaker there might be some mistakes). Otherwise we can just go back to the previous branch public/spkg-devdoc-1 which is just two commits before.

Best Vincent


New commits:

56de113reorganisation + mention of install scripts in packaging.rst
ecba6b6set line width to 72 characters in packaging.rst

comment:18 follow-up: Changed 8 years ago by rws

  • Authors changed from Ralf Stephan to Ralf Stephan, Vincent Delecroix
  • Reviewers changed from R. Andrew Ohanar, Jeroen Demeyer, Vincent Delecroix to R. Andrew Ohanar, Jeroen Demeyer, Vincent Delecroix, Ralf Stephan
  • Status changed from needs_review to positive_review

I think this is fine and I like it better. If you don't mind I'll set the ticket fields accordingly.

comment:19 in reply to: ↑ 18 Changed 8 years ago by vdelecroix

Replying to rws:

I think this is fine and I like it better. If you don't mind I'll set the ticket fields accordingly.

Prefect, thanks!

comment:20 Changed 8 years ago by vbraun

  • Branch changed from public/spkg-devdoc-2 to ecba6b6283c17800eb5bb7d9e8f55a79428b67ec
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:21 Changed 7 years ago by kcrisman

  • Commit ecba6b6283c17800eb5bb7d9e8f55a79428b67ec deleted
  • Reviewers changed from R. Andrew Ohanar, Jeroen Demeyer, Vincent Delecroix, Ralf Stephan to R. Andrew Ohana, Jeroen Demeyer, Vincent Delecroix, Ralf Stephan
Note: See TracTickets for help on using tickets.