Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build linux and darwin arm64 binary #43

Merged
merged 14 commits into from
Jun 8, 2021
Merged

Conversation

wiget
Copy link
Contributor

@wiget wiget commented Jan 27, 2021

This allow using ls-lint on Linux run on arm64 CPU (Raspberry Pi4, Rock Pi4, AWS EC2 with AWS Graviton CPU (A1, T4g, C6g, M6g, R6g)).

@loeffel-io loeffel-io self-assigned this Jan 27, 2021
@loeffel-io
Copy link
Owner

Thank you very much for this pull request @wiget - looks great

2 updates i would love to see:

  • We should also add the different darwin versions
  • We should also add a docker arm version

@loeffel-io
Copy link
Owner

closes #43

@wiget
Copy link
Contributor Author

wiget commented Jan 27, 2021

@loeffel-io If by different darwin version you mean version for Apple Silicon then this is planned for Go 1.16.

I will look at docker image for linux/arm64.

@loeffel-io
Copy link
Owner

Hey @wiget , crazy that darwin arm64 is not released yet

Alright 👍

@wiget
Copy link
Contributor Author

wiget commented Jan 27, 2021

@loeffel-io I finished Docker part too. Please review.

@wiget
Copy link
Contributor Author

wiget commented Jan 29, 2021

@loeffel-io anything missing or need to be adjusted before merge?

@wiget
Copy link
Contributor Author

wiget commented Feb 1, 2021

@loeffel-io I'm trying to bring Renovate to arm64. As ls-lint is dependency of Renovate this PR is pre-requisite of my future work on the topic.

@loeffel-io
Copy link
Owner

loeffel-io commented Feb 2, 2021

Thanks @wiget, i currently try to dig into that - first of all: you must update the package.json version to release the new ls-lint version: v1.10.0

edit: done

@loeffel-io
Copy link
Owner

Thanks for the update but the branch is also very important ✌️

@wiget
Copy link
Contributor Author

wiget commented Feb 4, 2021

@loeffel-io I'm not sure if I understand correctly. Can you describe when Docker image should be build and published to Docker Hub? Than I will try to translate this into .drone.yml.

@wiget
Copy link
Contributor Author

wiget commented Feb 11, 2021

@loeffel-io any changes needed or this PR can be merged as-is?

@wiget
Copy link
Contributor Author

wiget commented Feb 15, 2021

@loeffel-io Any chance for this PR? 🙃

@wiget
Copy link
Contributor Author

wiget commented Feb 18, 2021

@loeffel-io Good news! golang 1.16 released with support for Apple Silicon. I added relevant changes to build darwin/arm64 version.

@wiget wiget changed the title Build linux arm64 binary Build linux and darwin arm64 binary Feb 18, 2021
@wiget wiget requested a review from loeffel-io February 22, 2021 09:04
@wiget wiget mentioned this pull request Feb 22, 2021
@loeffel-io
Copy link
Owner

Thank you very much! I think the biggest problem is that my ci works only for amd images - i need some more time to dig into this! sorry for the delay

@kxxoling
Copy link

kxxoling commented Apr 13, 2021

Any updates? I tries to install ls-lint on my M1 Mac mini but failed...

@caiofbpa
Copy link
Contributor

Any news on merging this PR?

.drone.yml Outdated
image: plugins/manifest
settings:
auto_tag: true
ignore_missing: true
Copy link
Owner

Choose a reason for hiding this comment

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

why do we want to ignore missing source images?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think is needed. I just copied this from some other project.

Copy link
Owner

Choose a reason for hiding this comment

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

i removed it for now

@loeffel-io loeffel-io merged commit 455ad95 into loeffel-io:master Jun 8, 2021
@loeffel-io
Copy link
Owner

v1.10.0-beta.1 is online; please test the arm versions

@wiget
Copy link
Contributor Author

wiget commented Jun 8, 2021

Looks like it works on RockPi 4

root@288e2632b2e9:/tmp/test# npx @ls-lint/[email protected]
npx: installed 1 in 2.723s
2021/06/08 14:21:31 ./node_modules failed for rules: kebabcase
2021/06/08 14:21:31 ./node_modules/.bin failed for rules: kebabcase
2021/06/08 14:21:31 ./node_modules/@ls-lint failed for rules: kebabcase
root@288e2632b2e9:/tmp/test# uname -a
Linux 288e2632b2e9 5.10.21-rockchip64 #21.02.3 SMP PREEMPT Mon Mar 8 01:05:08 UTC 2021 aarch64 GNU/Linux

@loeffel-io
Copy link
Owner

Would love to hear more feedback about the v1.10.1-beta.1 before release ✌️

@padevone
Copy link

padevone commented Jun 23, 2021

@loeffel-io, thanks for changes, but I think I have found a typo:

https://github.com/loeffel-io/ls-lint/blob/v1.10.0-beta.1/npm/bin/cli.js#L29
In this case is value ls-lint-darwin-arm64?

I have tested this on M1

@loeffel-io
Copy link
Owner

@wiget

@wiget
Copy link
Contributor Author

wiget commented Jun 28, 2021

@padevone that's true. Are you able to submit PR with the fix? If not I will create PR later this week.

@loeffel-io
Copy link
Owner

would love to see a PR for this, currently at holidays @wiget

caiofbpa added a commit to caiofbpa/ls-lint that referenced this pull request Jul 14, 2021
This fixes a typo and is related to loeffel-io#43
@caiofbpa
Copy link
Contributor

Here: #57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants