AEP for developer/packager workflow supporting changes

[mosteo]

This is a proposal for changes without any code yet. Related to points raised in #240, and intended to clarify ideas before starting development on publishing automation.

Since I expected this to be somewhat long and not very manageable for changes, the description is given in the doc/AEPs/aep-0002.md file.

In summary, the proposal is to clarify how the manifest file is to be used/edited/versioned and to fully embrace the manifest to be edited by hand, mainly by packagers.

Although the description talks about a new *.diff delta file, I think this is doable by having a "tail" section in the current manifest that be autogenerated (e.g., the dev-depends that you @Fabien-Chouteau suggested in #240), while the rest is only hand-edited (so, like project files and "with"s). Thus, maintainers could move information from the autogenerated part into the hand-maintained part as part of the publishing process, and we don't need to complicate things outwards by having an extra file.

If we decide the proposal can move forward, I would update it to reflect this unified toml file.

[Fabien-Chouteau]

I will toss around my own idea of the workflow I would like to have as a user.
So we can hopefully brainstorm on each others view.

I would prefer to not have an extra file for dependencies, but I am not closed
to the idea. In my opinion, we can make alr with edit alire.toml in a
way that preserve the file.

In my opinion we only need two files:

• alire.toml (or my_project.alire): contains everything about the current
  state of the project. Basically the same as the manifest we have right now,
  except that there is no notion of multiple releases. There is only one
  release, the "current" one. We can have a field for the users to specify a
  version number for "current".

  Dependencies are added/removed to/from this file, either automatically or
  manually (see above).

  This files doesn't have any mandatory field because it can be a
  work-in-progress, and this is not the file that will be contributed to the
  index.

  It is version controlled and can be in the root dir of the project.

• alire.lock: same purpose as right now. It is version controlled so it
  can be in the root dir of the project as well.

When a packager wants to publish a version of the crate to the index, the
command alr publish 1.0.0 will gather the info from alire.toml and
alire.lock to produce a "release manifest": for instance
alire/releases/my_project-1.0.0.toml. This is the file that is contributed to
the index, and it is clear that it is machine-generated and not supposed to be
edited by hand.

alr publish can also produce an archive and its sha512, or ask for an origin URL.
If it is an archive, the sha512 is automatically computed and added to the "release manifest".

Once the packager has contributed the "release manifest" to an index, it can forget about
it. Working a new next release just means editing alire.toml, the code and
then doing another alr publish.

This means a big refactoring on how alire reads the index, because there is
now multiple toml files for a single crate.

[Fabien-Chouteau]

Cargo doesn't have any command to add a dependency, right?
cargo.toml is always edited manually.

For OPAM and Conan, looks like there is an install command but it doesn't change the "manifest".

So it would mean that the features provided by alr with are beyond what is commonly available.
The more common scheme is to add new dependencies by editing a file.

[mosteo]

Cargo doesn't have any command to add a dependency, right? cargo.toml is always edited manually.

Right.

For OPAM and Conan, looks like there is an install command but it doesn't change the "manifest".

Since we don't install, I think that goes beyond what we have right now anyway.

So it would mean that the features provided by alr with are beyond what is commonly available. The more common scheme is to add new dependencies by editing a file.

Right again. In Cargo at least, you edit and the next cargo command detects the edition and updates dependencies as needed. So it's always a two-step process.

[Fabien-Chouteau]

This is a extra difficulty that we have maybe compared to other package manager, the fact that there is another tool and file format for build (gprbuild and gpr files).

This question of adding with statement in a gpr file doesn't exists for other package managers, AFAIK.

[mosteo]

This is true, although with the "section markers" you recently implemented I think we are now in an equivalent situation (the automatic section can be generated at any moment is convenient for us).

[mosteo]

I will toss around my own idea of the workflow I would like to have as a user.
So we can hopefully brainstorm on each others view.

Great! Actually I think we have very close positions, just details to iron out.

I would prefer to not have an extra file for dependencies, but I am not closed to the idea. In my opinion, we can make alr with edit alire.toml in a way that preserve the file.

Agreed. Having a section with markers should work, if we end needing a part of the file that is autogenerated.

In my opinion we only need two files:

• alire.toml (or my_project.alire): contains everything about the current
  state of the project. Basically the same as the manifest we have right now,
  except that there is no notion of multiple releases.

The current local manifest is already like that; no more than one release should be found there. I forgot to mention that in the proposal, but the idea is as you say: locally, the manifest represents the current state. Only the index stores all "frozen" (for each version) copies of the manifest.

There is only one release, the "current" one. We can have a field for the users to specify a version number for "current".

The version is currently taken from the table name that stores the release, but this would be even simpler if we move to a single file per release in the index.

Dependencies are added/removed to/from this file, either automatically or
manually (see above).
This files doesn't have any mandatory field because it can be a
work-in-progress, and this is not the file that will be contributed to the
index.
It is version controlled and can be in the root dir of the project.

My only note here is: if the information it contains can be moved directly into the index copy, it may save work for packagers and simplify our automation.

• alire.lock: same purpose as right now. It is version controlled so it
  can be in the root dir of the project as well.

Actually, the lockfile should only be version-controlled for crates containing end-products (Cargo nomenclature, so binaries or static libraries), so their dependencies can be fixed for reproducibility. For non-end-product crates, the lockfile should not be distributed as it precludes conflict resolution. This is the same distinction made in Cargo (explained here and here).

Another aspect to consider here is that the lockfile is platform-dependent, as dynamic expressions are resolved to obtain platform dependencies. For non-end crates, or platform-independent crates, this is a non-issue (for the former, there's no lockfile to distribute; for the latter, it is the same one for all platforms).

For end-product crates, I suspect one would have to distribute several lockfiles for the different scenarios. And this involves solving for all foreseeable combinations in advance. I see this as a further separate issue though.

(...)

No disagreement on the skipped part about generating the index file.

Once the packager has contributed the "release manifest" to an index, it can forget about
it. Working a new next release just means editing alire.toml, the code and
then doing another alr publish.

Right, agreed.

This means a big refactoring on how alire reads the index, because there is now multiple toml files for a single crate.

I think this is a matter of implementation, but related to something I wanted to brought up sooner than later, so since you mention it, it's a good moment. I agree on the idea that we need either:

• To move to separate files, as you suggest (this is what at least Cargo and OPAM are doing), or
• Remove any "reusable" information (project files, dependencies, anything that affects the releases) from the general section, so changes to it cannot retrospectively affect past releases.

I think both are pretty equivalent, although separate files is probably simpler to manage.

For now I would leave the lockfile out of the picture (although keeping an eye on future trouble).

I have to scram rn so I cannot write the conclusions. More to come.

[Fabien-Chouteau]

The current local manifest is already like that; no more than one release should be found there.

One release should be found, but the reality is actually confusing for users.
For instance because the syntax from alr init is like this, with a version number:

["0.0.0"]
origin = "file://.."
["0.0.0".depends-on]
libhello = "*"

And because release manifests have multiple versions: https://github.com/Fabien-Chouteau/ada-voxel-space-demo/blob/master/alire/ada_voxel_space_demo.toml

Just to be sure we are on the same page. What I suggest is that we remove the release to top-level TOML object.

I forgot to mention that in the proposal, but the idea is as you say: locally, the manifest represents the current state. Only the index stores all "frozen" (for each version) copies of the manifest.

Nice, I do think that this will be better in the end.
And actually it is my understanding of how Cargo works, except that users never see the "release manifest" format because it is automatically uploaded to a server and never seen or edited.

My only note here is: if the information it contains can be moved directly into the index copy, it may save work for packagers and simplify our automation.

Not sure what you have in mind here.

• alire.lock: same purpose as right now. It is version controlled so it
  can be in the root dir of the project as well.

Actually, the lockfile should only be version-controlled for crates containing end-products (Cargo nomenclature, so binaries or static libraries), so their dependencies can be fixed for reproducibility. [...]

Makes sense 👍

I think this is a matter of implementation, but related to something I wanted to brought up sooner than later, so since you mention it, it's a good moment. I agree on the idea that we need either:

* To move to separate files, as you suggest (this is what at least Cargo and OPAM are doing), or
* Remove any "reusable" information (project files, dependencies, anything that affects the releases) from the general section, so changes to it cannot retrospectively affect past releases.

I think both are pretty equivalent, although separate files is probably simpler to manage.

I seems like the second solution is more complex with different ways to read manifests.

For now I would leave the lockfile out of the picture (although keeping an eye on future trouble).

Sounds good.

[mosteo]

One release should be found, but the reality is actually confusing for users.

That's fair.

And because release manifests have multiple versions: https://github.com/Fabien-Chouteau/ada-voxel-space-demo/blob/master/alire/ada_voxel_space_demo.toml

Just to be sure we are on the same page. What I suggest is that we remove the release to top-level TOML object.

Understood. Indeed that indirection is bothersome and not necessary for the local manifest.

And actually it is my understanding of how Cargo works, except that users never see the "release manifest" format because it is automatically uploaded to a server and never seen or edited.

That's my understanding as well. Though, I'm not sure where the whole information is stored. There is some partial information that can be seen in the crates.io repo: see for example the rand crate. The description is missing there, for example.

My only note here is: if the information it contains can be moved directly into the index copy, it may save work for packagers and simplify our automation.

Not sure what you have in mind here.

I mean that there could be no need to regenerate the index copy from memory information if the local manifest can be included directly (even if we gather some pieces of information from different places). Not really a crucial point.

I seems like the second solution is more complex with different ways to read manifests.

I'm inclined to think the same.

Now, I don't think we have fundamental disagreements about the workflow but I feel we are starting to bite too much for a single issue, so I'm going to try to take a step back.

The core ideas behind this proposal to me are:

1. To sanction that the manifest be put into VCS and distributed with sources.
   • I think we agree on this one.
2. To stop mangling the manifest, so packagers can fill in all necessary metadata conveniently at any time by hand.
   • I think we agree on this one.
3. To keep alr with functionality despite the previous point.
   • I think you put this one up for debate.

Other big format changes in the index or manifest can be addressed separately, so I would discuss those in a follow-up issue. With the previous points in mind:

1. Requires no changes but to documentation.
2. Requires straightforward changes (but for the dependencies touched by alr with).
3. The question is: do we keep the option for dependency modification through alr with?
   1. No, all edition is manual. End of scope for this proposal.
   2. Yes, we keep it. We can then discuss how to best support this without mangling the manifest.

On the one hand, I see how removing alr with [for edition] simplifies things. OTOH, I prefer a single action, where I see the changes before applying them, than a two-step action where I first edit and then if I made a mistake I won't realize until the delayed update.

As you said, we are in a particular situation because to use a new library we need to update three places: the GPR, the manifest, and the Ada code. As a whole, with alr with we make this a two-step process: alr with + Ada edit. Without it, it is a three-step process: edit manifest + alr update + Ada edit.

(Side note: I've been rethinking about putting the lockfile into VCS as you said. I think it may work in our particular case, because it is not used for dependencies, so the Cargo rationale does not translate directly to Alire. I would like simplifying this point too, it's simpler if we can simply advice having both always in VCS.)

[Fabien-Chouteau]

3. To keep `alr with` functionality despite the previous point.
   * I think you put this one up for debate.

I think that alr with should not block the refactoring discussed here.
That means, let alr with as is for the moment, and when the refactoring is done we will see if we can do better or simply remove alr with.

3. The question is: do we keep the option for dependency modification through `alr with`?
   1. No, all edition is manual. End of scope for this proposal.
   2. Yes, we keep it. We can then discuss how to best support this without mangling the manifest.

I would go for 2. We are still in beta, we have time to rethink about alr with.

On the one hand, I see how removing alr with [for edition] simplifies things. OTOH, I prefer a single action, where I see the changes before applying them, than a two-step action where I first edit and then if I made a mistake I won't realize until the delayed update.

I do like alr with myself for the same reason.

[mosteo]

Thanks for the feedback, Fabien. I have a clear view now on how to proceed. I think we can attain the three objectives with a relatively simple patch that removes regeneration of the manifest (unless there's no manifest, to serve as a transitional measure) and that uses e.g. a top-level [reserved] table at the end of the manifest to store the information that is a machine insert (alr with dependencies at the moment. We could consider the lockfile separately).

As for the big index refactoring (single file per release) and related local manifest changes, I think that can be ironed out also separately. I don't think it needs to interfere with this one. I'll open an issue for these topics to clarify.

[mosteo]

As an alternative to what I just pushed (a private part in the manifest), I can see having two files like now, manifest + lockfile, but redefine them as the user manifest and the private alr storage. Kind of an alire.spec/alire.body dichotomy, (or alire.user/alire.priv, or keep .toml/.lock as-is).

Steps to implement this proposal piecemeal:

• Repurpose the lockfile as the alr-generated private storage, also to be under VC (per The case for always putting crate.lock under version control #468).
• Load/Store alr-with dependencies in the private storage.
• Stop regenerating the manifest.
  • Transitionally, regenerate it on get if not present in sources.

Follow-up changes raised in the discussion

• Consider storing the machine-generated storage within the manifest.
• Consider moving the manifest out of the alire folder.
• Remove/merge the [general] and [version] tables.
  • Do not require/forbid origin in the local manifest (other fields?).
• Convert the index to single-file-per-release format.

[Fabien-Chouteau]

I think we should move it to the root dir: ${crate}/alire.toml.

[mosteo]

And make it always alire in name, I guess. I think it's less confusing. And the lock file in tow, too (on both naming and placement).

[Fabien-Chouteau]

That means you want to use the manifest in the source package of a release for instance, rather than use the one already in the index?

[mosteo]

If I understand the context you quote correctly, yes, but this is only a concern for a root crate retrieved with alr get.

For the root crate we rely exclusively on the on-disk manifest. So, for alr get crates we currently regenerate the local manifest from the index information and from then on is as if the user had provided the manifest. So the idea is to actually use the one they provide at the time of packaging (that should match the one published).

Dependencies OTOH use the index information from which we find and retrieve them; any manifest they include is ignored (and we do not recreate one for them). (Except for dependencies linked from a directory; the local manifest is used because there's no corresponding index information.)

[Fabien-Chouteau]

use the one they provide at the time of packaging (that should match the one published)

That is my concern, we don't know if the manifest packaged in a release is going to be the same as the one in the index.

[mosteo]

This is also on my radar. Thoughts:

• Compare local and remote manifest and warn/err if they differ.
  • We should check this too server-side for new submissions, so the logic could be reused.
• Use the index manifest by default after alr get, until a manual edition to the local manifest is done.
  • Overkill if we have checks, simpler if we don't have.

As far as I see, this is only a concern for root crates retrieved via alr get. Otherwise the local manifest is the only source of metadata.

[Fabien-Chouteau]

If we change depends-on to be a TOML array of table: [[depends-on]], alr with can just append to the manifest:

$ alr with gnatcoll

[[depends-on]]
gnatcoll = "*"

$ alr with apdf

[[depends-on]]
gnatcoll = "*"
[[depends-on]]
apdf = "*"

This makes alr with very simple to implement and doesn't require a special "do not touch" section in the manifest.
It also means that we cannot have alr with --del apdf, but I don't this it is a huge problem.

[mosteo]

Interesting idea. I need to think how that would impact current dynamic expressions.

I don't like that one can end with dependencies scattered over the manifest, and the "redundant" depends-on. OTOH I don't like having dependencies split between the manifest and the lockfile or private part either. This looks cleaner.

[mosteo]

I have updated the document to reflect the implemented changes. Since this is an internal document, I'm going ahead and merge but if you see any discrepancy that merits fixing please let me know.