Remove def release()#707
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates Nanvix “release” packaging into the existing build workflow by removing the separate release() command and refactoring packaging to reuse the already-produced build artifacts.
Changes:
- Update
.nanvix/z.pysobuild()also runs packaging + verification logic (and removerelease()). - Refactor
.nanvix/package.pyto stop doing an internal build and instead stage/install from existing artifacts, usingnanvix_zutil.paths. - Adjust
package.verify()API to no longer requirerepo_root.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .nanvix/z.py | Moves release packaging/verification into build() and removes the dedicated release() command. |
| .nanvix/package.py | Removes the redundant build step from package(), switches to paths.* helpers, and updates verify() signature. |
Comments suppressed due to low confidence (2)
.nanvix/package.py:22
pathsis imported twice, which is redundant and can confuse future edits.
import build as build_mod
import config
import lxml as lxml_mod
import ramfs as ramfs_mod
from nanvix_zutil import paths
.nanvix/package.py:78
package()accepts areleaseparameter but currently hard-codesrelease=Truewhen callingbuild_mod.install(). This makes the function ignore its own API and can break when callers passrelease=False(e.g., viaz.pykwargs).
platform=platform,
process_mode=process_mode,
memory_size=memory_size,
install_prefix=install_prefix,
release=True,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| package_mod.package( | ||
| sysroot, | ||
| toolchain, | ||
| **kwargs, # pyright: ignore[reportArgumentType] | ||
| run_fn=lambda *args, **kw: run(*args, docker=self.docker, **kw), # type: ignore[arg-type] | ||
| docker=self.docker is not None, | ||
| ) | ||
| package_mod.verify( | ||
| kwargs["platform"], # pyright: ignore[reportArgumentType] | ||
| kwargs["process_mode"], # pyright: ignore[reportArgumentType] | ||
| kwargs["memory_size"], # pyright: ignore[reportArgumentType] | ||
| ) |
There was a problem hiding this comment.
Currently, the NANVIX_RELEASE variable does not really make a difference. The only thing it does is strip some files for production builds. Additionally, NANVIX_RELEASE is never set in CI, so collapsing the behavior here is harmless.
That being said, this is a design decision we should take into account when cleaning up this repo.
| def clean(self) -> None: | ||
| """Remove build artifacts.""" | ||
| build_mod.clean(repo_root()) |
There was a problem hiding this comment.
./z release still exists, it's just automatic now.
| # Build. | ||
| build_mod.build( | ||
| sysroot, | ||
| toolchain, | ||
| repo_root, | ||
| platform=platform, | ||
| process_mode=process_mode, | ||
| memory_size=memory_size, | ||
| install_prefix=install_prefix, | ||
| release=True, | ||
| run_fn=run_fn, | ||
| docker=docker, | ||
| ) |
There was a problem hiding this comment.
AFAIK. this is unfortunately required, until we completely refactor how build is decoupled from test.
ada-x64
force-pushed
the
fix/release-override
branch
3 times, most recently
from
f11415a to
e8ead61
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.nanvix/package.py:78
package()accepts areleaseflag but the build step hard-codesrelease=True, so callers cannot actually control whether a release or non-release build is packaged. This also makesz.py's computedreleasevalue ineffective.
platform=platform,
process_mode=process_mode,
memory_size=memory_size,
install_prefix=install_prefix,
release=True,
run_fn=run_fn,
docker=docker,
| @@ -93,12 +93,11 @@ | |||
| docker=docker, | |||
| ) | |||
| release = os.environ.get(_MAKE_VAR_RELEASE, "no") == "yes" | ||
| kwargs = self._build_kwargs(release=release) | ||
| build_mod.build( | ||
| sysroot, | ||
| toolchain, |
| # Cleanup staging. | ||
| shutil.rmtree(release_staging) | ||
|
|
||
| print("Release tarballs created in dist/") |
ada-x64
force-pushed
the
fix/release-override
branch
from
e8ead61 to
028726e
Compare
Moves release builds to
def build(), removesdef release().Removes redundant build.
Towards nanvix/zutils#191
STACKED ON #705
Future work: #710