Gitpod: track remote trac branch

[tobiasdiez]

Currently, gitpod tracks the branch on github as the remote. Thus, one might accidentally changes to github instead of trac. With this ticket, the remote is changed to trac.

CC: @mkoeppe

Component: build

Author: Tobias Diez

Branch/Commit: bbe8724

Reviewer: Matthias Koeppe

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

[mkoeppe]

comment:2

I think git remote set-url --push origin git@trac.sagemath.org:sage.git would also be good to add

[tobiasdiez]

comment:3

Not sure if that's really a good idea. The git manual contains

Note that the push URL and the fetch URL, even though they can be set differently, must still refer to the same place. What you pushed to the push URL should be what you would see if you immediately fetched from the fetch URL. If you are trying to fetch from one place (e.g. your upstream) and push to another (e.g. your publishing repository), use two separate remotes.

Your idea was to prevent accidental commits to github, right? Would it be possible to restrict write access for the trac mirror repo to only allow the sync-hook to have write access?

[mkoeppe]

comment:4

It's already documented as standard procedure in src/doc/en/developer/manual_git.rst

[tobiasdiez]

comment:5

Where exactly? I can only find git remote set-url --push trac git@trac.sagemath.org:sage.git at https://doc.sagemath.org/html/en/developer/manual_git.html

[mkoeppe]

comment:6

Yes, that's the one.

[tobiasdiez]

comment:7

This is already included in the gitpod config:

      git remote add trac git@trac.sagemath.org:sage.git -t master -t develop -t $(git branch --show-current)
      git remote set-url --push trac git@trac.sagemath.org:sage.git
      git fetch trac
      git branch -u trac/$(git branch --show-current)

[mkoeppe]

comment:8

No, the first line is setting the wrong URL for fetching.

[tobiasdiez]

comment:9

I took this from the docs: https://doc.sagemath.org/html/en/developer/manual_git.html#the-trac-server

git remote add trac git@trac.sagemath.org:sage.git -t master

What is then the correct url?

[mkoeppe]

comment:10

Further up in the same section.

[tobiasdiez]

comment:11

git remote add trac https://github.com/sagemath/sagetrac-mirror.git -t master

Does not work well. If you then push something to trac, VS still notifies you that there are changes between your local and remote branch. The reason is that VS invokes git fetch directly after pushing, but then the sync hook has not yet been executed so it still fetches the old branch on github.

[mkoeppe]

comment:12

OK. Then I'd suggest that we make switching to the ssh remote (git@trac.sagemath.org:sage.git) part of the instructions for adding the SSH key.

[mkoeppe]

comment:13

