Move aports into own repository (pmaports)

added 1 commit

77fc261e - Move aports into own repository (pmaports)

Compare with previous version

By Oliver Smith on 2018-08-23T21:44:37

added 1 commit

8fee658e - bye bye aports

Compare with previous version

By Oliver Smith on 2018-08-23T21:48:09

changed the description

By Oliver Smith on 2018-08-23T21:52:44

Two things that are not clear from reading the description:

will there be a symlink made from the pmaports folder to the pmbootstrap folder so you can easily access the aports?
Will the history of the aports be preserved for the pmaports repo? There are nice git commands that can do that.

By Luca Weiss on 2018-08-24T06:59:10

will there be a symlink made from the pmaports folder to the pmbootstrap folder so you can easily access the aports?

Yes, see "New pmbootstrap init workflow":

if pmbootstrap/aports does not exist, try to symlink it to the cloned repo

On the terminal, it looks like this:

$ git clone https://gitlab.postmarketos.org/postmarketos/pmbootstrap.git
$ cd pmbootstrap
$ ./pmbootstrap.py init
[19:11:28] Location of the 'work' path. Multiple chroots (native, device arch, device rootfs) will be created in there.                                                                                           
[19:11:28] Work path [/home/user/.local/var/pmbootstrap]:
[19:11:29] pmbootstrap does everything in Alpine Linux chroots, so your host system does not get modified.                                                                                                        
In order to work with these chroots, pmbootstrap calls 'sudo' internally.
To see the commands it runs, you can run 'pmbootstrap log' in a second terminal.
[19:11:29] Setting up the native chroot and cloning the package build recipies (pmaports)...
[sudo] password for user:
[19:11:29] (native) install alpine-base
[19:11:30] (native) install git
[19:11:31] (native) git clone https://gitlab.com/postmarketOS/pmaports.git
[19:11:33] NOTE: pmaports path: /home/user/.local/var/pmbootstrap/cache_git/pmaports
[19:11:33] Target device (either an existing one, or a new one for porting).
...
$ file aports
aports: symbolic link to /home/user/.local/var/pmbootstrap/cache_git/pmaports

Will the history of the aports be preserved for the pmaports repo? There are nice git commands that can do that.

Yep, @GrantM11235 showed how to do it in #383 (comment 95390707).

The new repository exists already (that's how the test cases can run through in this MR):

https://gitlab.com/postmarketOS/pmaports

I hope we can merge this rather soon, but when new aports change in the pmbootstrap repository in the meantime, I will change them in that new repository as well.

By Oliver Smith on 2018-09-05T07:56:24

When the --aports arg is pointed to a non-existent directory it logs the first init message.

$ pmbootstrap --aports ~/src/not_a_dir/ init

[15:07:13] Setting up the native chroot and cloning the package build recipies (pmaports)...
[15:07:13] NOTE: pmaports path: /home/r/src/not_a_dir/
[15:07:13] ERROR: We have split the aports repository from the pmbootstrap repository (#383). Please run 'pmbootstrap init' again to clone it.

I've got the default pmaports initialized. I've tried to pass --aports a directory that doesn't exist. So that error message isn't accurately describing what has happened.

By ryang on 2018-08-24T19:17:58

Works great, but I can reproduce the error as mentioned by @ryang2678.

Other than that, this is definitely promising.

One question, right now I wasn't asked for what version of the pmaports repo to clone (like 3.8, edge, etc). This makes sense now as we don't yet have a non-edge release, but can I assume that once we do have such a release, the user will be asked for it?

By Bart Ribbers on 2018-08-26T11:01:08

One question, right now I wasn't asked for what version of the pmaports repo to clone (like 3.8, edge, etc). This makes sense now as we don't yet have a non-edge release, but can I assume that once we do have such a release, the user will be asked for it?

Yeah, it makes sense to implement that when we have multiple branches.

By Oliver Smith on 2018-08-26T21:43:17

added 1 commit

cd6273d0 - Check if --aports path exists, more docstrings

Compare with previous version

By Oliver Smith on 2018-08-28T20:07:25

added 1 commit

ab48ad0b - Revert "Check if --aports path exists, more docstrings"

Compare with previous version

By Oliver Smith on 2018-08-28T20:12:42

TODOs:

fix the bug @ryang2678 pointed out
add tests to pmaports repository again (https://gitlab.com/postmarketOS/pmaports/merge_requests/1)
add new commits from pmbootstrap to pmaports repository

By Oliver Smith on 2018-09-04T07:17:40

added 10 commits

ab48ad0b...6991c135 - 8 commits from branch master
358a763d - Move aports into own repository (pmaports)
4ae5eb32 - bye bye aports

Compare with previous version

By Oliver Smith on 2018-08-30T22:20:56

added 2 commits

f4d2310d - Move aports into own repository (pmaports)
eeef055a - bye bye aports

Compare with previous version

By Oliver Smith on 2018-08-30T22:24:23

I did a lot of work in adjusting the test cases and CI for pmaports (https://gitlab.com/postmarketOS/pmaports/merge_requests/1), and that's looking pretty good now (still WIP though). The bug @ryang2678 found is fixed here. For some reason, the CI in this MR is now failing with something related to the gitlab cache, clearing the cache did not help:

Creating cache default-1...
venv: found 1998 matching files                    
FATAL: open ../../../../../postmarketOS/pmbootstrap/default-1/archive_353320690: no such file or directory 
Failed to create cache
Uploading artifacts...
Incorrect Usage: flag provided but not defined: -artifact-format

NAME:
   gitlab-runner artifacts-uploader - create and upload build artifacts (internal)

USAGE:
   gitlab-runner artifacts-uploader [command options] [arguments...]

OPTIONS:
   --id value             The build ID to upload artifacts for (default: "93537347") [$CI_JOB_ID]
   --token value          Build token (default: "xxxxxxxxxxxxxxxxxxxx") [$CI_JOB_TOKEN]
   --url value            GitLab CI URL (default: "https://gitlab.com/") [$CI_SERVER_URL]
   --tls-ca-file value    File containing the certificates to verify the peer when using HTTPS (default: "/home/pmos/builds/postmarketOS/pmbootstrap.tmp/CI_SERVER_TLS_CA_FILE") [$CI_SERVER_TLS_CA_FILE]
   --tls-cert-file value  File containing certificate for TLS client auth with runner when using HTTPS [$CI_SERVER_TLS_CERT_FILE]
   --tls-key-file value   File containing private key for TLS client auth with runner when using HTTPS [$CI_SERVER_TLS_KEY_FILE]
   --path value           Add paths to archive (default: "[log.txt, log_testsuite.txt, dmesg.txt, pmbootstrap.cfg]")
FATAL: flag provided but not defined: -artifact-format 
   --untracked            Add git untracked files
   --verbose              Detailed information
   --retry value          How many times to retry upload (default: "2")
   --retry-time value     How long to wait between retries (default: "1s")
   --name value           The name of the archive (default: "artifacts")
   --expire-in value      When to expire artifacts (default: "1 week")
   
ERROR: Job failed: Process exited with: 1. Reason was:  ()

@craftyguy: you're good with gitlab CI, do you have an idea why this is failing?

@all: would someone like to do a code review?

By Oliver Smith on 2018-08-31T00:37:47

I'm guessing that has something to do with the failing Gitlab CI.

I can't do a code review for you sadly as I'm not that good with Python

By Bart Ribbers on 2018-08-31T02:34:27

Yes, that gitlab-runner failure was due to an incompatible gitlab-runner exe installed in the VM vs what was installed on the host (i.e. I updated the host, but forgot to update the vm image). It should be resolved now.

By clayton craft on 2018-08-31T02:36:29

Couple of things:

the CI tests removed here seemed important (else they wouldn't have been added, right?) but they don't seem to be re-implemented in the pmaports repo. There should be a way to still run them in some fashion, right?
I was going to help review python changes, but there's >500 files changes in this MR and I cannot find any python changes without spending hours (slight exageration, ?) collapsing things in gitlab's 'changes' section for this MR.

For #2 (closed), I can pull the branch and grep for the py files. But I wanted to point that out since it's a shortcoming of gitlab/web git review processes.

By clayton craft on 2018-08-31T02:43:52

Thanks for fixing it @craftyguy!

the CI tests removed here seemed important (else they wouldn't have been added, right?) but they don't seem to be re-implemented in the pmaports repo. There should be a way to still run them in some fashion, right?

Yes, they are important. I'm working on adding them again here: https://gitlab.com/postmarketOS/pmaports/merge_requests/1

I was going to help review python changes, but there's >500 files changes in this MR and I cannot find any python changes without spending hours (slight exageration, ?) collapsing things in gitlab's 'changes' section for this MR.

Right, going through all these files won't work - but I've put it in two commits, so one can click on "commits" and then look at the one with the code changes.

By Oliver Smith on 2018-08-31T20:11:40

I've tried a bunch of things and the split code seems to work great.

Only question I have is how is updating the pmaports repo checkout handled? I guess it makes sense to make pmbootstrap init warn if the pmaports checkout gets old.

By Martijn Braam on 2018-09-01T21:31:39

Awesome! Yeah, whenever you run pmbootstrap (not only on init), it will check if the pmaports version (as defined in pmaports.cfg) has at least the version required in pmb.config.min_version_pmaports:

pmb/config/pmaports.py:

def check_version_pmaports(args):
    # Compare versions
    real = args.pmaports["version"]
    min = pmb.config.pmaports_min_version
    if pmb.parse.version.compare(real, min) >= 0:
        return

    # Outated error
    logging.info("NOTE: your pmaports folder has version " + real + ", but" +
                 " version " + min + " is required.")
    raise RuntimeError("Please update your local pmaports repository. Usually"
                       " with: 'git -C \"" + args.aports + "\" pull'")

By Oliver Smith on 2018-09-01T22:34:31

This still doesn't update the pmaports repo daily/weekly which will probably cause problems where problems are already fixed (e.g. alpine soname bumps) but the users don't have that.

By Luca Weiss on 2018-09-01T22:36:54

Yeah that works but I was more thinking along the line of the code that runs apk update if the repositories are (a day?) old so you'll not working on an outdated aports checkout. I don't think the required pmaports will be bumped weekly.

Edit: @z3ntu beat me to it, that's what I meant.

By Martijn Braam on 2018-09-01T22:40:47

This still doesn't update the pmaports repo daily/weekly which will probably cause problems where problems are already fixed (e.g. alpine soname bumps) but the users don't have that.

if we have a soname bump, then the pkgrel will be increased in the pmaports version, and the new binary package will be built. pmbootstrap will use the package from the binary repo with the newer version in case the pmaports checkout is outdated.

So this should be fine, or am I missing something?

By Oliver Smith on 2018-09-01T22:53:35

In the soname case that would work (and probably in most other cases). But it would still be nice to have a warning when the pmaports checkout is old so you don't accidentally only do git pull on the pmbootstrap repo and keep working on an old pmaports repo.

By Martijn Braam on 2018-09-01T23:14:07

I think it would be weird for new users to see every day that they need to update the pmaports repository with this rather long command, and not having it automated.

But you have a point there, if someone wants to work on an existing aport, they should start their work on the latest version of the aport.

I would solve this with good documentation in the wiki TBH, because now that we do have the extra repository for the aports, contribution merge requests requires a bit more thinking anyway (one needs to enter the pmaports repository and do their changes there). We could bump the work folder version to explain this to existing users once (and display the link). What do you think about that?

By Oliver Smith on 2018-09-01T23:28:57

I think pmbootstrap should automatically check for updates to pmaports before certain commands (not sure which), and if there are any ask the user if they should be pulled

By Grant Miller on 2018-09-01T23:38:25

@GrantM11235: how would that work with using various branches?

EDIT: in other words, I think this would be problematic when the user is working on a custom branch, that would have merge conflicts with the upstream pmaports repository. That's why I came up with the minimum version concept - just let the local pmaports version lack behind the upstream one on purpose until the user really needs to update it. This way the user is also less likely to get aports checked out of which binary packages don't exist yet. I do understand that this changes the workflow quite a bit, but I don't see a way around it that would work for all the edge cases like handling different branches. But as always I'm happy for more feedback, maybe there is a more elegant way to do it!

By Oliver Smith on 2018-09-01T23:49:15

It could check for updates for the current branch only. If there are local changes, it could simply warn the user (Do you wish to continue? [y/n]:)

This functionality could probably be added in a later MR

By Grant Miller on 2018-09-01T23:59:31

In my view pmbootstrap becomes much more mature over time and the user will be able to use it without being required to update pmboostrap very often. From the user perspective they will from now on primarily be working with the pmaports repo.

In the end not much is different from the user perspective. The aports were managed by the user before when it was under pmbootstrap but now it becomes standalone.

By ryang on 2018-09-02T00:21:54

added 5 commits

eeef055a...5ad6302b - 2 commits from branch master
a9f6cd36 - Move aports into own repository (pmaports)
ac4d8980 - bye bye aports
a76583e7 - add aportgen test case again

Compare with previous version

By Oliver Smith on 2018-09-04T06:01:57

So I've added a few finishing touches in this repository (added the aportgen test case and rewrote it to work independent of changes in the pmaports repository). Also in the pmaports repository, where the test cases are integrated again, and the new commits from pmbootstrap are there as well.

I think this is good to go and will merge this tomorrow morning (so in ~22 hours), unless anyone has a good reason why we should not do it. After that I'll adjust documentation in the wiki to tell people to manually update the repository. I like the way @ryang2678 put it - instead of updating the pmbootstrap repository, users need to update their pmaports repository now.

This MR is also a big step towards pmbootstrap 1.0.

By Oliver Smith on 2018-09-04T07:24:02

approved this merge request

By Craig Tatlor on 2018-09-04T08:50:12

approved this merge request

By Luca Weiss on 2018-09-04T16:25:32

added 5 commits

a76583e7...520f34a2 - 2 commits from branch master
286c451c - Move aports into own repository (pmaports)
08419c54 - bye bye aports
1894a48c - add aportgen test case again

Compare with previous version

By Oliver Smith on 2018-09-05T04:35:25

Rebased one last time. When CI is through, I'll merge it. And without squash commits, because the commit that removes all aports has a huge diff that makes it hard to find the other changes. So better have them separated.

By Oliver Smith on 2018-09-05T04:36:49

merged

By Oliver Smith on 2018-09-05T05:57:39

mentioned in merge request !1681 (closed)

By Oliver Smith on 2018-09-05T07:10:09

mentioned in merge request !1666 (closed)

By Oliver Smith on 2018-09-05T07:10:57

mentioned in merge request !1643 (closed)

By Oliver Smith on 2018-09-05T07:11:30

mentioned in merge request !1623 (closed)

By Oliver Smith on 2018-09-05T07:12:12

mentioned in merge request !1294 (closed)

By Oliver Smith on 2018-09-05T07:12:51

mentioned in merge request !1610 (closed)

By Oliver Smith on 2018-09-05T07:13:14

mentioned in merge request pmaports!2 (merged)

By Oliver Smith on 2018-09-05T07:26:42

Move aports into own repository (pmaports)

Motivation

New workflows

New `pmbootstrap init` workflow:

New checks that run at the beginning of pmbootstrap:

Implementation details

Extended `pmb.helpers.git.clone()`:

Changes for initializing logging early:

Other changes:

How to review

How to test

Merged by Administrator 6 years ago (Sep 5, 2018 5:57am UTC) 6 years ago

Activity

Move aports into own repository (pmaports)

Motivation

New workflows

New pmbootstrap init workflow:

New checks that run at the beginning of pmbootstrap:

Implementation details

Extended pmb.helpers.git.clone():

Changes for initializing logging early:

Other changes:

How to review

How to test

Merge request reports

Merged by Administrator 6 years ago (Sep 5, 2018 5:57am UTC) 6 years ago

Activity

New `pmbootstrap init` workflow:

Extended `pmb.helpers.git.clone()`: