You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/01/21 23:38:10 UTC

[GitHub] [mynewt-newt] ccollins476ad opened a new pull request #365: [RFC] Simplify repo dependencies

ccollins476ad opened a new pull request #365: [RFC] Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365
 
 
   This is a massive PR.  I separated the work into a few different commits, but reviewing this is still going to suck no matter how you slice it.
   
   This PR concerns Mynewt's repo dependency system.  That is to say, what happens when a user runs `newt upgrade`.  This functionality is burdened by two unfortunate features:
   
   1. The ability to depend on a *range* of repo versions, and
   2. Mandatory `version.yml` files.
   
   I propose that we greatly simplify repo management by removing both of these features.  Below I will describe each of these features and why I think they should be removed.  This PR implements the proposal.
   
   #### BAD FEATURE 1: RANGED DEPENDENCIES
   
   I'll start by giving some examples [1].
   
   ```
       # Example 1
       repository.apache-mynewt-core:
           type: git
           vers: '>=1.6.0'
           url: git@github.com:apache/mynewt-core.git
   
       # Example 2 [2]
       repository.apache-mynewt-nimble:
           type: git
           vers: '>=1.1.0 <=1.2.0'
           url: git@github.com:apache/mynewt-nimble.git
   ```
   
   Each dependency uses comparison operators to express a range of acceptable versions.
   
   Now let me explain why I think ranged dependencies is a bad feature.  To begin with, a ranged dependency like `>=1.0.0 <2.0.0` only works if the repo follows the SemVer model (or something like it).  SemVer requires that the major version number be bumped whenever a compability-breaking change is made to the interface.  If a repo doesn't faithfully follow SemVer, then there's no guarantee that 1.0.1 has the same interface as 1.0.0 (for example).  Ranged dependencies clearly aren't appropriate for such a repo.  My feeling is that most library maintainers don't follow SemVer even if they intend to.  Part of the problem is that people don't always want to bump the major version when they ought to because to do so has certain non-technical implications (e.g., "2.0" of something is often accompanied by a press release or other fanfare).
   
   But more importantly, Mynewt is used for embedded development, and I think most embedded developers want to know exactly which libraries they're putting in their products.  In this sense, allowing a range of repo versions does more harm than good because it takes control away from the developer.  Ranged dependencies allow repos to unexpectedly change versions when you run `newt upgrade`.
   
   Finally, use of version ranges makes the job of maintaining a Mynewt repo more difficult.  A new repo version must be tested against a range of dependency versions rather than just one.
   
   So the first part of my proposal is to remove support for version ranges in repo dependencies.  A repo dependency must specify a single version using one of three forms:
   
   * Fixed version (`1.0.0`)
   * Floating version (`1.6-latest`)
   * Commit (`7088a7c029b5cc48efa079f5b6bb25e4a5414d24-commit`) OR (`tagname-commit`) OR (`branchname-commit`)
   
   #### BAD FEATURE 2: `VERSION.YML` FILES
   
   To determine the version of a particular commit of a repo, newt checks out that commit and reads the contents of a file called `version.yml`.  This file contains a single fixed version string.  When a maintainer releases a new version of a repo, they have to update `version.yml` before creating the tag.
   
   The entire reason for this `version.yml` requirement is to support `*-commit` dependencies.  I'll give an example to illustrate what I mean:
   
   * mynewt-core 1.6.0 DEPENDS ON mynewt-nimble 1.1.0
   * mynewt-core 1.7.0 DEPENDS ON mynewt-numble 1.2.0
   
   Now say a user doesn't want to use a versioned release of mynewt-core.  Instead, he wants his project to use a commit of mynewt-core that is somewhere between 1.6.0 and 1.7.0 in the git history.  What version of mynewt-nimble should this particular commit depend on?  What if the user depends on a mynewt-core commit from a completely different branch?  Newt answers these questions by reading mynewt-core's `version.yml` file from the requested commit.  If the file contains "1.6.0", then core depends on nimble 1.1.0.  If it says "1.7.0", core depends on nimble 1.2.0, and so on.
   
   So that's the rationale and the history of the `version.yml` file.  In retrospect, I think this `version.yml` idea was a mistake.  Arbitrary commits are *not* versioned releases and they should not be treated like them.  When the user specifies a `*-commit` dependency he is putting newt into a sort of "manual override" mode.  Newt should not try to figure out inter-repo dependencies in this mode.
   
   Another reason why `version.yml` files are bad is that they make repo maintenance more difficult.  To keep a `version.yml` file in a sane state, a repo maintainer has to use a particular branching strategy when preparing a new release (step 0 and step 5b here: https://cwiki.apache.org/confluence/display/MYNEWT/Release+Process).  It's easy to get this wrong, and it is just not a very user friendly experience for a repo maintainer.  The multi-platform project MCUboot doesn't use branches in its release process and the requirement to maintain a `version.yml` file has been a burden on its developers.
   
   So the second part of my proposal is to remove the `version.yml` requirement entirely.  Newt should not try to figure out what an arbitrary commit's "real version number" is.
   
   #### PROPOSAL SUMMARY
   
   Let me try to summarize what I've written and explore some of the implications.
   
   * If `project.yml` and `repository.yml` files use versioned dependencies only then nothing changes.
   
   * If someone depends on a particular commit of repo X, then X ceases to introduce inter-repo dependencies.  These dependencies must be manually added to `project.yml`.
   
   * If someone depends on a particular commit of repo X, this dependency is incompatible with all other dependencies on X.  The only exception is a dependency on the exact same commit.  Newt aborts the `newt upgrade` operation when it detects such a conflict.
   
   * (Old behavior but worth mentioning): If `project.yml` depends on a version or commit of repo X, this dependency *overrides* any inter-repo dependencies on X.
   
   ##### The typical user workflow would look like this:
   
   1. Start with a `project.yml` file that depends on repo versions.
   
   2. If you want to depend on a specific commit of a repo, change your `project.yml` file to use a `*-commit` dependency.  If the dependee repo has any dependencies of its own, you'll need to add these dependencies to `project.yml` as well (assuming they aren't already there).
   
   3. Given this dependency tree:
   ```
       project.yml --> repo-X/1.7.0 --> repo-Y/1.2.0
   ```
   If you don't want to use version 1.2.0 of repo Y (either you want a different commit or you want to use a fork of the repo instead), then you should change your `project.yml` file to use a `*-commit` dependency on repo Y.
   
   ##### The typical repo release process would look like this:
   
   1. Create a git tag where you want the release to sit.
   2. In the master branch, modify the `repository.yml` file:
       a. Add a new entry that maps a fixed version to your new tag.
       b. Update any floating versions to point to the new tag.
   
   [1]: All of these examples are dependencies that you would find in a `project.yml` file.  Everything in this email applies equally to inter-repo dependencies (dependencies in a `repository.yml` file).
   
   [2]: Furthermore, newt won't even accept multiple conditions as in example 2r.  This is supposed to work, and it used to work, but it seems it was broken a while back.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] utzig commented on issue #365: [RFC] Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
utzig commented on issue #365: [RFC] Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577834388
 
 
   @ccollins476ad Looks good to me; not sure someone else is taking a look but IMO this is OK to merge (once squashed!), nice work!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] ccollins476ad edited a comment on issue #365: Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
ccollins476ad edited a comment on issue #365: Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577937315
 
 
   Thanks @utzig, that's a nice trick.  I used it to verify the travis tests.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] ccollins476ad commented on issue #365: Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
ccollins476ad commented on issue #365: Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577937315
 
 
   Thanks @utzig, that's a nice trick.  I was able to verify the travis tests.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] ccollins476ad commented on issue #365: [RFC] Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
ccollins476ad commented on issue #365: [RFC] Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577819792
 
 
   Wow, thanks for looking at this @utzig!  Actually, I meant to remove all the `version.yml` code.  It looks like I missed some of it.  I pushed another commit that removes this code.
   
   I know... the commits are really hard to follow.  I think the only way I could make the changes easier to follow would be to reimplement the feature (but maybe I'm wrong about that).  I started out with about 20 commits and tried to simplify the history as much as I could (which I admit, wasn't much).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] ccollins476ad merged pull request #365: Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
ccollins476ad merged pull request #365: Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] utzig commented on issue #365: Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
utzig commented on issue #365: Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577879096
 
 
   @ccollins476ad Btw, you could as well add a commit that changes https://github.com/apache/mynewt-newt/blob/892b2bcab938861057e8ba7efc15926284a0fa13/.travis.yml#L56 to:
   
   ```
   git clone -b newt-upgrade https://github.com/ccollins476ad/mynewt-travis-ci $HOME/ci
   ```
   Then the CI would pass here, just need to remember to drop the commit when merging!
   
   Maybe we should consider moving those CI targets to this repo under `ci/targets` or something similar in the future...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-newt] ccollins476ad edited a comment on issue #365: [RFC] Simplify repo dependencies

Posted by GitBox <gi...@apache.org>.
ccollins476ad edited a comment on issue #365: [RFC] Simplify repo dependencies
URL: https://github.com/apache/mynewt-newt/pull/365#issuecomment-577819792
 
 
   Wow, thanks for looking at this @utzig!  Actually, I meant to remove all the `version.yml` code.  It looks like I missed some of it.  I pushed another commit that removes this code.
   
   I know... the commits are really hard to follow.  I think the only way I could make the changes easier to follow would be to reimplement the feature (but maybe I'm wrong about that).  I started out with about 20 commits and tried to simplify the history as much as I could (which I admit, wasn't much).
   
   EDIT: Yes, the plan is to squash everything into two commits (the first one with the detailed description (23b140), and then everything else into a second commit).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services