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

V1.76 #7

Merged
merged 28 commits into from
Mar 18, 2024
Merged

V1.76 #7

merged 28 commits into from
Mar 18, 2024

Conversation

SC426
Copy link

@SC426 SC426 commented Mar 7, 2024

rust 1.76.0

Destination channel: defaults

Links

Explanation of changes:

  • Update version and sha256
  • Linter fixes
  • Update tests
  • include forge_test.sh
  • Change clang deployment target version for osx-64 to 10.12
  • missing_dso relocated due to conda-build handling

@SC426 SC426 changed the title V1.76 [Skip CI] V1.76 Mar 7, 2024
@SC426 SC426 closed this Mar 7, 2024
@SC426 SC426 reopened this Mar 7, 2024
Copy link

@cbouss cbouss left a comment

Choose a reason for hiding this comment

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

The PR looks okay. From the release note, I am not sure whether we are affected by this point: rust-lang/rust#117947

Have you tried building packages with it? What is the outcome?

@danpetry
Copy link

danpetry commented Mar 8, 2024

I agree, for compilers it's useful to build some other packages with them to validate them. the testing in the feedstocks here is quite basic too, you might want to include some of the tests from conda-forge's feedstock

@SC426
Copy link
Author

SC426 commented Mar 8, 2024

@danpetry Let me know if this is more along the lines of what you were thinking.

@JeanChristopheMorinPerso

@cbouss @danpetry doesn't rustc come with its own copy of llvm? Doing a quick search, I'm not too sure what they mean by "external LLVM"...

@cbouss
Copy link

cbouss commented Mar 8, 2024

@cbouss @danpetry doesn't rustc come with its own copy of llvm? Doing a quick search, I'm not too sure what they mean by "external LLVM"...

I think so. That you have the option to build rust from source and use your own llvm. So it shouldn't affect us. But it's still good to verify the compiler is actually working.

@SC426 SC426 marked this pull request as ready for review March 13, 2024 23:15
Copy link

@cbouss cbouss left a comment

Choose a reason for hiding this comment

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

LGTM

@skupr-anaconda
Copy link

recipe/conda_build_config.yaml Show resolved Hide resolved
- /usr/lib/libc .1.dylib
- /usr/lib/libc abi.dylib
- /usr/lib/libiconv.2.dylib
- /usr/lib/libcurl.4.dylib

Choose a reason for hiding this comment

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

curl is needed on unix, see https://github.com/rust-lang/rust/blob/master/INSTALL.md#dependencies
Should we add it to host?

Copy link

Choose a reason for hiding this comment

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

The build is installing but not actually building, so we are fine here.

recipe/meta.yaml Show resolved Hide resolved
- /usr/lib/libresolv.9.dylib
- /usr/lib/libc .1.dylib
- /usr/lib/libc abi.dylib
- /usr/lib/libiconv.2.dylib

Choose a reason for hiding this comment

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

libiconv is required on linux but here is only osx. Hmm https://github.com/rust-lang/rust/blob/master/INSTALL.md#dependencies

Comment on lines 49 to 56
- /System/Library/Frameworks/Python.framework/Versions/2.7/Python
- /System/Library/PrivateFrameworks/DebugSymbols.framework/Versions/A/DebugSymbols
- /usr/lib/libcompression.dylib
- /usr/lib/libedit.3.dylib
- /usr/lib/libform.5.4.dylib
- /usr/lib/libncurses.5.4.dylib
- /usr/lib/libpanel.5.4.dylib
- /usr/lib/libxml2.2.dylib

Choose a reason for hiding this comment

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

Should all .dylib have the selector # [osx]?

Copy link

Choose a reason for hiding this comment

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

we could, but that won't change the end result

- '**/ld-linux-x86-64.so.2'
- '**/libc.so.6'
- '**/libdl.so.2'
- '**/ld64.so.*'

Choose a reason for hiding this comment

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

Is this a known defect of s390x only like $RPATH/ld64.so.1?

@danpetry
Copy link

danpetry commented Mar 14, 2024

As noted above, I'd like to see some stuff compiled with this, in conjunction with the activation feedstock ideally

requirements:
build:
- {{ compiler('c') }} # [osx]
- posix # [win]
test:
requires:
- {{ compiler('c') }}

Choose a reason for hiding this comment

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

what is a c compiler necessary for?

Copy link

Choose a reason for hiding this comment

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

To run forge_test.sh, a backend compiler is required.
Usually this is brought in by the activation feedstock, but here we are one step early, so we need to include it in test/requires.

@SC426 SC426 merged commit 45ada94 into master Mar 18, 2024
8 checks passed
@SC426 SC426 deleted the v1.76 branch March 18, 2024 19:51
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.

None yet

6 participants