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

bfs: new port #3421

Merged
merged 1 commit into from Jan 18, 2019
Merged

bfs: new port #3421

merged 1 commit into from Jan 18, 2019

Conversation

ylluminarious
Copy link
Contributor

Description

This adds a new port for the bfs program, a breadth-first version of the Unix find command.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.14.2 18C54
Xcode 10.1 10B61

Verification

Have you

  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Travis Build #4898 Passed.

Lint results
--->  Verifying Portfile for bfs
Warning: Line 27 should say "use_configure no" instead of declaring an empty configure phase
--->  0 errors and 1 warnings found.

Port bfs success on xcode9.4. Log
Port bfs success on xcode8.3. Log
Port bfs success on xcode7.3. Log

@ylluminarious
Copy link
Contributor Author

Oops, I accidentally meant to commit a change which changed configure {} to use_configure no. I just force-pushed it... hope it didn't cause any problems.

@macportsbot
Copy link

Travis Build #4899 Passed.

Lint results
--->  Verifying Portfile for bfs
--->  0 errors and 0 warnings found.

Port bfs success on xcode9.4. Log
Port bfs success on xcode8.3. Log
Port bfs success on xcode7.3. Log
Port bfs success on xcode10.1. Log

textproc/bfs/Portfile Outdated Show resolved Hide resolved
textproc/bfs/Portfile Outdated Show resolved Hide resolved
textproc/bfs/Portfile Outdated Show resolved Hide resolved
size 88162

worksrcdir ${name}-${version}
use_configure no
Copy link
Contributor

Choose a reason for hiding this comment

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

You are going to need to make sure that you are using the right compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mf2k I don't think this program cares very much about which compiler it uses. It's supposed to be portable, so the default one chosen by Macports should suffice.

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to at least make sure the universal variant works or not. Without the configure phase, manual changes, as mentioned in the wiki page, need to happen to make it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mf2k I am on Mojave, where the universal variant does not work and is no longer an option. Do I need to test it on another machine, then?

Copy link
Contributor

@mf2k mf2k Jan 14, 2019

Choose a reason for hiding this comment

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

Macports supports the current and prior 2 versions of macOS officially. So you need to account for it. You can disable the universal variant and say in a comment that it is not tested and likely will not work. Because it will not work without modification and no configure script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mf2k Ok, I added some code for the universal variant so that it can build a universal binary if chosen by the user. According to the docs: "The [universal] variant can be overridden if the default code does not work."

Selecting +universal does not work on Mojave; it fails with this error:

Error: bfs cannot be installed for the configured universal_archs 'x86_64 i386' because it only supports the arch(s) ''.

I'm inclined to believe this error occurred because Mojave does not seem to play nice with universal binaries anymore. I don't know whether this warrants a note or not.

However, selecting +universal does work on High Sierra. It installed successfully and the output of file /opt/local/bin/bfs is:

/opt/local/bin/bfs: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit executable x86_64] [i386:Mach-O executable i386]
/opt/local/bin/bfs (for architecture x86_64):	Mach-O 64-bit executable x86_64
/opt/local/bin/bfs (for architecture i386):	Mach-O executable i386

@ylluminarious
Copy link
Contributor Author

@mf2k I implemented the changes you requested, except for the one about using the right compiler. I don't think that's an issue in this case.

@macportsbot
Copy link

Travis Build #4902 Passed.

Lint results
--->  Verifying Portfile for bfs
--->  0 errors and 0 warnings found.

Port bfs success on xcode9.4. Log
Port bfs success on xcode8.3. Log
Port bfs success on xcode7.3. Log

textproc/bfs/Portfile Outdated Show resolved Hide resolved
@macportsbot
Copy link

Travis Build #4958 Passed.

Lint results
--->  Verifying Portfile for bfs
--->  0 errors and 0 warnings found.

Port bfs success on xcode9.4. Log
Port bfs success on xcode8.3. Log
Port bfs success on xcode7.3. Log
Port bfs success on xcode10.1. Log

@ylluminarious
Copy link
Contributor Author

@mf2k I went ahead and added some code for running the tests and I also added a build argument for setting CC. I basically just implemented @tavianator's suggestions from this comment. The aforementioned tests didn't all pass on my computer, but there were only 2 failures so I think it's negligible. Plus, tests aren't required to pass anyway, as I understand things.

So, all in all I think this is good to go, unless you have any other concerns.

@macportsbot
Copy link

Travis Build #4959 Passed.

Lint results
--->  Verifying Portfile for bfs
--->  0 errors and 0 warnings found.

Port bfs success on xcode9.4. Log
Port bfs success on xcode8.3. Log
Port bfs success on xcode7.3. Log
Port bfs success on xcode10.1. Log

@mf2k mf2k merged commit 350908a into macports:master Jan 18, 2019
@mf2k
Copy link
Contributor

mf2k commented Jan 18, 2019

Thanks!

@tavianator
Copy link

The aforementioned tests didn't all pass on my computer, but there were only 2 failures so I think it's negligible.

@ylluminarious Feel like opening a bfs issue with those failures?

@ylluminarious
Copy link
Contributor Author

@tavianator Strange, after I installed bfs via the default Macports rsync source (instead of the test source on my local filesystem), the tests actually pass now. So I guess it's a non-issue now.

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