Repository: dandavison/delta
Pull request #279 (2020-08-07 13:33:28)

Add TARGET=aarch64-unknown-linux-gnu cross-compile
Assuming etc/ci/before_install.sh is already set up correctly, aarch64 may work now. Untested.

Relates to #278 

dandavison: > Assuming `etc/ci/before_install.sh` is already set up correctly

Hi @gavinbeatty, can you expand on this? `etc/ci/before_install.sh` is  in this git repo so feel free to propose changes to it.
dandavison: I'm not a travis expert, but I do want to learn.

It looks to me like your build failed: https://travis-ci.com/github/dandavison/delta/jobs/369722313

Compare with https://travis-ci.com/github/dandavison/delta/jobs/369722308

The build page for your first commit is https://github.com/dandavison/delta/runs/958453193

Do you have access to those pages?
gavinbeatty: I was saying etc/ci/before_install.sh appeared to already support aarch64, but it wasn't set up in travis, so who knows if the script has all the necessary changes. Turns out it needed changes, attempted in 0aee25a.

But this build also failed, due to inconsistent/missing setting of CC for cross compile. Travis is exporting:
```
$ export HOST=x86_64-unknown-linux-gnu
$ export PROJECT_NAME=delta
$ export PACKAGE_NAME=git-delta
$ export TARGET=aarch64-unknown-linux-gnu
$ export CC_arm_unknown_linux_gnueabihf=/usr/bin/aarch64-linux-gnu-gcc-4.8
$ export CARGO_TARGET_ARM_UNKNOWN_LINUX_GNUEABIHF_LINKER=aarch64-linux-gnu-gcc-4.8
```

But cargo is seeing:
```
TARGET = Some("aarch64-unknown-linux-gnu")
OPT_LEVEL = Some("0")
HOST = Some("x86_64-unknown-linux-gnu")
CC_aarch64-unknown-linux-gnu = None
CC_aarch64_unknown_linux_gnu = None
TARGET_CC = None
CC = None
CROSS_COMPILE = None
CFLAGS_aarch64-unknown-linux-gnu = None
CFLAGS_aarch64_unknown_linux_gnu = None
TARGET_CFLAGS = None
CFLAGS = None
CRATE_CC_NO_DEFAULTS = None
DEBUG = Some("true")
CARGO_CFG_TARGET_FEATURE = None
running: "aarch64-linux-gnu-gcc"
```

It should be running "aarch64-linux-gnu-gcc-4.8".

Should be an easy fix, but is there any reason gcc-4.8 is chosen specifically? Would using the default gcc version be acceptable to you, @dandavison?
gavinbeatty: https://travis-ci.com/github/dandavison/delta/jobs/369732634 succeeded.
dandavison: Great! @navarroaxel or @Kr1ss-XD would you have a moment to confirm this PR looks good to you?

For the record, basically I originally copied my travis config from bat, so the gcc version is inherited from PRs like https://github.com/sharkdp/bat/pull/297.
navarroaxel: hey @gavinbeatty could you please sort the matrix to get all the Linux compilations together? 
navarroaxel: The AUR `git-delta-bin` use the `.deb` packages as source, could you generate the `.deb` for `aarch64` too?
gavinbeatty: Yes, I understand now, I'll figure that out.
gavinbeatty: etc/ci/before_deploy.sh appears to already support aarch64 deb creation, though it doesn't appear to run that script at this stage of a PR. Eyeballing it, it looks good.

@dandavison I presume this is only run when you tag, but that's just my guess.

@navarroaxel what do you mean regarding the sorting the matrix? I do think before_install.sh could be folded into travis.yml via a stanza like below, and I'm sure there are more simplifications that could be made, but I'd rather do that later (if ever), after we get it working the way armhf is done already.
```
      addons:
        apt:
          packages:
            - gcc-4.8
```
gavinbeatty: @dandavison to clarify the main point, if we can do a test deploy now that would be useful, because aarch64 deb creation should work already. If we can't test, perhaps merge and create a beta tag to test it?
navarroaxel: @gavinbeatty I mean this sort in the `matrix`:

