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

Fix R 3.3.0-3.3.2 builds #179

Merged
merged 3 commits into from
Jul 20, 2023
Merged

Fix R 3.3.0-3.3.2 builds #179

merged 3 commits into from
Jul 20, 2023

Conversation

gaborcsardi
Copy link
Contributor

Closes #178.

@gaborcsardi gaborcsardi requested a review from glin July 19, 2023 09:09
Thanks .gitignore!
Copy link
Contributor

@glin glin left a comment

Choose a reason for hiding this comment

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

Nice! We've had this around forever and can finally remove the caveat note from the docs.

Just one more thing, handler.py will need to be updated to not skip these builds anymore:

r-builds/handler.py

Lines 149 to 154 in b541a8f

# In R 3.3.0, 3.3.1, and 3.3.2, the configure script check for the
# zlib version fails to handle versions longer than 5 characters.
# Skip builds affected by this bug. Most newer distros will be affected.
# https://github.com/rstudio/r-builds/issues/76
if version in ['3.3.0', '3.3.1', '3.3.2'] and platform not in ['centos-7']:
continue

This should also close #76 and #36

@@ -73,4 +73,5 @@ ENV R_RD4PDF="times,hyper"

COPY package.centos-7 /package.sh
COPY build.sh .
COPY patches /patches
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on previous notes, CentOS 7 was supposedly unaffected, and I do see an installer up at https://cdn.rstudio.com/r/centos-7/pkgs/R-3.3.0-1-1.x86_64.rpm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, OK. OTOH, it also does not hurt to do it for CentOS 7 as well, and then it is simpler if we do it everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that's a fine reason to keep it in then.

We have a patch now, so we can build these
R versions.
@gaborcsardi
Copy link
Contributor Author

OK, I updated handler.py hopefully it is OK now.

Let me know if you want to remove the patch from CentOS 7. I think it is better to keep it, one less special case. I did try the build with the patch on CentOS 7 (in the arm64 builds) and it works fine.

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.

Some builds are missing for Debian 12
2 participants