We should probably also note this in the documentation (https://doc.sagemath.org/html/en/developer/manual_git.html) that the first configuration is not recommended for users who use VS Code.

[tobiasdiez]

comment:14

Replying to @mkoeppe:

OK. Then I'd suggest that we make switching to the ssh remote (git@trac.sagemath.org:sage.git) part of the instructions for adding the SSH key.

The point of this ticket was that this happens automatically for each ticket/gitpod session without manual intervention.

[tobiasdiez]

comment:15

Replying to @mkoeppe:

We should probably also note this in the documentation (https://doc.sagemath.org/html/en/developer/manual_git.html) that the first configuration is not recommended for users who use VS Code.

What's the reason for this config in the first place? It is also against the git recommendations that I've cited above.

[mkoeppe]

comment:16

Replying to @tobiasdiez:

What's the reason for this config in the first place? It is also against the git recommendations that I've cited above.

That's what read-only mirrors are for - they scale better than 1 read/write server.

I don't think that recommendation that you found in the doc is intended to rule out mirrors.

[mkoeppe]

comment:17

Replying to @tobiasdiez:

Replying to @mkoeppe:

OK. Then I'd suggest that we make switching to the ssh remote (git@trac.sagemath.org:sage.git) part of the instructions for adding the SSH key.

The point of this ticket was that this happens automatically for each ticket/gitpod session without manual intervention.

I think we shouldn't force users to do the SSH key thing if all they want to do is fetch from trac.

[mkoeppe]

comment:18

Maybe it should be made conditional on PRIVATE_SSH_KEY being set already?
By the way, the current script in .gitpod.yml creates an empty id_rsa file if PRIVATE_SSH_KEY is not provided; creating that should probably be avoided too

[sagetrac-git]

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

77b11ea

Only add trac if sshkey is set

[sagetrac-git]

Changed commit from ac4ec05 to 77b11ea

[tobiasdiez]

comment:20

Replying to @mkoeppe:

Maybe it should be made conditional on PRIVATE_SSH_KEY being set already?
By the way, the current script in .gitpod.yml creates an empty id_rsa file if PRIVATE_SSH_KEY is not provided; creating that should probably be avoided too

Good suggestion, thanks. I've implemented that now.

[mkoeppe]

comment:21

That should be "-n "${PRIVATE_SSH_KEY}"`

[tobiasdiez]

comment:22

Yes...

[mkoeppe]

comment:23

And everyone needs the trac remote.
When PRIVATE_SSH_KEY is set, just switch the fetch URL to the good one.

[sagetrac-git]

Changed commit from 77b11ea to a5d10a2

[sagetrac-git]

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

a5d10a2

Fix typo

[mkoeppe]

comment:40

I've added comment:13 to #33088

[mkoeppe]

comment:41

+ 4. Close this Gitpod workspace.

OK, that's a good solution too

[mkoeppe]

comment:42

I think all of these steps:

+        git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild

and

+        git remote set-url --push trac git@trac.sagemath.org:sage.git
+        git fetch trac
+        git branch -u trac/$(git branch --show-current)

should be unconditional

[tobiasdiez]

comment:43

Replying to @mkoeppe:

I think all of these steps:

+        git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild

During the prebuild there is never a ssh key set, so the trac remote is always either unset or set to the github mirror.

and

+        git remote set-url --push trac git@trac.sagemath.org:sage.git
+        git fetch trac
+        git branch -u trac/$(git branch --show-current)

should be unconditional

Fetching trac doesn't work without ssh keys (at least for me).

[mkoeppe]

comment:44

Replying to @tobiasdiez:

Fetching trac doesn't work without ssh keys (at least for me).

It doesn't if the remote is ssh, which is why we have changed that.

[tobiasdiez]

comment:45

I don't get what you want.

+        git remote set-url --push trac git@trac.sagemath.org:sage.git

is in conflict with

+        git remote set-url --push trac pushing-needs-ssh-key

Since the ssh setup is more involved for gitpod, its better in my opinion to explicitly tell the user that something is off so he might look this up in the docs.

Ps: can we please migrate to github/gitlab; their server are beefy enough to don't need a special mirror setup ;-)

[mkoeppe]

comment:46

I mean like this (untested).

diff --git a/.gitpod.yml b/.gitpod.yml
index 7c6800ab8f..8244bafae9 100644
--- a/.gitpod.yml
+++ b/.gitpod.yml
@@ -36,7 +36,9 @@ image:
 tasks:
   - name: Setup
     before: |
+      git remote set-url --push origin pushing-only-via-trac
       # Setup trac as remote
+      git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild
       ## In order to push to trac, generate a new key with `ssh-keygen -f tempkey` and save the private key to gitpod `gp env PRIVATE_SSH_KEY="$(<tempkey)"` (or by following https://www.gitpod.io/docs/environment-variables#using-the-account-settings)
       ## then follow https://doc.sagemath.org/html/en/developer/trac.html#linking-your-public-key-to-your-trac-account to register the public key with trac.
       ## Afterwards, create a new gitpod workspace.
@@ -50,18 +52,15 @@ tasks:
         ssh-keyscan -H trac.sagemath.org >> ~/.ssh/known_hosts
 
         # Setup trac repo
-        git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild
         git remote add trac git@trac.sagemath.org:sage.git -t master -t develop -t $(git branch --show-current)
         git remote set-url --push trac git@trac.sagemath.org:sage.git
-        git fetch trac
-        git branch -u trac/$(git branch --show-current)
-        git remote set-url --push origin pushing-only-via-trac
       else
         # Fallback to sagemath mirror
-        git remote add trac https://github.com/sagemath/sagetrac-mirror.git -t master -t develop
+        git remote add trac https://github.com/sagemath/sagetrac-mirror.git -t master -t develop -t $(git branch --show-current)
         git remote set-url --push trac pushing-needs-ssh-key
-        git remote set-url --push origin pushing-only-via-trac
       fi
+      git fetch trac
+      git branch -u trac/$(git branch --show-current)
       
       ## No need for pyenv
       pyenv shell --unset 2> /dev/null

[tobiasdiez]

comment:47

That would work, I guess. But I fail to see the point of setting the upstream to "trac" even if you don't have the ssh keys configured. Why change the gitpod default?

I would like to leave the whole trac remote business as minimal as possible.

[mkoeppe]

comment:48

Replying to @tobiasdiez:

That would work, I guess. But I fail to see the point of setting the upstream to "trac" even if you don't have the ssh keys configured.

Because it means that there are fewer differences between "without ssh key" and "with ssh key", which reduces the complexity in explaining it to developers?

[tobiasdiez]

comment:49

If you cannot push anything, then you don't need to worry about trac at all. Everything you need is in "origin". The remove trac is only added because of the limitations of git-trac you mentioned above.

[mkoeppe]

comment:50

OK, I think this makes sense

[mkoeppe]

comment:51

Now if we also want to support a development model in which users use their own GitHub forks as origin, then we should probably do the line git remote set-url --push origin pushing-only-via-trac in the "else" branch only when origin is in the sagemath organization.

[mkoeppe]

comment:52

And I'd still prefer to move the line

+        git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild

to the very beginning so that it runs in both cases - it's easier to understand that it is correct

[tobiasdiez]

comment:53

Replying to @mkoeppe:

Now if we also want to support a development model in which users use their own GitHub forks as origin, then we should probably do the line git remote set-url --push origin pushing-only-via-trac in the "else" branch only when origin is in the sagemath organization.

Do you know how/with which credentials the sync hook pushes to the github mirror? Maybe its easy to setup a branch protection rule that would prevent any pushes to the mirror (except for the syncs).

[mkoeppe]

comment:54

No idea, sorry

[sagetrac-git]

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

80e8e2e

Remove special treatment of origin for pushes

[sagetrac-git]

Changed commit from e9035ae to 80e8e2e

[sagetrac-git]

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

0250acc	Test commit
cc45a3b	Revert

[sagetrac-git]

Changed commit from 80e8e2e to cc45a3b

[tobiasdiez]

comment:57

According to

curl -H "Accept: application/vnd.github.v3+json" https://api.github.com/networks/sagemath/sagetrac-mirror/events?per_page=100 | ConvertFrom-Json | where { $_.type -eq "PushEvent" } | Select actor

it is always sageb0t. I've now added a branch protection rule, see https://groups.google.com/g/sage-devel/c/Z1rx_uP7LtY. Thus the special handling of origin is no longer needed.

[tobiasdiez]

comment:58

Replying to @mkoeppe:

And I'd still prefer to move the line

+        git remote remove trac 2> /dev/null # might still exists from a previous run/prebuild

to the very beginning so that it runs in both cases - it's easier to understand that it is correct

I agree.

[sagetrac-git]

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

bbe8724

Move reset of trac remote

[sagetrac-git]

Changed commit from cc45a3b to bbe8724

[mkoeppe]

Reviewer: Matthias Koeppe

[tobiasdiez]

comment:61

Thanks for the review and the many helpful suggestions!

[vbraun]

Changed branch from public/build/gitpod_trac_tracking to bbe8724