```yaml
matrix
  include:
    - os: linux
      rust: stable
      env: TARGET=x86_64-unknown-linux-gnu
    - os: linux
      rust: stable
      env:
        - TARGET=x86_64-unknown-linux-musl
        - CC_x86_64_unknown_linux_musl=/usr/bin/musl-gcc
    - os: linux
      rust: stable
      env: TARGET=i686-unknown-linux-gnu
    - os: linux
      rust: stable
      env:
        - TARGET=arm-unknown-linux-gnueabihf
        - CC_arm_unknown_linux_gnueabihf=/usr/bin/arm-linux-gnueabihf-gcc-4.8
        - CARGO_TARGET_ARM_UNKNOWN_LINUX_GNUEABIHF_LINKER=arm-linux-gnueabihf-gcc-4.8
    - os: linux
      rust: stable
      env:
        - TARGET=aarch64-unknown-linux-gnu
        - CC_aarch64_unknown_linux_gnu=/usr/bin/aarch64-linux-gnu-gcc-4.8
        - CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-linux-gnu-gcc-4.8
    - os: windows
      rust: stable
      env: TARGET=x86_64-pc-windows-msvc
    - os: osx
      rust: stable
      env: TARGET=x86_64-apple-darwin
```
dandavison: > @dandavison I presume this is only run when you tag, but that's just my guess.

My understanding is that certain things run on every commit but that full upload of all assets to the release area happens automatically when I push a tag.

```yaml
deploy:
  provider: releases
  file:
    - $PROJECT_NAME-$TRAVIS_TAG-$TARGET.*
    - $PACKAGE_NAME*.deb
  ...
  on:
    tags: true
```

> , if we can do a test deploy now that would be useful, because aarch64 deb creation should work already. If we can't test, perhaps merge and create a beta tag to test it?

Yes, I don't know how to do a test deploy without pushing a tag. But I would like to know. One thing I've been wondering is whether I can host a set of "latest master snapshot" binaries for people to use to test the bleeding edge.
navarroaxel: Maybe we can do a `pre-release` to build the assets.
gavinbeatty: https://docs.github.com/en/github/administering-a-repository/managing-releases-in-a-repository describes how to create a prerelease. There is also a `draft` option in travis which may or may not be useful: https://docs.travis-ci.com/user/deployment/releases/
dandavison: I'll admit my knowledge is more focused on application development than on infrastructure.

> If we can't test, perhaps merge and create a beta tag to test it?

I'm concerned that there are things out there watching tags in the delta repo. I guess the way tags trigger things, and other aspects of this, are still a bit mysterious to me.

For a draft release, it sounds like the assets don't get pushed to the hosting area automatically; we'd have to copy them from somewhere.

I'm sure you understand that Delta development and maintenance takes some time! So I think I don't want to invest time in getting more confident with release pipelines/infrastructure right now. If anyone would like to test out the asset build in a separate repository that is a fork of delta then that would be great. Otherwise I think I'm inclined to merge reasonable-looking change to the travis config that have green builds and see what happens at next conventional release. (And not document anything in the README until it is proven to work.) If this is a bit short-sighted/luddite of me then please say so!
gavinbeatty: No, that's pretty reasonable. I may experiment with travis and test this out in my fork, but maybe I won't get around to it.

But yes, if you could merge this before the next "natural" release, that would be great.
dandavison: Cool, will do. LGTY to merge this @navarroaxel ?

Patch:
diff --git a/etc/ci/before_install.sh b/etc/ci/before_install.sh
index 9acf602..d26cf1a 100755
--- a/etc/ci/before_install.sh
+++ b/etc/ci/before_install.sh
@@ -33,5 +33,6 @@ if [[ $TARGET == aarch64-unknown-linux-gnu ]]; then
     sudo apt-get install -y \
         gcc-4.8-aarch64-linux-gnu \
         binutils-aarch64-linux-gnu \
-        gcc-aarch64-linux-gnu
+        libc6-arm64-cross \
+        libc6-dev-arm64-cross
 fi
diff --git a/src/bat/.travis.yml b/src/bat/.travis.yml
index db97aa2..ec594b2 100644
--- a/src/bat/.travis.yml
+++ b/src/bat/.travis.yml
@@ -25,6 +25,12 @@ matrix:
         - TARGET=arm-unknown-linux-gnueabihf
         - CC_arm_unknown_linux_gnueabihf=/usr/bin/arm-linux-gnueabihf-gcc-4.8
         - CARGO_TARGET_ARM_UNKNOWN_LINUX_GNUEABIHF_LINKER=arm-linux-gnueabihf-gcc-4.8
+    - os: linux
+      rust: stable
+      env:
+        - TARGET=aarch64-unknown-linux-gnu
+        - CC_aarch64_unknown_linux_gnu=/usr/bin/aarch64-linux-gnu-gcc-4.8
+        - CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-linux-gnu-gcc-4.8
 
 sudo: required
 
diff --git a/src/delta.rs b/src/delta.rs
index 814401f..db3c752 100644
--- a/src/delta.rs
+++ b/src/delta.rs
@@ -188,6 +188,9 @@ where
 
 /// Should a handle_* function be called on this element?
 fn should_handle(state: &State, config: &Config) -> bool {
+    if *state == State::HunkHeader && config.line_numbers {
+        return true;
+    }
     let style = config.get_style(state);
     !(style.is_raw && style.decoration_style == DecorationStyle::NoDecoration)
 }
@@ -413,7 +416,9 @@ fn handle_hunk_header_line(
     let (raw_code_fragment, line_numbers) = parse::parse_hunk_header(&line);
     // Emit the hunk header, with any requested decoration
     if config.hunk_header_style.is_raw {
-        writeln!(painter.writer)?;
+        if config.hunk_header_style.decoration_style != DecorationStyle::NoDecoration {
+            writeln!(painter.writer)?;
+        }
         draw_fn(
             painter.writer,
             &format!("{} ", line),
diff --git a/src/features/line_numbers.rs b/src/features/line_numbers.rs
index 092629c..ca024ca 100644
--- a/src/features/line_numbers.rs
+++ b/src/features/line_numbers.rs
@@ -549,6 +549,16 @@ pub mod tests {
         assert_eq!(lines.next().unwrap(), "     ⋮10000│bb = 2");
     }
 
+    #[test]
+    fn test_color_only() {
+        let config = make_config_from_args(&["--line-numbers", "--color-only"]);
+        let output = run_delta(TWO_MINUS_LINES_DIFF, &config);
+        let mut lines = output.lines().skip(5);
+        let (line_1, line_2) = (lines.next().unwrap(), lines.next().unwrap());
+        assert_eq!(strip_ansi_codes(line_1), " 1  ⋮    │-a = 1");
+        assert_eq!(strip_ansi_codes(line_2), " 2  ⋮    │-b = 2");
+    }
+
     pub const TWO_MINUS_LINES_DIFF: &str = "\
 diff --git i/a.py w/a.py
 index 223ca50..e69de29 100644


Test patch:
diff --git a/src/tests/test_example_diffs.rs b/src/tests/test_example_diffs.rs
index 7fc6773..a2684ea 100644
--- a/src/tests/test_example_diffs.rs
+++ b/src/tests/test_example_diffs.rs
@@ -811,6 +811,28 @@ src/align.rs
         assert!(output.contains("@@ -71,11 +71,8 @@ impl<'a> Alignment<'a> {"));
     }
 
+    #[test]
+    fn test_hunk_header_style_raw_no_decoration_with_line_numbers() {
+        let config = integration_test_utils::make_config_from_args(&[
+            "--hunk-header-style",
+            "raw",
+            "--hunk-header-decoration-style",
+            "omit",
+            "--line-numbers",
+        ]);
+        let output = integration_test_utils::run_delta(GIT_DIFF_SINGLE_HUNK, &config);
+        assert!(output.contains("
+@@ -71,11 +71,8 @@ impl<'a> Alignment<'a> {"));
+        assert!(!output.contains("
+
+@@ -71,11 +71,8 @@ impl<'a> Alignment<'a> {"));
+        ansi_test_utils::assert_line_has_no_color(
+            &output,
+            9,
+            "@@ -71,11 +71,8 @@ impl<'a> Alignment<'a> {",
+        );
+    }
+
     #[test]
     fn test_hunk_header_style_colored_input_color_is_stripped_under_normal() {
         let config = integration_test_utils::make_config_from_args(&[