Skip to content

Ubuntu/devel 22.1 hotfix2#1350

Closed
blackboxsw wants to merge 2 commits into
canonical:ubuntu/devel-22.1-hotfix2from
blackboxsw:ubuntu/devel-22.1-hotfix2
Closed

Ubuntu/devel 22.1 hotfix2#1350
blackboxsw wants to merge 2 commits into
canonical:ubuntu/devel-22.1-hotfix2from
blackboxsw:ubuntu/devel-22.1-hotfix2

Conversation

@blackboxsw
Copy link
Copy Markdown
Collaborator

Given cherry-pick won't be able to merge cleanly into our hotfix branch, the following procedure is require to "backport" this fix in the quilt patch we create:

procedure

$ git fetch upstream
$ git checkout upstream/ubuntu/devel-22.1-hotfix1 -B ubuntu/devel-22.1-hotfix2
# Note not `git cherry-pick`
$ cherry-pick eee603294f120cf98696351433e7e6dbc9a3dbc2
# will fail to apply one changeset  to coud-init-generator.tmpl leaving a systemd/cloud-init-generator.tmpl.rej file around
# Check patch failure in systemd/cloud-init-generator.tmpl.rej


$ vi systemd/cloud-init-generator.tmpl
# Manually apply the missing +RUN_DISABLED_FILE="$LOG_D/$DISABLE" below RUN_ENABLED_FILE="$LOG_D/$ENABLE"

# update failed quilt patch to accept your changes
$ quilt refresh
$ quilt pop -a
$ git add debian/patches/series
$ git add debian/patches/cpick-eee60329-Fix-cloud-init-status-wait-when-no-datasource-found
$ git commit -am 'cherry-pick eee60329: minor backport of quilt patch'
$ dch -i
# add the following entry for a new version cloud-init (22.1-14-g2e17a0d6-0ubuntu1~22.04.4) jammy

+  * cherry-pick eee60329: Fix cloud-init status --wait when no datasource
+    found (#1349)
$ git commit -am 'update changelog'

Proposed Commit Message

  • will be merged from cmdline (do not squash merge)

Additional Context

Test Steps

quilt push -a; tox -e py3; tox -e flake8; tox -e do_format; quilt pop -a
build-package;
sbuild --resolve-alternatives --dist=jammy --arch=amd64 ../out/cloud-init_22.1-14-g2e17a0d6-0ubuntu1~22.04.4.dsc

I pushed this changeset to ppa:chad.smith/recipe-test-ground and debdiff is viewable here

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

Copy link
Copy Markdown
Collaborator

@cpaelzer cpaelzer left a comment

Choose a reason for hiding this comment

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

A few stylistic inline comments ...

Comment thread debian/changelog
@@ -1,3 +1,10 @@
cloud-init (22.1-14-g2e17a0d6-0ubuntu1~22.04.4) jammy; urgency=medium

* cherry-pick eee60329: Fix cloud-init status --wait when no datasource
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This isn't a "you have to redo this" level feedback, but FYI you'd usually reference any new patch added in debian/patches here.

Two common styles are

* d/p/cpick-eee60329-Fix-cloud-init-status-wait-when-no-datasource-found:
  cherry-pick eee60329: Fix cloud-init status --wait when no datasource
  found (#1349)
* cherry-pick eee60329: Fix cloud-init status --wait when no datasource
  found (#1349)
  - d/p/cpick-eee60329-Fix-cloud-init-status-wait-when-no-datasource-found 

The latter is more common if you have a lift of patches picked for one overall change/fix.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, this clearer.

"disabled" file, so "status --wait" will wait indefinitely if no
datasource is found.

LP: #1966085
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is great that you already have LP: #1966085 but for formal style (https://dep-team.pages.debian.net/deps/dep3/) you'd usually refer those like:

Origin: https://github.com/canonical/cloud-init/commit/eee60329
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1966085
Last-Update: 2022-03-24

In fact since you needed to modify it here it would be

Origin: backport, https://github.com/canonical/cloud-init/commit/eee60329
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1966085
Last-Update: 2022-03-24

If ever in doubt that link above has all you'd ever want to express in a patch :-)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to add the quilt patch metadata to all "cherry-picked" commits that cloud-init tooling adds via our simple cherry-pick script so I'll take this as a separate action item for a different PR to adapt our process for all future processed 'cherry-picks'. Good suggestion

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've also updated the quilt patch header in this PR.

@TheRealFalcon
Copy link
Copy Markdown
Contributor

What does "backport" mean here?

@cpaelzer
Copy link
Copy Markdown
Collaborator

What does "backport" mean here?

it means that one needed to modify (according to chads description) the patch to apply

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

LGTM (after we take Christian's comments into account).

Ran the procedure (with a missing quilt push -f) and got the same results. Also ran the integration test against your PPA and it passed.

@blackboxsw
Copy link
Copy Markdown
Collaborator Author

Closing this pull request as it was pushed directly to ubuntu/devel-22.1-hotfix branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants