hexops/mach
1
0
Fork 0
Code Issues 152 Pull requests 18 Releases Activity Actions 3

CI: fixes #429

Merged
alichraghi merged 4 commits from main into main 2022-07-24 16:28:57 +00:00
Conversation 22 Commits 4 Files changed 7 +49 -32
alichraghi commented 2022-07-23 08:26:12 +00:00 (Migrated from github.com)
Copy link
  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.
- [x] By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.
emidoots (Migrated from github.com) reviewed 2022-07-23 15:10:31 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
emidoots (Migrated from github.com) commented 2022-07-23 15:10:30 +00:00
Copy link

I've been making an effort to not create unused static libraries (which add to compile times), and not to install them for the reasons described in https://github.com/hexops/mach/issues/232

This change and the GLFW change below go against that. But why? How's it helpful?

I've been making an effort to not create unused static libraries (which add to compile times), and not to install them for the reasons described in https://github.com/hexops/mach/issues/232 This change and the GLFW change below go against that. But why? How's it helpful?
alichraghi (Migrated from github.com) reviewed 2022-07-23 15:46:51 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
alichraghi (Migrated from github.com) commented 2022-07-23 15:46:50 +00:00
Copy link

because zig build need to do something. otherwise e.g cross-compiling in CI is pointless

because `zig build` need to do something. otherwise e.g cross-compiling in CI is pointless
emidoots (Migrated from github.com) reviewed 2022-07-24 02:29:57 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 02:29:57 +00:00
Copy link

Either this should be just libasound2 (not -dev), or whatever header dependency we have in here should be distributed via https://github.com/hexops/sdk-linux-x86_64 and https://github.com/hexops/sdk-linux-aarch64 instead.

Either this should be just `libasound2` (not `-dev`), or whatever header dependency we have in here should be distributed via https://github.com/hexops/sdk-linux-x86_64 and https://github.com/hexops/sdk-linux-aarch64 instead.
emidoots (Migrated from github.com) reviewed 2022-07-24 03:36:52 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 03:36:52 +00:00
Copy link

It's a bit unfortunate we now have different formatting for CI files throughout this repository.

I suggest we add a new script: dev/ci-lint.sh which runs:

on all our CI yml files.

It's a bit unfortunate we now have different formatting for CI files throughout this repository. I suggest we add a new script: `dev/ci-lint.sh` which runs: * actionlint: https://github.com/rhysd/actionlint/blob/main/docs/install.md * Some YAML formatter.. preferable not one from the npm ecosystem, maybe this Go one https://github.com/UltiRequiem/yamlfmt on all our CI yml files.
emidoots (Migrated from github.com) reviewed 2022-07-24 03:40:44 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
emidoots (Migrated from github.com) commented 2022-07-24 03:40:44 +00:00
Copy link

OK, I see what you're going for here. But it doesn't seem like we should .install() these, as they are for testing only after all.

Separately, I want to ask: does zig build not build the test binary themselves? It seems to me we should be able to have zig build build the tests.. and those should build for the target OS, too - and then we wouldn't need to build useless static libraries like this.

OK, I see what you're going for here. But it doesn't seem like we should `.install()` these, as they are for testing only after all. Separately, I want to ask: does `zig build` not build the test binary themselves? It seems to me we should be able to have `zig build` build the tests.. and those should build for the target OS, too - and then we wouldn't need to build useless static libraries like this.
emidoots (Migrated from github.com) reviewed 2022-07-24 03:41:07 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 03:41:07 +00:00
Copy link

(I can address this in a future PR)

(I can address this in a future PR)
emidoots (Migrated from github.com) reviewed 2022-07-24 03:41:25 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 03:41:25 +00:00
Copy link

(should address this before merging)

(should address this before merging)
emidoots (Migrated from github.com) reviewed 2022-07-24 03:41:32 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
emidoots (Migrated from github.com) commented 2022-07-24 03:41:32 +00:00
Copy link

(should address this before merging)

(should address this before merging)
alichraghi (Migrated from github.com) reviewed 2022-07-24 06:42:16 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
alichraghi (Migrated from github.com) commented 2022-07-24 06:42:16 +00:00
Copy link

there's b.addTestExe

there's `b.addTestExe`
alichraghi (Migrated from github.com) reviewed 2022-07-24 06:43:22 +00:00
.github/workflows/ci.yml
alichraghi (Migrated from github.com) commented 2022-07-24 06:43:21 +00:00
Copy link

i'd tried without -dev but it didn't work. i do the same in my machine (linux mint)

i'd tried without `-dev` but it didn't work. i do the same in my machine (linux mint)
alichraghi (Migrated from github.com) reviewed 2022-07-24 07:22:58 +00:00
.github/workflows/ci.yml
alichraghi (Migrated from github.com) commented 2022-07-24 07:22:58 +00:00
Copy link

yamlfmt results the same output. at least im gonna say it has nothing to do with quotes ", '

EDIT: i did a lot of research but didn't find a good yaml formatter. i think we should just have to be careful with yaml files

`yamlfmt` results the same output. at least im gonna say it has nothing to do with quotes `"`, `'` **EDIT:** i did a lot of research but didn't find a good yaml formatter. i think we should just have to be careful with yaml files
emidoots (Migrated from github.com) reviewed 2022-07-24 08:47:37 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
emidoots (Migrated from github.com) commented 2022-07-24 08:47:37 +00:00
Copy link

addTestExe looks good to me!

`addTestExe` looks good to me!
emidoots (Migrated from github.com) reviewed 2022-07-24 08:48:44 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 08:48:44 +00:00
Copy link

What part about it didn't work? This would indicate cross-compilation won't work

What part about it didn't work? This would indicate cross-compilation won't work
emidoots (Migrated from github.com) reviewed 2022-07-24 08:49:59 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 08:49:59 +00:00
Copy link

To be clear, I am fine with the formatting being anything (double quotes, whatever) as long as we have a way for it to be consistent. We can use yamlfmt.

To be clear, I am fine with the formatting being anything (double quotes, whatever) as long as we have a way for it to be consistent. We can use yamlfmt.
alichraghi (Migrated from github.com) reviewed 2022-07-24 12:22:44 +00:00
.github/workflows/ci.yml
alichraghi (Migrated from github.com) commented 2022-07-24 12:22:43 +00:00
Copy link

missing headers/static libs

missing headers/static libs
alichraghi (Migrated from github.com) reviewed 2022-07-24 12:26:35 +00:00
.github/workflows/ci.yml
alichraghi (Migrated from github.com) commented 2022-07-24 12:26:35 +00:00
Copy link

i think actionlint is enough and yaml formatters doesn't make any change as long as we just edit github workflows. that's why there's no many yaml formatters, or at least they are unused in most cases
i mean yaml is already very strict. formatting a formatted file make no sense

i think **actionlint** is enough and yaml formatters doesn't make any change as long as we just edit github workflows. that's why there's no many yaml formatters, or at least they are unused in most cases i mean yaml is already very strict. formatting a formatted file make no sense
emidoots (Migrated from github.com) reviewed 2022-07-24 16:24:37 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 16:24:37 +00:00
Copy link

https://github.com/hexops/mach/issues/430

https://github.com/hexops/mach/issues/430
emidoots (Migrated from github.com) reviewed 2022-07-24 16:25:32 +00:00
.github/workflows/ci.yml
emidoots (Migrated from github.com) commented 2022-07-24 16:25:32 +00:00
Copy link

https://github.com/hexops/mach/issues/431

https://github.com/hexops/mach/issues/431
emidoots (Migrated from github.com) reviewed 2022-07-24 16:27:26 +00:00
freetype/build.zig
@ -51,0 +52,4 @@
test_app.setBuildMode(mode);
test_app.setTarget(target);
link(b, test_app, .{ .freetype = .{ .brotli = true } });
test_app.install();
emidoots (Migrated from github.com) commented 2022-07-24 16:27:26 +00:00
Copy link

https://github.com/hexops/mach/issues/433

https://github.com/hexops/mach/issues/433
emidoots (Migrated from github.com) approved these changes 2022-07-24 16:28:44 +00:00
emidoots (Migrated from github.com) left a comment
Copy link

Filed issues for my outstanding concerns with this PR. Will merge now since there are good changes here otherwise.

Filed issues for my outstanding concerns with this PR. Will merge now since there are good changes here otherwise.
alichraghi commented 2022-07-24 16:34:09 +00:00 (Migrated from github.com)
Copy link

omg i had a lot of changes. forgot to mark as draft lol

omg i had a lot of changes. forgot to mark as draft lol
emidoots commented 2022-07-25 01:38:32 +00:00 (Migrated from github.com)
Copy link

ah gotcha, my bad

ah gotcha, my bad
❤️ 1
Sign in to join this conversation.
Reviewers
No reviewers
emidoots
Labels
Clear labels   
CI

   
all

   
basisu

   
blog

   
bug

Something isn't working

  
build

   
contributor-friendly

Good issue for contributors to take on

  
core

   
correctness

   
deferred

   
dev

   
direct3d-headers

   
docs

Improvements or additions to documentation

  
driver-os-issue

Issues that are very likely to be graphics driver / OS configuration issues

  
duplicate

This issue or pull request already exists

  
dxcompiler

   
editor

Graphical editor and Mach CLI

  
examples

   
experiment

   
feature-idea

New feature or request

  
feedback

   
flac

   
freetype

   
gamemode

   
gkurve

   
glfw

   
gpu

   
gpu-dawn

   
harfbuzz

   
help welcome

   
in-progress

   
infrastructure

   
invalid

This doesn't seem right

  
libmach

   
linux-audio-headers

   
long-term

   
mach

   
mach.gfx

   
mach.math

   
mach.physics

   
mach.testing

   
model3d

   
needs-triage

Issues that need triaging

  
object

object system

  
opengl-headers

   
opus

   
os/linux

   
os/macos

   
os/wasm

WebAssembly support

  
os/windows

   
package-manager

   
priority

   
proposal

   
proposal-accepted

   
question

Further information is requested

  
roadmap

   
slipped

   
stability

   
sysaudio

   
sysgpu

   
sysjs

   
validating-fix

   
vulkan-zig-generated

   
wayland-headers

   
website

   
wontfix

This will not be worked on

  
wrench

   
www

   
x11-headers

   
xcode-frameworks

   
zig-update
No labels
CI
all
basisu
blog
bug
build
contributor-friendly
core
correctness
deferred
dev
direct3d-headers
docs
driver-os-issue
duplicate
dxcompiler
editor
examples
experiment
feature-idea
feedback
flac
freetype
gamemode
gkurve
glfw
gpu
gpu-dawn
harfbuzz
help welcome
in-progress
infrastructure
invalid
libmach
linux-audio-headers
long-term
mach
mach.gfx
mach.math
mach.physics
mach.testing
model3d
needs-triage
object
opengl-headers
opus
os/linux
os/macos
os/wasm
os/windows
package-manager
priority
proposal
proposal-accepted
question
roadmap
slipped
stability
sysaudio
sysgpu
sysjs
validating-fix
vulkan-zig-generated
wayland-headers
website
wontfix
wrench
www
x11-headers
xcode-frameworks
zig-update
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
hexops/mach!429
No description provided.