Reorganize and overhaul Makefile & release archive workflows #9381

cevich · 2021-02-15T16:22:03Z

Reviewer Note: The diff here is enormous and likely unwieldy to look at all in one go. I've broken the changes up in stand-alone stages to reduce your eye-strain.

Fix incorrect version number output
Remove unnecessary/not-needed release.txt target
Don't shell to obtain current directory
Simplify Makefile help target
Reorganize Makefile with sections and guide
Overhaul Makefile binary and release worflows (includes Makefile: rename build artifact for windows and darwin #9918)
Exclude .gitignore from test req.

cevich · 2021-02-15T16:22:54Z

Note to me: Confirm OSX and Static build tasks capture release and binary artifacts

cevich · 2021-02-15T16:56:29Z

Oof..."homeless" and wrong arch errors 😖

cevich · 2021-02-16T17:51:39Z

ugh...this is getting out of hand...our Makefile has some really serious problems:

make podman-remote-release-darwin.zip
...cut...
cmd/go: unsupported GOOS/GOARCH pair release-darwin.zip/amd64

😢

cevich · 2021-02-22T19:03:10Z

Cirrus Build tasks are now producing release-ready archives:

podman-release.tar.gz (under gosrc) for F33, F32, U2010, and U2004 tasks.
podman-v3.1.0.msi and podman-remote-release-windows.zip (under gosrc) for Windows
podman-remote-release-darwin.zip (under gosrc) for OS-X
podman and podman-remote (under binary/bin) for Static Linux

cevich · 2021-02-22T19:29:29Z

@mheon PTAL - this fixes the missing OSX zip we noticed during the 3.0 release. I'm a little nervous about the target rename, please let me know if you have a better idea.

mheon · 2021-02-22T19:31:04Z

Why do we need the rename, for reference?

cevich · 2021-02-22T20:17:47Z

Why do we need the rename, for reference?

I tried to explain this in the commit message, but it's complex, so I probably failed. I'll try again:

Wildcard targets are greedy (match as much as possible), and the order they appear is also significant. Previously, the podman-remote-darwin-release target was not reachable. References to it were matching podman-remote-% instead, with
the % scooping up darwin-release or release-darwin.zip (instead of just darwin). Because the % is used for the GOOS value, building the release archive on OSX was failing to compile. Otherwise, calling podman-remote-darwin directly was working fine - which is probably why nobody noticed.

Since the podman-remote-% target is the most specific, it was simpler to rename it, than re-architect the zip, tar.gz, and msi targets (or massively re-arrange them). I'm sure there are other solutions possible, but the PR diff (along with the bald-spot on my head) might be much bigger. This change was the quickest + simplest I could come up with, but the target rename might impact some people (I dunno, maybe not?).

@edsantiago speaks Makefile fairly well, maybe he has some idea for a simpler solution that gets us our tarballs, zips, msi's, and cross-compiled binaries. Though, fair-warning: this is a rat's nest mapping exercise.

cevich · 2021-02-23T14:33:04Z

Update: Had an idea this morning for (perhaps) a better way to tackle this problem. I can start with a vastly simplified Makefile, where all targets simply touch a file. Perhaps this will allow me to re-arrange the targets in such a way so that they all function as intended, without me wanting to gouge out my eyeballs. Then use this as a template for updating the actual Makefile.

Ugh...still sounds like it will be quite a bit of work. @mheon is there any pressure to solving this problem in the near-term (e.g. for an important release/branch)?

mheon · 2021-02-23T14:37:23Z

Not really? And honestly, even if there were, I can build the OS X binaries myself without altering my own process. This obviously isn't ideal but it does work.

cevich · 2021-02-23T14:51:33Z

This obviously isn't ideal but it does work.

I feel the context here is important: An extremely long, detailed, manual podman release workflow. Still your're right, there is a "plan b" available, so perhaps this isn't super urgent. Thanks for the input. I'll flip this back to WIP, dabble at it as I'm able, but focus on other items for now.

github-actions · 2021-03-26T00:09:11Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2021-03-26T10:28:41Z

@cevich what should we do with this PR, it is still in WIP state?

cevich · 2021-03-26T17:41:29Z

Yes, still a WIP. I'm procrastinating on it until I finish working on the new get_ci_vm since Makefile is so brain-hurty. I'll resume work here next week most likely.

Also sort the explicit files by name, since the list is growing. Signed-off-by: Chris Evich <[email protected]>

Signed-off-by: Chris Evich <[email protected]>

Previously this was needed for an automated release process. That automation has long since been removed. Simplify the Makefile by removing the target and references. Signed-off-by: Chris Evich <[email protected]>

Instead of shelling out frequently to resolve the current directory, use the Makefile built-in `$(CURDIR)`. It has the exact same meaning w/in the context of a `Makefile`. Ref.: https://www.gnu.org/software/make/manual/html_node/Quick-Reference.html Signed-off-by: Chris Evich <[email protected]>

cevich · 2021-04-12T14:15:15Z

Confirmed, the _HLP* variables are only used on reference from the help target due to simple substitution at reference-time. Example (make target_one does not produce a foobarbaz file)

FOO = $(shell echo $$RANDOM)
BAR = one two $(FOO)
BAZ = $(shell echo "$(BAR)" | tee foobarbaz)

target_one:
        @echo "This is $@" | tee $@
        -cat foobarbaz

target_two:
        @echo "This is $(BAZ)" | tee $@
        -cat foobarbaz

I'll add a comment above the definitions to make this situation more clear.

An in-line Python script, while flexible, is arguably more complex and less stable than the long-lived `grep`, `awk`, and `printf`. Make use of these simple tools to display a column-aligned table of target and description help output. Also, the first target that appears in a Makefile is considered the default (when no target is specified on the command-line). However, despite it's name, the `default` target was not listed first. Fix this, and redefine "default" target to "all" as intended, instead of "help". Lastly, add a small workaround for a vim syntax-hilighting bug. Signed-off-by: Chris Evich <[email protected]>

Over several years the podman Makefile has become a bloated complex mess. This impedes both debugging and maintenance, besides causing general eye-strain. Fix this by adding a simple navigation/layout guide, to help developers quickly find what's needed. Re-organize the entire file according to the new layout guide. Add section headers that call out the purpose of the encompassed content, and are easy to locate with search-tools. Note: No recipes or definitions have been altered by this commit, only re-arranged. Signed-off-by: Chris Evich <[email protected]>

* Incorporate changes from abandoned containers#9918: Use dedicated `bin` sub-directories for `windows` and `darwin` when building `podman-remote`. The linux flavor remains under `bin` as before. * Fix MacOS Documentation-generation for release-packaging. The `install-podman-remote-%-docs` target requires local execution of `podman-remote`, but it was assuming GOOS=linux. Fix this by dynamically discovering the local OS/architecture type while still permitting cross-building of MacOS binaries under Linux. * Unify temporary directory/file behavior to use a common template. In case of left-over temporary items left in the repository, update the `clean` target accordingly to remove them. * Fix broken podman-remote-static and MacOS release archive targets mismatching the `podman-remote-%` target. Disambiguate this target for all platforms by spelling each out in full, instead of using a wild-card recipe. * Fix Windows-installer target to properly recognize existing output files and not constantly rebuild every time. * Include the podman version number in the Windows-installer target in case a user downloads multiple releases. * Include a subdirectory containing the podman version number for both `tar.gz` and `zip` targets. This prevents users clobbering existing directories when un-archiving from releases. Signed-off-by: Chris Evich <[email protected]>

cevich · 2021-04-12T14:25:40Z

Rebased and force-pushed with all'y'alls suggestions. Hopefully the tests will remain green.

edsantiago

LGTM, thank you! One test flake (#9751), restarted.

edsantiago · 2021-04-12T16:32:28Z

/lgtm
/hold

Makefile

jwhonce · 2021-04-12T17:10:48Z

Makefile

+	## TODO(ssbarnea): make version number predictable, it should not change
+	## on each execution, producing duplicates.


Should scrape_version() be exposed to take care of this TODO?

Oh, good catch. I have no idea why specifically that TODO was added, but from the text it seems we could remove it.

This came in by #4601

I looked through contrib/build_rpm.sh but don't see any references or direct use of hack/get_release_info.sh or scraping one of the podman-*-release target filenames.

@ssbarnea is this TODO in the Makefile package: target relevant to any of the release targets present in the Makefile or just something internal for the package: target?

(Let's not hold up this PR for the question...it's not worth re-running all the tests just to remove a TODO - if necessary).

rhatdan · 2021-04-12T19:16:07Z

/approve
/hold cancel

openshift-ci-robot · 2021-04-12T19:16:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, edsantiago, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [edsantiago,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ssbarnea · 2021-04-13T06:25:25Z

I hope no major changes come out of this from the behaviour point of view. What happens inside does bot concern me much.

I already use and parse that Makefile with https://pypi.org/project/mk/ as I consider it a good model of makefile that exposes/documented its targets.

cevich · 2021-04-13T14:56:06Z

I hope no major changes come out of this from the behaviour point of view.

Correct, this was my intention for the most part. I don't know anything about mk but I can summarize what happened here in case that's helpful for you:

Update the help target aesthetic appearance
Fix the (literal) default target, to redirect to (actual) default all target
Fix the various podman-*-release targets so they function for all OSes

HTH

ssbarnea · 2021-04-13T15:27:58Z

@cevich Yeah, nothing to worry. I was able to easily fix it with https://github.com/pycontribs/mk/pull/65/files -- this being the only side effect. In fact it worked quite nice.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 15, 2021

cevich force-pushed the add_make_release branch from 1f5a7e4 to 72480d1 Compare February 16, 2021 16:22

cevich changed the title ~~WIP: Cirrus: Make OSX and Static release archives~~ WIP: Fix inaccessible Makefile target & Cirrus: OSX Build artifact Feb 16, 2021

cevich force-pushed the add_make_release branch 5 times, most recently from 6f3fc60 to 1d63ef4 Compare February 22, 2021 15:27

cevich changed the title ~~WIP: Fix inaccessible Makefile target & Cirrus: OSX Build artifact~~ Fix inaccessible Makefile target & Cirrus: OSX Build artifact Feb 22, 2021

cevich marked this pull request as ready for review February 22, 2021 19:06

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2021

cevich marked this pull request as draft February 23, 2021 14:51

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 23, 2021

cevich changed the title ~~Fix inaccessible Makefile target & Cirrus: OSX Build artifact~~ WIP: Fix inaccessible Makefile target & Cirrus: OSX Build artifact Feb 23, 2021

github-actions bot added the stale-pr label Mar 26, 2021

rhatdan removed the stale-pr label Mar 26, 2021

cevich mentioned this pull request Apr 1, 2021

Makefile: rename build artifact for windows and darwin #9918

Closed

cevich added 4 commits April 12, 2021 09:59

Exclude .gitignore from test req.

de82434

Also sort the explicit files by name, since the list is growing. Signed-off-by: Chris Evich <[email protected]>

Fix incorrect version number output

e296b60

Signed-off-by: Chris Evich <[email protected]>

Remove unnecessary/not-needed release.txt target

837bb9f

Previously this was needed for an automated release process. That automation has long since been removed. Simplify the Makefile by removing the target and references. Signed-off-by: Chris Evich <[email protected]>

cevich added 3 commits April 12, 2021 10:19

cevich force-pushed the add_make_release branch from 97de2c1 to b6b0b6e Compare April 12, 2021 14:23

edsantiago approved these changes Apr 12, 2021

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2021

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2021

openshift-ci-robot assigned edsantiago Apr 12, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 12, 2021

jwhonce reviewed Apr 12, 2021

View reviewed changes

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2021

openshift-merge-robot merged commit 6933d46 into containers:master Apr 12, 2021

edsantiago mentioned this pull request Apr 14, 2021

remote: podman kill: loses exit code #9751

Closed

mheon mentioned this pull request Jun 11, 2021

podman-remote-static in release v3.2.0 is not static #10656

Closed

cevich deleted the add_make_release branch June 30, 2021 18:08

edsantiago mentioned this pull request Jun 28, 2022

[low pri] Makefile: bad dependencies on podman-remote #14756

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize and overhaul Makefile & release archive workflows #9381

Reorganize and overhaul Makefile & release archive workflows #9381

cevich commented Feb 15, 2021 •

edited

Loading

cevich commented Feb 15, 2021

cevich commented Feb 15, 2021

cevich commented Feb 16, 2021

cevich commented Feb 22, 2021

cevich commented Feb 22, 2021

mheon commented Feb 22, 2021

cevich commented Feb 22, 2021

cevich commented Feb 23, 2021

mheon commented Feb 23, 2021

cevich commented Feb 23, 2021

github-actions bot commented Mar 26, 2021

rhatdan commented Mar 26, 2021

cevich commented Mar 26, 2021

cevich commented Apr 12, 2021 •

edited

Loading

cevich commented Apr 12, 2021

edsantiago left a comment

edsantiago commented Apr 12, 2021

jwhonce Apr 12, 2021

cevich Apr 12, 2021

cevich Apr 12, 2021

cevich Apr 12, 2021

rhatdan commented Apr 12, 2021

openshift-ci-robot commented Apr 12, 2021

ssbarnea commented Apr 13, 2021

cevich commented Apr 13, 2021

ssbarnea commented Apr 13, 2021

		## TODO(ssbarnea): make version number predictable, it should not change
		## on each execution, producing duplicates.

Reorganize and overhaul Makefile & release archive workflows #9381

Reorganize and overhaul Makefile & release archive workflows #9381

Conversation

cevich commented Feb 15, 2021 • edited Loading

cevich commented Feb 15, 2021

cevich commented Feb 15, 2021

cevich commented Feb 16, 2021

cevich commented Feb 22, 2021

cevich commented Feb 22, 2021

mheon commented Feb 22, 2021

cevich commented Feb 22, 2021

cevich commented Feb 23, 2021

mheon commented Feb 23, 2021

cevich commented Feb 23, 2021

github-actions bot commented Mar 26, 2021

rhatdan commented Mar 26, 2021

cevich commented Mar 26, 2021

cevich commented Apr 12, 2021 • edited Loading

cevich commented Apr 12, 2021

edsantiago left a comment

Choose a reason for hiding this comment

edsantiago commented Apr 12, 2021

jwhonce Apr 12, 2021

Choose a reason for hiding this comment

cevich Apr 12, 2021

Choose a reason for hiding this comment

cevich Apr 12, 2021

Choose a reason for hiding this comment

cevich Apr 12, 2021

Choose a reason for hiding this comment

rhatdan commented Apr 12, 2021

openshift-ci-robot commented Apr 12, 2021

ssbarnea commented Apr 13, 2021

cevich commented Apr 13, 2021

ssbarnea commented Apr 13, 2021

cevich commented Feb 15, 2021 •

edited

Loading

cevich commented Apr 12, 2021 •

edited

Loading