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

docker: Add gfortran support #1458

Merged
merged 3 commits into from
May 1, 2024
Merged

docker: Add gfortran support #1458

merged 3 commits into from
May 1, 2024

Conversation

Dirreke
Copy link
Contributor

@Dirreke Dirreke commented Mar 13, 2024

Add gfortran support for all docker except target of *-none* and *-andriod*.

Close #1457

@Dirreke Dirreke requested a review from a team as a code owner March 13, 2024 15:03
Emilgardis
Emilgardis requested changes Mar 13, 2024
View reviewed changes
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

looks good so far!

One issue with this, it doesn't install it for the case where we use Dockerfile.native, which is used when doing cargo build-docker-image x86_64-unknown-linux-gnu --platform="linux/x86_64=x86_64-unknown-linux-gnu" (or just cargo build-docker-image x86_64-unknown-linux-gnu)

Easiest way to fix that would be to just install gfortran in linux-image.sh, perhaps add a install here:

cross/docker/linux-image.sh

Lines 420 to 423 in 085092c

# need to reinstall the removed libgcc packages, which are required for apt
if [[ "${arch}" == "${dpkg_arch}" ]]; then
apt-get install --no-install-recommends --assume-yes "${packages[@]}"
fi

docker/dragonfly.sh Outdated Show resolved Hide resolved
docker/Dockerfile.powerpc64-unknown-linux-gnu Outdated Show resolved Hide resolved
docker/Dockerfile.mips-unknown-linux-gnu Outdated Show resolved Hide resolved
@Emilgardis
Copy link
Member

/ci try

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 14, 2024

No idea about why x86_64-unknown-dragonfly and
x86_64-unknown-netbsd failed

@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 14, 2024

looks good so far!

One issue with this, it doesn't install it for the case where we use Dockerfile.native, which is used when doing cargo build-docker-image x86_64-unknown-linux-gnu --platform="linux/x86_64=x86_64-unknown-linux-gnu" (or just cargo build-docker-image x86_64-unknown-linux-gnu)

Easiest way to fix that would be to just install gfortran in linux-image.sh, perhaps add a install here:

cross/docker/linux-image.sh

Lines 420 to 423 in 085092c

# need to reinstall the removed libgcc packages, which are required for apt
if [[ "${arch}" == "${dpkg_arch}" ]]; then
apt-get install --no-install-recommends --assume-yes "${packages[@]}"
fi

It seems that common.sh will install it. and I don't need to add it to linux-image.sh

@Dirreke @Emilgardis
fix misspelling
758d7a4 
Co-authored-by: Emil Gardström <[email protected]>
@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 14, 2024

I can also add fortran test CI if needed.

@Emilgardis
Copy link
Member

/ci try

lets try again!

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 21, 2024 •
edited
Loading

This failure is related to libc/3618

It failed because of old_locale. I would patch it with gcc8/patches/patch-libgfortran_io_io.h.

Notice: I use the gcc8/patches/patch-libgfortran_io_io.h. It seems that every version of gcc after gcc8 has this patch except gcc9 and I don't know why. Maybe we can create the PR later.

Besides, we should undefine "HAVE_NEWLOCALE" and "HAVE_FREELOCALE", because there's no uselocale on netbsd(ref: gcc-11/libgfortran/io/io.h) and there's no ifdef to check this before gcc-11. According to this, I should set ac_cv_func_newlocale=no, ac_cv_func_freelocale=no, ac_cv_func_uselocale=no

Edit nothing about this target. And there's no log remained so I don't know why it failed.

@Dirreke
Copy link
Contributor Author

Dirreke commented Mar 21, 2024

Let's try x86_64-unknown-netbsd and aarch64-linux-android again. Thanks

@Emilgardis
Copy link
Member

/ci try

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@Dirreke
Copy link
Contributor Author

Dirreke commented Apr 3, 2024

Is any idea about why aarch64-linux-android failed? It seems that it just need more time to finish the build.

@Dirreke
Copy link
Contributor Author

Dirreke commented May 1, 2024

As libc/3618 has beed merged, let's try again @Emilgardis

@Emilgardis
Copy link
Member

/ci try

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link

github-actions bot commented May 1, 2024

Try run for comment

Failed Jobs

Successful Jobs

List

@Dirreke
Copy link
Contributor Author

Dirreke commented May 1, 2024

It doesn't seem to be my fault. Should we try again? Or could we merge it?

Emilgardis
Emilgardis approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

Lets merge this! Thank you!

@Emilgardis Emilgardis added this pull request to the merge queue May 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 1, 2024
@Dirreke
Copy link
Contributor Author

Dirreke commented May 1, 2024

timeout again. Could we adjust the settings of action?

@Emilgardis
Copy link
Member

Emilgardis commented May 1, 2024 •
edited
Loading

it should already be maxed out: https://github.com/cross-rs/cross/actions/runs/8907127039/workflow#L188 however I remember something about it not applying like you would expect. Lets just try again

@Emilgardis Emilgardis added this pull request to the merge queue May 1, 2024
Merged via the queue into cross-rs:main with commit 6ab0e7c May 1, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emilgardis Emilgardis Emilgardis approved these changes

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

Successfully merging this pull request may close these issues.

Could we add gfortran support to our images?
2 participants
@Dirreke @Emilgardis