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

Include patch for SOURCE_DATE_EPOCH #69

Closed
wants to merge 1 commit into from
Closed

Include patch for SOURCE_DATE_EPOCH #69

wants to merge 1 commit into from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented Jun 17, 2024 •
edited
Loading

This adds a fix for SOURCE_DATE_EPOCH in static libraries on macOS. The fix was merged upstream: tpoechtrager/cctools-port#140

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • This recipe is using a compiler, which now requires adding a build dependence on {{ stdlib("c") }} as well. Note that this rule applies to each output of the recipe using a compiler. For further details, please see META: {{ stdlib("c") }} migration conda-forge.github.io#2102.
  • You're setting a constraint on the __osx virtual package directly; this should now be done by adding a build dependence on {{ stdlib("c") }}, and overriding c_stdlib_version in recipe/conda_build_config.yaml for the respective platform as necessary. For further details, please see META: {{ stdlib("c") }} migration conda-forge.github.io#2102.
  • In your conda_build_config.yaml, please change the name of MACOSX_DEPLOYMENT_TARGET, to c_stdlib_version!

@wolfv wolfv changed the title use latest upstream version use latest upstream commit Jun 17, 2024
h-vetinari
h-vetinari reviewed Jun 17, 2024
View reviewed changes
recipe/meta.yaml Outdated
Comment on lines 16 to 18
# latest commit from branch {{ cctools_version }}-ld64-{{ ld64_version }}
- url: https://github.com/tpoechtrager/cctools-port/archive/437ced391dbf14dce86f977ca050a750d5682f39.tar.gz
sha256: cf4020ddb92467de020c322eb13f0cac9ed992daa862a09bc2607e42467b3042
- url: https://github.com/tpoechtrager/cctools-port/archive/2a3e1c2a6ff54a30f898b70cfb9ba1692a55fad7.tar.gz
sha256: b1c1b148e14c3dacf99018a519506a26424c635d24ab6820c20b21e5ca22b8a3
Copy link
Member

@h-vetinari h-vetinari Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Please use the commit from https://github.com/tpoechtrager/cctools-port/tree/1009.2-ld64-907 and update the version. If you need a specific later commit, add it to the patches below

Copy link
Member Author

Choose a reason for hiding this comment

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

Are we good to update the version? Happy to do that - but don't know if there is a migration involved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since it turned out to be quite some more work to update to the latest version, I've added the patch and bumped the build number. Do you want to merge your stdlib fixes into this PR?

isuruf
isuruf reviewed Jun 17, 2024
View reviewed changes
recipe/patches/0005-use-SOURCE_DATE_EPOCH.patch
* things for exact binary equality.
*/
- if (getenv("ZERO_AR_DATE") == NULL)
+ if (getenv("ZERO_AR_DATE") != NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't ZERO_AR_DATE the way to go about this? The xcode linker also supports this option.

Copy link
Member Author

@wolfv wolfv Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

SOURCE_DATE_EPOCH is a standard and much nicer (proper, but deterministic timestamps vs. everything zero).

https://reproducible-builds.org/docs/source-date-epoch/

This is also already merged upstream as mentioned.

Copy link
Member Author

Choose a reason for hiding this comment

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

there are indeed a few additional places that should be adjusted though. We'll send another patch to the repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened an issue here: tpoechtrager/cctools-port#150

@wolfv wolfv changed the title use latest upstream commit Include patch for SOURCE_DATE_EPOCH Jun 17, 2024
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

That patch now is included through #70. I think we can close this PR?

@wolfv wolfv closed this Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isuruf isuruf isuruf left review comments

@h-vetinari h-vetinari h-vetinari left review comments

@davidbrochart davidbrochart Awaiting requested review from davidbrochart davidbrochart is a code owner

@katietz katietz Awaiting requested review from katietz katietz is a code owner

@mingwandroid mingwandroid Awaiting requested review from mingwandroid mingwandroid is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@wolfv @h-vetinari @isuruf