Skip to content
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

Add documentation of tox and GitHub actions workflow to developer's manual #29401

Closed
mkoeppe opened this issue Mar 24, 2020 · 72 comments
Closed

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Mar 24, 2020

CC: @dimpase @jhpalmieri @videlec @tscrim @orlitzky @kiwifb @slel

Component: documentation

Author: Matthias Koeppe

Branch/Commit: 99aca39

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29401

@mkoeppe mkoeppe added this to the sage-9.1 milestone Mar 24, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

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

5061729add info on [GitHub](../wiki/GitHub) Actions runners

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

Commit: 5061729

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

Author: Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

comment:4

doc does not build.

in portability_testing.rst warning:
Could not lex literal_block as "shell-session".

perhaps it needs tagging as yaml, somehow

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

comment:5
--- a/src/doc/en/developer/portability_testing.rst
+++ b/src/doc/en/developer/portability_testing.rst
@@ -9,7 +9,7 @@ Testing on multiple platforms
 =============================
 
 Sage is intended to build and run on a variety of platforms,
-including all major Linux distributions, as well as macOS, and
+including all major Linux distributions, as well as MacOS, and
 Windows (with Cygwin).
 
 There is considerable variation between these platforms.
@@ -142,7 +142,9 @@ Using Sage's database of distribution prerequisites
 
 The source code of the Sage distribution contains a database of
 package names in various distributions' package managers.  For
-example, the file ``build/pkgs/debian.txt`` contains the following::
+example, the file ``build/pkgs/debian.txt`` contains the following
+
+.. code-block:: yaml
 
   # This file, build/pkgs/debian.txt, contains names of Debian/Ubuntu packages
   # needed for installation of Sage from source.

please apply this patch - then you can set it to positive review.

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

Reviewer: Dima Pasechnik

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

Changed commit from 5061729 to a4ce2d4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

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

a4ce2d4Reviewer patch

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

comment:7

I'd add a link to https://wiki.sagemath.org/patchbot in the Sage patchbots section, and there are more typos and formatting issues. Let me post another patch...

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

reviewer patch 2

@dimpase
Copy link
Member

dimpase commented Mar 26, 2020

comment:8

Attachment: moar.diff.gz

please apply this, too

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

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

71bf833Reviewer patch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

Changed commit from a4ce2d4 to 71bf833

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

comment:10

Thank you!

@jhpalmieri
Copy link
Member

comment:13

Trivial request:

diff --git a/src/doc/en/developer/portability_testing.rst b/src/doc/en/developer/portability_testing.rst
index 20737e3ce1..16ea65340c 100644
--- a/src/doc/en/developer/portability_testing.rst
+++ b/src/doc/en/developer/portability_testing.rst
@@ -12,7 +12,7 @@ Sage is intended to build and run on a variety of platforms,
 including all major Linux distributions, as well as MacOS, and
 Windows (with Cygwin).
 
-There is considerable variation between these platforms.
+There is considerable variation among these platforms.
 To ensure that Sage continues to build correctly on users'
 machines, it is crucial to test changes to Sage, in particular
 when external packages are added or upgraded, on a wide

Some people may also complain that some of the lines are longer than 80 characters. I don't care much about this.

I just tried out the instructions for Docker. When I was just starting, apt-get install ... wasn't working: I needed to run apt-get update first (as in https://stackoverflow.com/questions/27273412/cannot-install-packages-inside-docker-ubuntu-image). Should this command be added?

In the "Generating Dockerfiles" section: am I understanding that you don't need to create a new git worktree with this approach? That's not completely clear as written.

@jhpalmieri
Copy link
Member

comment:14

Also in the "Generating Dockerfiles" section: I tried

$ build/bin/write-dockerfile.sh debian "@(standard|optional)" > ../Dockerfile
$ docker build . -f ../Dockerfile ...

so as not to pollute my Sage directory with untracked files, but it didn't work:

ERRO[0000] Can't add file /Users/palmieri/Desktop/Sage_stuff/git/sage/.git/TEMP/rebase-apply/0001 to tar: io: read/write on closed pipe 
ERRO[0000] Can't close tar writer: io: read/write on closed pipe 
Sending build context to Docker daemon  37.89kB
error during connect: Post http://%2Fvar%2Frun%2Fdocker.sock/v1.40/build?buildargs=%7B%22BASE_IMAGE%22%3A%22ubuntu-latest-sage%22%2C%22NUMPROC%22%3A%224%22%7D&cachefrom=%5B%5D&cgroupparent=&cpuperiod=0&cpuquota=0&cpusetcpus=&cpusetmems=&cpushares=0&dockerfile=.dockerfile.13cd21991950b4666007&labels=%7B%7D&memory=0&memswap=0&networkmode=default&rm=1&shmsize=0&target=&ulimits=null&version=1: archive/tar: write too long

Giving an absolute path didn't help.

Also, once I get this going, where does the build take place? That is, can I look at the log files without running a second iteration of docker?

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

comment:15

Docker contexts are tricky!

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

comment:16

It's easiest to put the Dockerfile somewhere in the tree.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 26, 2020

comment:17

Thanks for the feedback. I'll revise the writing to address your questions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2020

Changed commit from 71bf833 to f7954db

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Changed commit from 8698126 to b7b45fe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

7eac94amore on [GitHub](../wiki/GitHub) Actions
426aec1More explanation regarding mac / linux
43195f6add links to docker docs
3d2c3faadd crossref to Installation Guide
6153945merge the sections on tox factors
ab11ebdelide some details from terminal output
c7fcf91change 'ls -l' to 'ls -la' and elide some details
e73bc8dElide more details, shorten directory names more
a31b2a0Reviewer patch
b7b45feReviewer patch

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 29, 2020

comment:46

Rebased on 9.1.beta9, needs review

@dimpase
Copy link
Member

dimpase commented Mar 29, 2020

comment:47

unfinished edit here at line 570, ending with several

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Changed commit from b7b45fe to f7d2dc9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

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

f7d2dc9Remove old section head

@dimpase
Copy link
Member

dimpase commented Mar 29, 2020

comment:49

make this a proper list:

--- a/src/doc/en/developer/portability_testing.rst
+++ b/src/doc/en/developer/portability_testing.rst
@@ -854,13 +854,12 @@ workflow have finished.  Each job generates one tarball.
 during the build.
 
 The following procedure seems to work well for testing branches during
-development.  Create a branch from a recent beta release that contains
-the default GitHub Actions configuration; name it ``TESTER``, say.
-Edit ``$SAGE_ROOT/.github/workflows/tox.yml`` to include the system
-configurations that you wish to test.  Commit and push the branch to
-your GitHub fork of sage.  Next, push your development branch to your
-GitHub repository and create a pull request against the ``TESTER``
-branch.  This will trigger the GitHub Actions workflow.
+development:
+
+- Create a branch from a recent beta release that contains the default GitHub Actions configuration; name it ``TESTER``, say.
+- Edit ``$SAGE_ROOT/.github/workflows/tox.yml`` to include the system configurations that you wish to test.
+- Commit and push the branch to your GitHub fork of sage.
+- Push your development branch to your GitHub repository and create a pull request against the ``TESTER`` branch. This will trigger the GitHub Actions workflow.

also, there are few lines in the file after this, which do not go into the output.
Perhaps, just remove them.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

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

99aca39Reformat list, remove unfinished commented out section

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Changed commit from f7d2dc9 to 99aca39

@dimpase
Copy link
Member

dimpase commented Mar 29, 2020

comment:51

OK. Squash these many commits, I guess, too.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Changed commit from 99aca39 to aba215d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

aba215dsrc/doc/en/developer/portability_testing.rst: New chapter. (squashed)

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 29, 2020

comment:53

Thank you!

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 30, 2020

comment:54

Looks like @vbraun has merged the unsquashed branch already. Resetting to that.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2020

Changed commit from aba215d to 99aca39

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 30, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. Last 10 new commits:

43195f6add links to docker docs
3d2c3faadd crossref to Installation Guide
6153945merge the sections on tox factors
ab11ebdelide some details from terminal output
c7fcf91change 'ls -l' to 'ls -la' and elide some details
e73bc8dElide more details, shorten directory names more
a31b2a0Reviewer patch
b7b45feReviewer patch
f7d2dc9Remove old section head
99aca39Reformat list, remove unfinished commented out section

@vbraun
Copy link
Member

vbraun commented Apr 5, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants