Merging fix for github CI

[David Roe]

Hi all,

Currently, almost all of the PRs on github aren't passing CI, and thus have red Xs.  The problem can be resolved by merging #34964 and #34987.  Several of us at Sage Days 117 propose to merge these two PRs into the develop branch on Github tonight, to help improve the reviewing process for everyone here, as well as for people who are getting used to the new Github workflow.  We don't anticipate this becoming a regular occurrence (it's mainly a consequence of the slightly different testing process on trac and github).

If we hear no objections, we'll proceed in an hour and a half (8:45pm CET, 2:45pm EST, 11:45am PST).

David

[John Cremona]

Strong yes from me.

#34987 concerns a 3-line doctest where only the first has a #long time tag thought the other lines depend on the first having been run, so causes a failure when tested without --long.  The fix adds two # long time tags

#34964 trims some whitespace and changes "" to r""" in a few necessary places.

--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAChs6_mnuBi-qd%3Da0%3DqAGt8Xp6SohQzWdtw%2B04t82MPq68ntPw%40mail.gmail.com.

[Dima Pasechnik]

done

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAD0p0K5c%2BOYxf8EAJ5bAA6cJUuzoMtpqwDCOHuKcEd1zjNfMrA%40mail.gmail.com.

[David Roe]

Thanks Dima!

There is now an "Update branch" button at the bottom of each PR, which you can press if you're the originator of the PR and it will merge in develop.

David

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAAWYfq3hp_TRB9ALs__jutcP_us3TA%3DGUxthMS%3DNYGN1KzNkPw%40mail.gmail.com.

[Dima Pasechnik]

On Tue, Feb 7, 2023 at 10:42 PM David Roe <roed...@gmail.com> wrote:
>
> Thanks Dima!
>
> There is now an "Update branch" button at the bottom of each PR, which you can press if you're the originator of the PR and it will merge in develop

(there is an option to rebase rather than to merge on this button -
although automatic rebase is less guaranteed)

I was impatient and I pressed this for a number of PRs myself :-)
I see now CI passing for quite a number, looks quite good.

Dima

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/CAChs6_n392HmzN_7rnfeU_3%3DZLgC444LmGc7vQKAuseJq9POmg%40mail.gmail.com.

[Martin R]

I cannot see any "update branch" button on https://github.com/sagemath/sage/pull/34974

Where should I look for it?

Martin

[Dima Pasechnik]

On Wed, 8 Feb 2023, 08:41 'Martin R' via sage-devel, <sage-...@googlegroups.com> wrote:

I cannot see any "update branch" button on https://github.com/sagemath/sage/pull/34974

Where should I look for it?

This button is somewhere near the ticket status at the bottom - if your base is indeed outdated.

But check the commits on the branch.

I went on the merging spree last night, to gauge the performance of the CI a bit.

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/4e9ab964-b760-4fda-a195-d1e56e8bbf86n%40googlegroups.com.

[Martin R]

Since the failures are

sage -t --random-seed=130442615951932153031063473470828108756 sage/schemes/elliptic_curves/ell_curve_isogeny.py # 1 doctest failed

sage -t --random-seed=130442615951932153031063473470828108756 sage/schemes/elliptic_curves/ell_number_field.py # 2 doctests failed

I assume that the PR is based on an old branch, but the PR page does not contain "update branch"..  It does contain "Merging is blocked", but I don't really understand what this means.

Martin

[Dima Pasechnik]

On Wed, Feb 8, 2023 at 10:23 AM 'Martin R' via sage-devel
<sage-...@googlegroups.com> wrote:
>
> Since the failures are
>
> sage -t --random-seed=130442615951932153031063473470828108756 sage/schemes/elliptic_curves/ell_curve_isogeny.py # 1 doctest failed
> sage -t --random-seed=130442615951932153031063473470828108756 sage/schemes/elliptic_curves/ell_number_field.py # 2 doctests failed
>
> I assume that the PR is based on an old branch, but the PR page does not contain "update branch".. It does contain "Merging is blocked", but I don't really understand what this means.

I just did
git fetch origin pull/34974/head
(with origin being the main new GitHub repo for sage)
to see the git log, and magically on https://github.com/sagemath/sage/pull/34974
there is now

This branch is out-of-date with the base branch
Merge the latest changes from develop into this branch.

and "Update branch" button just above "Merge is blocked"
(which is just a reflection of the fact that develop branch is
protected, you can't push/merge there (I can, as Admin, though))

Perhaps that was due to the base branch being so old, that no branch
info on that was computed by GitHub ahead of my fetch.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/096b1b96-6e0a-432a-bb25-5324777f301en%40googlegroups.com.

[Martin R]

Interesting, because the PR page is still the same.for me.

I feel so stupid, I'm sorry, I need more help.  In particular, I get:

martin@convex63:~/sage$ LANG=en git fetch origin pull/34974/head  
fatal: couldn't find remote ref pull/34974/head

(Note that this is not *my* pull request, but somebody else's, who is trying to provide a fix for my (easy) ticket, I want to serve as reviewer.  I do *not* want to give positive review to the pull request as it is currently.)

I cloned sage according to "Otherwise, start afresh:" at https://github.com/sagemath/trac-to-github/blob/master/docs/Migration-Trac-to-Github.md

I have:

martin@convex63:~/sage$ git remote -v
origin  g...@github.com:mantepse/sage.git (fetch)
origin  g...@github.com:mantepse/sage.git (push)
trac    https://github.com/sagemath/sagetrac-mirror.git (fetch)
trac    https://github.com/sagemath/sagetrac-mirror.git (push)
upstream        g...@github.com:sagemath/sage.git (fetch)
upstream        g...@github.com:sagemath/sage.git (push)

It is not clear to me what I should be doing to

1.) try the PR

2.) enable the linting, building and doc checks.

Please excuse me :-(

Martin

[Tobias Diez]

You need to use upstream (the official sage repo) instead of origin (your fork) to fetch the PR: git fetch upstream pull/PULL-REQUEST-ID/head:LOCAL-BRANCH-NAME see https://github.com/sagemath/trac-to-github/blob/master/docs/Migration-Trac-to-Github.md#for-reviewing-a-change

If you don't have an "update branch" button, then this means you currently don't have write access to the sagemath repo and thus cannot modify PRs (we still have to sort out all the permission stuff...). I've clicked the button for you now.

[Martin R]

Why would I need write access to the sagemath repo?

I would have thought that I'd "somehow" (no idea how) take over the branch and modify it.  Or are pull requests directly tied to a specific user?  In other words, what is the replacement for the following?

Alice:

git trac create "make sage even better"

...

git commit -m "initial attempt"

git trac push

Bob:

git trac checkout 34923

...editing...

git commit -m "improved attempt"

git trac push

Martin

[Dima Pasechnik]

On Wed, Feb 8, 2023 at 11:23 AM 'Martin R' via sage-devel
<sage-...@googlegroups.com> wrote:
>
> Why would I need write access to the sagemath repo?

you need to be on certain sagemath org team (in GitHub sense) called
"triage" to be able to do meaningful things, such a
reviewing/updating, with PRs.

At the moment not all of the hundreds of devs are there on this team.

> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/eb497a5d-4bbe-4a68-81ff-6f28eaf1d110n%40googlegroups.com.

[Martin R]

There must be some misunderstanding, I assume on my part. I *am* on the sagemath/triage team.  But for trying and modifying a pull request, I certainly do not want write access to any of the "official" sage branches.

I guess it will be best for me to just wait a little and exercise in patience.  The author of the pull request doesn't seem to be too eager anyway.

[David Roe]

On Wed, Feb 8, 2023 at 12:23 PM 'Martin R' via sage-devel <sage-...@googlegroups.com> wrote:

Why would I need write access to the sagemath repo?

I would have thought that I'd "somehow" (no idea how) take over the branch and modify it.  Or are pull requests directly tied to a specific user?  In other words, what is the replacement for the following?

Pull requests are from a specific user's fork of Sage. If you are not the author of a PR but want to suggest changes to it, you have several options.

1. You can make suggestions for changes within the review process.  The original author will then have the ability to accept these changes.  Here's a guide for doing this.

2. You can make your own PR onto their PR.  If they accept your changes, it will get incorporated into the original PR to Sage.  This requires no additional permissions, but does require that the original author merge your PR.  To do this, you can go to your own fork of sage on github and click on Pull Requests (for me, this takes me to https://github.com/roed314/sage/pulls).  If you click on the green "New pull request" button at the top right, it will open the interface for creating a PR.  By default it will make a pull request against the main sage develop branch, but you can change both the base repository (to the original author's fork) and the branch (to the branch from which the original PR is coming).

3. If you're collaborating closely with the author, they can give you permission to push to the branch from which the PR originates.  Then you could make your changes and push them directly.  Note that permissions are only possible on a repo basis, so they're actually giving you permission to push to their whole fork of Sage, but this may be acceptable if they trust you.

4. You can make your own competing PR.  I don't think we should commonly use this option in the sage community, but it may be appropriate if the original author has disappeared.  To do this, you can fetch and checkout their branch (merging develop if necessary), make your changes, and then make a new PR as normal.

Does that answer the question you had?

David

To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/eb497a5d-4bbe-4a68-81ff-6f28eaf1d110n%40googlegroups.com.

[Martin R]

cool, yes!  Thank you for your patience!

Martin

[Oscar Benjamin]

On Wed, 8 Feb 2023 at 13:10, David Roe <roed...@gmail.com> wrote:
>
> On Wed, Feb 8, 2023 at 12:23 PM 'Martin R' via sage-devel <sage-...@googlegroups.com> wrote:
>>
>> Why would I need write access to the sagemath repo?
>>
>> I would have thought that I'd "somehow" (no idea how) take over the branch and modify it. Or are pull requests directly tied to a specific user? In other words, what is the replacement for the following?
>
> Pull requests are from a specific user's fork of Sage. If you are not the author of a PR but want to suggest changes to it, you have several options.

...

> 3. If you're collaborating closely with the author, they can give you permission to push to the branch from which the PR originates. Then you could make your changes and push them directly. Note that permissions are only possible on a repo basis, so they're actually giving you permission to push to their whole fork of Sage, but this may be acceptable if they trust you.

Another option here is that when opening a PR there is a tickbox that
says something like "Allow edits by maintainers". This box is ticked
by default but the author can choose to untick it if they want. If the
box is ticked then I think it means that anyone who would have
permissions to merge the PR automatically has permissions to push to
the PR's branch in the originating author's fork which will update the
commits in the PR.

--
Oscar