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.

Sign up for GitHub

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

Jump to bottom

Add Windows module #40

Closed
wants to merge 4 commits into from
Closed

Add Windows module #40

wants to merge 4 commits into from

Conversation

vtorri
Copy link
Contributor

@vtorri vtorri commented May 31, 2024

No description provided.

@vtorri
Copy link
Contributor Author

vtorri commented May 31, 2024

2 issues afaik :

  1. i can't add dependencies with make_custom_target()
  2. if I pass 2 .rc files to windows.compile_resources(), like windows.compile_resources([ 'foo1.rc', 'foo2.rc' ], etc..., only the last one is linked in the final executable

@vtorri vtorri mentioned this pull request Jun 4, 2024
Open
annacrombie
annacrombie requested changes Jun 18, 2024
View reviewed changes
src/functions/modules/windows.c Outdated Show resolved Hide resolved
src/functions/modules/windows.c Outdated Show resolved Hide resolved
src/functions/modules/windows.c
ctx->suffix = "o";
}
} else {
LOG_E("Could not find C or C++ compiler");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try to use vm_error whenever you can because it shows the user the source code associated with the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which node should I pass ?

src/functions/modules/windows.c
// then get rc compiler in WINDRES environment variable

if (!rc_str) {
rc_str = getenv("WINDRES");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't use getenv here. Call set_binary_from_env in options.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function is not public. Do I make it public ?

@vtorri vtorri closed this Jul 22, 2024
@annacrombie
Copy link
Collaborator

So I think this module is a good candidate for a script module. I'll probably attempt one eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annacrombie annacrombie annacrombie requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vtorri @annacrombie