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

Semaphore CarbonCore Stubs #1468

Closed
wants to merge 0 commits into from
Closed

Conversation

Fancy2209
Copy link
Contributor

Not done yet

@Fancy2209
Copy link
Contributor Author

I had to do this because it wasn't compiling and I couldn't figure out why, so I hope gh Actions logs will make it easier to find out, unfortunately I can't test on a fork

@Fancy2209 Fancy2209 marked this pull request as ready for review January 13, 2024 15:26
Copy link
Contributor

@CuriousTommy CuriousTommy left a comment

Choose a reason for hiding this comment

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

LGTM. My only request is to combine all your commits into one and name it Implement Semaphore CarbonCore Stubs (or something similar to that)

@Fancy2209
Copy link
Contributor Author

Fancy2209 commented Jan 13, 2024

LGTM. My only request is to combine all your commits into one and name it Implement Semaphore CarbonCore Stubs (or something similar to that)

Can't you just Squash on Merge? I never learned how to merge commits and that should do the same

@Fancy2209
Copy link
Contributor Author

LGTM. My only request is to combine all your commits into one and name it Implement Semaphore CarbonCore Stubs (or something similar to that)

Figured it out

@Fancy2209
Copy link
Contributor Author

I am not sure if I am supposed to re ask for review every time, I am sorry if I am not

@CuriousTommy
Copy link
Contributor

Thanks for the PR! I'll merge it in after CICD reports a success.

Can't you just Squash on Merge? I never learned how to merge commits and that should do the same

I could... but it seems like a good learning experience if you learned how to do it 😉

I am not sure if I am supposed to re ask for review every time, I am sorry if I am not

No worries! You don't need to ask for a review every time.

@CuriousTommy
Copy link
Contributor

So it seems that the build failed, I want you to try and figure out what went wrong.


I had to do this because it wasn't compiling and I couldn't figure out why, so I hope gh Actions logs will make it easier to find out, unfortunately I can't test on a fork.

What I would recommend you do is clone the darling project as normal then add your fork as one of the remote repos.

# If you haven't cloned Darling already
git clone --recursive https://github.com/darlinghq/darling.git

# Then add your fork as one of the remotes
git remote add my-fork https://github.com/Fancy2209/darling.git

# Let GIT grab information about your fork.
git fetch my-fork

# Checkout the master branch from your fork (in the future, it's better to create a separate branch. Since there are two master branches, you will have to use a different name for your local branch...)
git checkout --b fork_master --track my-fork/master

The following instructions below are about submodules, but the same idea applies to the darling repo:
https://docs.darlinghq.org/contributing/index.html

@Fancy2209
Copy link
Contributor Author

git clone --recursive https://github.com/darlinghq/darling.git

I think it's easier if I just download the patch file and apply
https://github.com/darlinghq/darling/pull/1468.patch
Will try to bisect this though, thanks

@Fancy2209
Copy link
Contributor Author

2024-01-13T20:59:55.7792285Z /home/runner/work/darling/darling/src/frameworks/CoreServices/src/CarbonCore/Multiprocessing.cpp:28:15: error: use of undeclared identifier 'getenv'
2024-01-13T20:59:55.7794502Z     verbose = getenv("STUB_VERBOSE") != NULL;
2024-01-13T20:59:55.7795594Z               ^
2024-01-13T20:59:55.7817565Z /home/runner/work/darling/darling/src/frameworks/CoreServices/src/CarbonCore/Multiprocessing.cpp:120:18: error: use of undeclared identifier 'puts'
2024-01-13T20:59:55.7819468Z     if (verbose) puts("STUB: MPCreateSemaphore called");
2024-01-13T20:59:55.7820882Z                  ^
2024-01-13T20:59:55.7828563Z /home/runner/work/darling/darling/src/frameworks/CoreServices/src/CarbonCore/Multiprocessing.cpp:125:18: error: use of undeclared identifier 'puts'
2024-01-13T20:59:55.7830465Z     if (verbose) puts("STUB: MPDeleteSemaphore called");
2024-01-13T20:59:55.7832398Z                  ^
2024-01-13T20:59:55.7840004Z /home/runner/work/darling/darling/src/frameworks/CoreServices/src/CarbonCore/Multiprocessing.cpp:130:18: error: use of undeclared identifier 'puts'
2024-01-13T20:59:55.7841889Z     if (verbose) puts("STUB: MPSignalSemaphore called");
2024-01-13T20:59:55.7843597Z                  ^
2024-01-13T20:59:55.7851137Z /home/runner/work/darling/darling/src/frameworks/CoreServices/src/CarbonCore/Multiprocessing.cpp:135:18: error: use of undeclared identifier 'puts'
2024-01-13T20:59:55.7853021Z     if (verbose) puts("STUB: MPWaitOnSemaphore called");
2024-01-13T20:59:55.7854928Z                  ^
2024-01-13T20:59:55.7859919Z 5 errors generated.

aha

@Fancy2209
Copy link
Contributor Author

Oh great now I screwed something up

@Fancy2209
Copy link
Contributor Author

I'll redo this cleanly

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

Successfully merging this pull request may close these issues.

2 participants