Skip to content

Commit

Permalink
feat(forc-pkg): Print *before* compiling each package. Only print aft…
Browse files Browse the repository at this point in the history
…er on warnings/error. (FuelLabs#4187)

## Description

We now print with the following format *prior* to compiling a package:

```
Compiling <ty> <name> (<src>)
```

Rather than only printing `Compiled` after compilation is complete.

Closes FuelLabs#3780.

This also aims to improve the formatting of the output a little by using
ansi_term styling to distinguish between action, program kind, package
name and source more clearly.

The fetching, adding and removing actions have had their formatting
updated to match. E.g.

![Screenshot from 2023-02-26
20-14-25](https://user-images.githubusercontent.com/4587373/221402630-28b65eb3-ddfb-491f-81a6-4aead5538a28.png)

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
mitchmindtree authored Feb 26, 2023
1 parent 254c2e2 commit f357e24
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 63 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions forc-pkg/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ repository = "https://github.com/FuelLabs/sway"
description = "Building, locking, fetching and updating Sway projects as Forc packages."

[dependencies]
ansi_term = "0.12"
anyhow = "1"
fd-lock = "3.0"
forc-tracing = { version = "0.35.3", path = "../forc-tracing" }
Expand Down
31 changes: 18 additions & 13 deletions forc-pkg/src/lock.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::{pkg, source, DepKind, Edge};
use anyhow::{anyhow, Result};
use forc_tracing::{println_green, println_red};
use petgraph::{visit::EdgeRef, Direction};
use serde::{Deserialize, Serialize};
use std::{
Expand Down Expand Up @@ -346,8 +345,15 @@ where
{
for pkg in removed {
if !member_names.contains(&pkg.name) {
let name = name_or_git_unique_string(pkg);
println_red(&format!(" Removing {name}"));
let src = match pkg.source.starts_with(source::git::Pinned::PREFIX) {
true => format!(" {}", pkg.source),
false => "".to_string(),
};
tracing::info!(
" {} {}{src}",
ansi_term::Colour::Red.bold().paint("Removing"),
ansi_term::Style::new().bold().paint(&pkg.name)
);
}
}
}
Expand All @@ -358,20 +364,19 @@ where
{
for pkg in removed {
if !member_names.contains(&pkg.name) {
let name = name_or_git_unique_string(pkg);
println_green(&format!(" Adding {name}"));
let src = match pkg.source.starts_with(source::git::Pinned::PREFIX) {
true => format!(" {}", pkg.source),
false => "".to_string(),
};
tracing::info!(
" {} {}{src}",
ansi_term::Colour::Green.bold().paint("Adding"),
ansi_term::Style::new().bold().paint(&pkg.name)
);
}
}
}

// Only includes source after the name for git sources for friendlier printing.
fn name_or_git_unique_string(pkg: &PkgLock) -> Cow<str> {
match pkg.source.starts_with(source::git::Pinned::PREFIX) {
true => Cow::Owned(pkg.unique_string()),
false => Cow::Borrowed(&pkg.name),
}
}

#[cfg(test)]
mod tests {
use sway_core::fuel_prelude::fuel_tx;
Expand Down
14 changes: 11 additions & 3 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::{
};
use anyhow::{anyhow, bail, Context, Error, Result};
use forc_util::{
default_output_directory, find_file_name, kebab_to_snake_case, print_on_failure,
print_on_success, user_forc_directory,
default_output_directory, find_file_name, kebab_to_snake_case, print_compiling,
print_on_failure, print_warnings, user_forc_directory,
};
use fuel_abi_types::program_abi;
use petgraph::{
Expand Down Expand Up @@ -1751,7 +1751,7 @@ pub fn compile(
}) if bc_res.errors.is_empty()
&& (bc_res.warnings.is_empty() || !build_profile.error_on_warnings) =>
{
print_on_success(terse_mode, &pkg.name, &bc_res.warnings, &tree_type);
print_warnings(terse_mode, &pkg.name, &bc_res.warnings, &tree_type);

if let ProgramABI::Fuel(ref mut json_abi_program) = json_abi_program {
if let Some(ref mut configurables) = json_abi_program.configurables {
Expand Down Expand Up @@ -2077,6 +2077,14 @@ pub fn build(
let mut source_map = SourceMap::new();
let pkg = &plan.graph()[node];
let manifest = &plan.manifest_map()[&pkg.id()];
let program_ty = manifest.program_type().ok();

print_compiling(
program_ty.as_ref(),
&pkg.name,
&pkg.source.display_compiling(manifest.dir()),
);

let constants = if let Some(injected_ctc) = const_inject_map.get(pkg) {
let mut constants = manifest.config_time_constants();
constants.extend(
Expand Down
7 changes: 6 additions & 1 deletion forc-pkg/src/source/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,12 @@ impl source::Fetch for Pinned {
{
let _guard = lock.write()?;
if !repo_path.exists() {
info!(" Fetching {}", self);
info!(
" {} {} {}",
ansi_term::Color::Green.bold().paint("Fetching"),
ansi_term::Style::new().bold().paint(ctx.name),
self
);
fetch(ctx.fetch_id(), ctx.name(), self)?;
}
}
Expand Down
32 changes: 31 additions & 1 deletion forc-pkg/src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ pub(crate) enum DependencyPath {
Root(PinnedId),
}

/// A wrapper type for providing `Display` implementations for compiling msgs.
pub struct DisplayCompiling<'a, T> {
source: &'a T,
manifest_dir: &'a Path,
}

/// Error returned upon failed parsing of `SourcePinned::from_str`.
#[derive(Clone, Debug)]
pub struct PinnedParseError;
Expand Down Expand Up @@ -253,6 +259,19 @@ impl Pinned {
_ => None,
}
}

/// Wrap `self` in some type able to be formatted for the compiling output.
///
/// This refers to `<source>` in the following:
/// ```ignore
/// Compiling <kind> <name> (<source>)
/// ```
pub fn display_compiling<'a>(&'a self, manifest_dir: &'a Path) -> DisplayCompiling<'a, Self> {
DisplayCompiling {
source: self,
manifest_dir,
}
}
}

impl<'a> PinCtx<'a> {
Expand All @@ -276,7 +295,18 @@ impl fmt::Display for Pinned {
Self::Member => write!(f, "member"),
Self::Path(src) => src.fmt(f),
Self::Git(src) => src.fmt(f),
Self::Registry(_reg) => unimplemented!("pkg registries not yet implemented"),
Self::Registry(_reg) => todo!("pkg registries not yet implemented"),
}
}
}

impl<'a> fmt::Display for DisplayCompiling<'a, Pinned> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.source {
Pinned::Member => self.manifest_dir.display().fmt(f),
Pinned::Path(_src) => self.manifest_dir.display().fmt(f),
Pinned::Git(src) => src.fmt(f),
Pinned::Registry(_src) => todo!("registry dependencies not yet implemented"),
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions forc-tracing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ pub fn println_red_err(txt: &str) {
println_std_err(txt, Colour::Red);
}

pub fn println_green_err(txt: &str) {
println_std_err(txt, Colour::Green);
}

fn println_std_out(txt: &str, color: Colour) {
tracing::info!("{}", color.paint(txt));
}
Expand Down
79 changes: 38 additions & 41 deletions forc-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use annotate_snippets::{
display_list::{DisplayList, FormatOptions},
snippet::{Annotation, AnnotationType, Slice, Snippet, SourceAnnotation},
};
use ansi_term::Colour;
use anyhow::{bail, Result};
use forc_tracing::{println_green_err, println_red_err, println_yellow_err};
use forc_tracing::{println_red_err, println_yellow_err};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::str;
Expand Down Expand Up @@ -175,59 +176,55 @@ pub fn git_checkouts_directory() -> PathBuf {
user_forc_directory().join("git").join("checkouts")
}

pub fn print_on_success(
terse_mode: bool,
proj_name: &str,
warnings: &[CompileWarning],
tree_type: &TreeType,
) {
let type_str = match &tree_type {
pub fn program_type_str(ty: &TreeType) -> &'static str {
match ty {
TreeType::Script {} => "script",
TreeType::Contract {} => "contract",
TreeType::Predicate {} => "predicate",
TreeType::Library { .. } => "library",
};

if !terse_mode {
warnings.iter().for_each(format_warning);
}
}

pub fn print_compiling(ty: Option<&TreeType>, name: &str, src: &dyn std::fmt::Display) {
// NOTE: We can only print the program type if we can parse the program, so
// program type must be optional.
let ty = match ty {
Some(ty) => format!("{} ", program_type_str(ty)),
None => "".to_string(),
};
tracing::error!(
" {} {ty}{} ({src})",
Colour::Green.bold().paint("Compiling"),
ansi_term::Style::new().bold().paint(name)
);
}

pub fn print_warnings(
terse_mode: bool,
proj_name: &str,
warnings: &[CompileWarning],
tree_type: &TreeType,
) {
if warnings.is_empty() {
println_green_err(&format!(" Compiled {type_str} {proj_name:?}."));
} else {
println_yellow_err(&format!(
" Compiled {} {:?} with {} {}.",
type_str,
proj_name,
warnings.len(),
if warnings.len() > 1 {
"warnings"
} else {
"warning"
}
));
return;
}
}
let type_str = program_type_str(tree_type);

pub fn print_on_success_library(terse_mode: bool, proj_name: &str, warnings: &[CompileWarning]) {
if !terse_mode {
warnings.iter().for_each(format_warning);
}

if warnings.is_empty() {
println_green_err(&format!(" Compiled library {proj_name:?}."));
} else {
println_yellow_err(&format!(
" Compiled library {:?} with {} {}.",
proj_name,
warnings.len(),
if warnings.len() > 1 {
"warnings"
} else {
"warning"
}
));
}
println_yellow_err(&format!(
" Compiled {} {:?} with {} {}.",
type_str,
proj_name,
warnings.len(),
if warnings.len() > 1 {
"warnings"
} else {
"warning"
}
));
}

pub fn print_on_failure(terse_mode: bool, warnings: &[CompileWarning], errors: &[CompileError]) {
Expand Down

0 comments on commit f357e24

Please sign in to comment.