Skip to content

GH-45656: [C#] Fix failing MacOS builds#45734

Merged
CurtHagenlocher merged 1 commit intoapache:mainfrom
CurtHagenlocher:GH-45656
Mar 11, 2025
Merged

GH-45656: [C#] Fix failing MacOS builds#45734
CurtHagenlocher merged 1 commit intoapache:mainfrom
CurtHagenlocher:GH-45656

Conversation

@CurtHagenlocher
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher commented Mar 11, 2025

Rationale for this change

Fix MacOS builds by removing command-line sourcelink tool. This appears to be deprecated and the certificate is expired.

Are there any user-facing changes?

No.

Fix MacOS builds by removing command-line sourcelink tool. This appears to be deprecated and the certificate is expired.
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Wow! sourcelink is deprecated!?
Can we remove this

# Create a dummy .git/ directory to download the source files from GitHub with Source Link in C#.
dummy_git=${root_folder}/csharp/dummy.git
mkdir ${dummy_git}
pushd ${dummy_git}
echo ${release_hash} > HEAD
echo "[remote \"origin\"] url = https://github.com/${GITHUB_REPOSITORY:-apache/arrow}.git" >> config
mkdir objects refs
popd
workaround for sourcelink?

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Mar 11, 2025
@CurtHagenlocher
Copy link
Contributor Author

CurtHagenlocher commented Mar 11, 2025

+1

Wow! sourcelink is deprecated!? Can we remove this workaround for sourcelink?

Maybe?

It's the dotnet tool that's deprecated, at least according to its source site and the site for the package that replaces it. I don't know what this means for our release verification step, but a local build did seem to include all the right stuff in the form of the additional NuGet artifact and some bits in the PDB.

EDIT: This looks like it's for a source release. I don't know why the source link would be needed for a source release, as presumably someone who wants to build locally would not need to reference the GitHub sources. As I don't understand exactly what the script is supposed to achieve, I'm not going to remove those lines just yet as part of this PR.

@CurtHagenlocher CurtHagenlocher merged commit 19a67c7 into apache:main Mar 11, 2025
18 checks passed
@CurtHagenlocher CurtHagenlocher removed the awaiting merge Awaiting merge label Mar 11, 2025
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 19a67c7.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 15 possible false positives for unstable benchmarks that are known to sometimes produce them.

@kou
Copy link
Member

kou commented Mar 11, 2025

Ah, sourcelink command line is deprecated but Source Link itself is still used.

The csharp/dummy.git is used when we build .snuget (.pdb in .snuget) from source archive (not git clone-ed directory). Because https://github.com/dotnet/sourcelink refers .git/ to collect Git repository information. See also:

It seems that https://github.com/dotnet/sourcelink still does it. ( https://github.com/dotnet/sourcelink doesn't provide configuration items for repository name in GitHub such as apache/arrow.) So it seems that we can't remove the workaround...

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants