GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix FreeBSD build failures. Update tests for FreeBSD by Thefrank · Pull Request #92 · PowerShell/PowerShell-Native

Conversation

@Thefrank

@Thefrank Thefrank commented Mar 2, 2023

Copy link
Copy Markdown
Contributor

Fix FreeBSD build failures. Update tests for FreeBSD

Changes

  • src/libpsl-native/CMakeLists.txt -> add CMake case for FreeBSD
  • src/libpsl-native/src/getcurrentthreadid.cpp -> add FreeBSD case
  • src/libpsl-native/src/getppid.cpp -> rework function, add case for FreeBSD
  • src/libpsl-native/src/getuserfrompid.cpp -> fix FreeBSD build failure from missing header
  • src/libpsl-native/test/ -> add FreeBSD to Apple cases. Apple and FreeBSD use the same parameters for stat

There are still two test failures related to GetDeviceID. FreeBSD returns a nonsensically high number for /usr/bin/stat -f %d /

Ubuntu 20.04:

frank@buildbot:~$ /usr/bin/stat -c %d /
64768

FreeBSD 13.1:

root@freebsd:~ # /usr/bin/stat -f %d /
2870232080986416670

Reasoning

Current FreeBSD builds fail. This PR resolves it and adds supporting tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Thefrank added 2 commits February 25, 2023 21:22
@Thefrank
FreeBSD build and test fixes
338782c
@Thefrank
Dont break Linux
ee1b194
@Thefrank

Thefrank commented Mar 3, 2023

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Thefrank Thefrank mentioned this pull request Apr 1, 2023
Support for FreeBSD dotnet/runtime#14537
Closed
This was referenced Jul 28, 2023
Add platform support for FreeBSD PowerShell/PowerShell#20041
Open
Add FreeBSD support pester/Pester#2385
Closed
@Thefrank

Thefrank commented Aug 5, 2023

Copy link
Copy Markdown
Contributor Author

Not sure who I should get code review from / who has write access so I am guessing based on recent PR merges

PTAL Aditya Patwardhan (@adityapatwardhan)
adityapatwardhan reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Thefrank

Thefrank commented Aug 20, 2023

Copy link
Copy Markdown
Contributor Author

If there is nothing else, then this is ready to merge
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@shanselman

Scott Hanselman (shanselman) commented Oct 11, 2023

Copy link
Copy Markdown

Steve Lee (@SteveL-MSFT) this is the last one I think that's needed for dotnet/runtime#14537 upstream
emaste, SchaichAlonso, ZeBobo5, and preetpalS reacted with thumbs up emoji sec reacted with rocket emoji
All reactions
  • 👍 4 reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
emaste
Ed Maste (emaste) reviewed Nov 16, 2023
View reviewed changes
Comment thread src/libpsl-native/CMakeLists.txt Outdated
elseif (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl")
elseif (${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl")

@emaste Ed Maste (emaste) Nov 16, 2023

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It seems strange to pass -Wl with no arg. Of course the oddity was just duplicated from the Darwin case (where it exists in the first public version of this file).

It seems to me we ought to just mimic the Linux behaviour here, e.g. make the existing case above if (${CMAKE_SYSTEM_NAME} MATCHES "Linux" OR ${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Thefrank Thefrank Nov 16, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I found it unusual too, but opted to include it as I assumed if it was important enough for "Darwin" to have it then so should "FreeBSD". I will go ahead and add "FreeBSD" to the "Linux" case as suggested. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/libpsl-native/src/getcurrentthreadid.cpp Outdated
tid = (pid_t)tid64;
#elif defined(__FreeBSD__)
tid = pthread_getthreadid_np();
return (int)tid;

@emaste Ed Maste (emaste) Nov 16, 2023

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This cast is superfluous - this function returns pid_t, which is the type of tid. Probably want in the line above tid = (pid_t)pthread_getthreadid_np();, or just avoid explicit casts altogether.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Thefrank Thefrank Nov 16, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This one...I tried my best here to follow what is done under dotNET for pthread_getthreadid_np() but found it inconsistent. I can make changes both to this PR and open PRs in runtime to correct the usage it in order to avoid any information loss due to casting.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/libpsl-native/src/getppid.cpp
pid_t GetPPid(pid_t pid)
{

#if defined (__APPLE__) && defined(__MACH__) || defined(__FreeBSD__)

@emaste Ed Maste (emaste) Nov 16, 2023

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

roll this commit into previous?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lukeb1961 and michael-o reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@Thefrank
Changes from review
96d7c69
emaste
Ed Maste (emaste) approved these changes Feb 2, 2024
View reviewed changes
@Thefrank Thefrank mentioned this pull request May 16, 2024
Powershell build ? sec/dotnet-core-freebsd-source-build#26
Open
@erwinlem erwinlem mentioned this pull request Jun 1, 2024
FreeBSD include fix #97
Merged
@Thefrank

Thefrank commented Jul 2, 2024

Copy link
Copy Markdown
Contributor Author

What else is needed to get this merged?

Any feedback would be great!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Thefrank Thefrank mentioned this pull request Jul 6, 2024
First attempt at running PowerShell under FreeBSD. freebsd/freebsd-ports#278
Closed
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Sep 4, 2024

Copy link
Copy Markdown
Member

/azp run
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@azure-pipelines

azure-pipelines Bot commented Sep 4, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
adityapatwardhan
Aditya Patwardhan (adityapatwardhan) approved these changes Sep 4, 2024
View reviewed changes
@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit 482c219 into PowerShell:master Sep 4, 2024
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Sep 4, 2024

Copy link
Copy Markdown
Member

Thefrank thank you for your contribution!
Thefrank reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Thefrank
Thefrank deleted the freebsd branch November 29, 2024 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
+1 more reviewer
@emaste Ed Maste (emaste) emaste approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Thefrank @shanselman @adityapatwardhan @emaste
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.