Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor main arguments + other improvements #51

Merged
merged 6 commits into from
Oct 23, 2024

Conversation

sebalix
Copy link
Collaborator

@sebalix sebalix commented Sep 16, 2024

Context

This is a rework of #33 , and bring breaking changes, both for CLI tool and API if used as a Python package.

Description

Introduce 3 concepts:

  • source: source of the comparison
  • target: target of the comparison
  • destination: where to push the work done

All of them accept git [remote/]branch references, so additional info like repository and organization is retrieved from the remote URL.

This is also a preliminary step to support porting from one repo to another (very handy in case of modules moved to different repos).

CLI syntax changed to this:

$ oca-port origin/14.0 origin/16.0 my_module [--destination camptocamp/dev]

A local dev branch where commits have already been ported can be resumed this way:

$ oca-port origin/14.0 dev my_module --target-version=16.0 [--destination camptocamp/dev]

Here we are forced to use an additional parameter --target-version as the tool is unable to guess it from dev name.

A new --dry-run option has been added too:

$ oca-port origin/14.0 origin/16.0 my_module --dry-run

If no remote is defined (detected from destination or target), porting can still be done but the development branch won't be pushed, nor the PR open against the expected repository. However, a brief PR description will be proposed to the user at the end of the session so he'll be able to open the PR by its own if needed:
image

Remote is not mandatory for source, target and destination, so we can work fully with local branches:

$ oca-port 14.0 16.0 my_module --destination 16.0-port-things

Another improvement is that PortAddonPullRequest class doesn't create one branch for each ported PR, resulting in tons of branches created during a porting session. Instead, the destination branch is used (if provided) or one will be created automatically (name based on list of PRs to port, so re-running the command twice generates the same branch name in
most use cases). Of course, setting your own destination branch is recommended to preserve the full control of your porting session.

Also, the blacklisting of PRs/modules have been refactored to not create files to commit in the middle of a porting session. Instead, if a PR is set as blacklisted, this information will be stored in the user's session and commited at the end of the porting session. If for any reason the porting session is interrupted, such data are not lost when re-running the command again, and the user will be prompted if he still wants to blacklist such PR:
image

How to install and test this PR

$ pip install -U git+https://github.com/OCA/oca-port.git@refs/pull/51/head

Examples

Run a comparison for stock_restrict_lot in verbose mode first in --dry-run with --fetch to ensure we get the last changes locally:

$ oca-port origin/14.0 origin/16.0 stock_restrict_lot --verbose --dry-run --fetch

image
This had the effect to put some data in the user's cache, so further similar commands will be faster to run.

Re-run the same command without --dry-run nor --fetch, but with --destination (here a local branch) to not let the tool generates the destination branch automatically:

$ oca-port origin/14.0 origin/16.0 stock_restrict_lot --verbose --destination=16-port-from-14-stock_restrict_lot

image
Up to the user to resolve the conflicts manually (then git add [...] && git am --continue or git am --skip) before continuing the porting session. Answering no will perform a git am --abort before trying to port the next commit of the PR (if any), or propose to port the next PR.

In case the porting session is interrupted, we can resume it by changing restarting the comparison using our destination branch also as the target (with the help of --target-version so the tool knows which version we are talking about), so already ported commits won't be listed anymore:

$ oca-port origin/14.0 16-port-from-14-stock_restrict_lot --target-version=16.0 stock_restrict_lot --verbose --destination=16-port-from-14-stock_restrict_lot

Running the same command once everything is ported will produce this result:
image

TODO:

  • update the README

sebalix and others added 2 commits September 16, 2024 15:43
During a porting session, we could have the need to store some
information outside of the repository (to not have dangling files in
it).

With this new `Session` class, the app is now able to store data like PRs ported
or blacklisted during a session (identified by a name generated by dedicated tools)
such data could be used at the end of the session, like:

- blacklisted PRs/module will be commited all at once in the destination
  branch (instead of creating a commit in the middle of the session, which
  doesn't work well)
- list of ported PRs will be used to generate the description of the PR
  opened against the upstream org, or proposed to the user if the PR
  could not be opened (no remote/org provided by the destination)
- ability to remember the blacklisted items of an interrupted porting session,
  so they can be commited at the end of the session.
@sebalix sebalix force-pushed the refactor-cli-params branch 2 times, most recently from 6db0963 to e5a218e Compare September 16, 2024 15:31
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Comment on lines 121 to 126
"""Migrate ADDON from SOURCE to TARGET or list Pull Requests to port
if ADDON already exists on TARGET.

E.g.:

$ oca-port OCA/server-tools#14.0 OCA/server-tools#16.0 auditlog
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Migrate ADDON from SOURCE to TARGET or list Pull Requests to port
if ADDON already exists on TARGET.
E.g.:
$ oca-port OCA/server-tools#14.0 OCA/server-tools#16.0 auditlog
"""Migrate ``addon``` from ``source`` to ``target`` or list PRs.
If ``addon`` already exists....
E.g.:
$ oca-port OCA/server-tools#14.0 OCA/server-tools#16.0 auditlog

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I used these capitalized terms to align with the command as displayed by --help thanks to click:

$ oca-port --help
Usage: oca-port [OPTIONS] SOURCE TARGET ADDON

  Migrate ADDON from SOURCE to TARGET or list Pull Requests to port     if
  ADDON already exists on TARGET.

But we still need to fix the example in this docstring.

@sebalix sebalix added the enhancement New feature or request label Sep 18, 2024
@sebalix
Copy link
Collaborator Author

sebalix commented Sep 25, 2024

With last changes, when re-running the same command twice while having ported some commits in the first run, a WIP status will be displayed:

$ oca-port origin/14.0 origin/17.0 server_environment --verbose

image

And running the proposed command to recover the ongoing work could show there is nothing left to port, and therefore propose the user to open the PR (manually here as no destination was set):

$ oca-port origin/14.0 oca-port-server_environment-14.0-to-17.0-761c9d server_environment --target-version=17.0 --verbose

image

@sebalix sebalix marked this pull request as ready for review September 30, 2024 13:10
@sebalix sebalix force-pushed the refactor-cli-params branch 2 times, most recently from b95c374 to d057730 Compare October 14, 2024 08:41
simahawk and others added 4 commits October 23, 2024 17:34
Introduce 3 concepts:

* source: source of the comparison
* target: target of the comparison
* destination: where to push the work done

All of them accept git `[remote/]branch` references, so additional info
like repository and organization is retrieved from the remote URL.

This is also a preliminary step to support porting from one repo to
another (very handy in case of modules moved to different repos).

CLI syntax changed to this:

    $ oca-port origin/14.0 origin/16.0 my_module [--destination camptocamp/dev]

A local `dev` branch where commits have already been ported can be resumed this
way:

    $ oca-port origin/14.0 dev my_module --target-version=16.0 [--destination camptocamp/dev]

Here we are forced to use an additional parameter `--target-version` as
the tool is unable to guess it from `dev` name.

A new `--dry-run` option has been added too:

    $ oca-port origin/14.0 origin/16.0 my_module --dry-run

If no remote is defined (detected from destination or target), porting
can still be done but the development branch won't be pushed, nor the PR
open against the expected repository. However, a brief PR description
will be proposed to the user at the end of the session so he'll be able
to open the PR by its own if needed.

Another improvement is that `PortAddonPullRequest` class doesn't create
one branch for each ported PR, resulting in tons of branches created
during a porting session. Instead, the destination branch is used (if
provided) or one will be created automatically (name based on list of PRs to
port, so re-running the command twice generates the same branch name in
most use cases). Of course, setting your own destination branch is
recommended to preserve the full control of your porting session.

Also, the blacklisting of PRs/modules have been refactored to not create
files to commit in the middle of a porting session. Instead, if a PR
is set as blacklisted, this information will be stored in the user's session
and commited at the end of the porting session. If for any reason the
porting session is interrupted, such data are not lost when re-running
the command again.

Co-authored-by: Simone Orsi <simahawk@gmail.com>
Co-authored-by: Sébastien Alix <sebastien.alix@camptocamp.com>
When using oca-port as a Python package to collect migration data where
repositories are cloned on a filesystem with very low IO perf, the
process could take a lot of time to finish (every underlying git operations
to retrieve for instance updated file paths of a commit are very
costly). Such process could be killed before it actually finishes (and
write the data in cache).

By setting the OCA_PORT_AGRESSIVE_CACHE_WRITE environment variable, we
force oca-port to update the cache very often, generating more IO during
the first analysis of a module, but making the next process execution
much faster.

As this is an advanced option, it is exposed through an environment
variable to not clutter the usual parameters.
@sebalix sebalix merged commit dce5068 into OCA:main Oct 23, 2024
5 checks passed
@sebalix sebalix added this to the 0.16 milestone Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants