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

pkg-diff.sh: Ignore R build timestamp & temp paths #34

Merged
merged 2 commits into from
Aug 14, 2019

Conversation

jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Aug 13, 2019

R DESCRIPTION contains the build timestamp, and
Meta/package.rds is a binary cache of DESCRIPTION.

R/*.rd[bx] are R cache files of the interpreted R code,
and they include temporary paths. Any change in the R caching
will occur in the R-base packages, and those changes already
trigger other changes in each built package which will ensure
rebuilds of these interpreted R caches.

@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 13, 2019

This change can be seen in action at https://build.opensuse.org/project/show/home:jayvdb:R , especially https://build.opensuse.org/package/live_build_log/home:jayvdb:R/R-sys/openSUSE_Tumbleweed/x86_64 where the rebuild was triggered by a change to the R-unix .spec file.

@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 13, 2019

The DESCRIPTION and Meta/package.rds items in that PR are self-explanatory.

grailbio/rules_r#21 (bazel build system) has some more information about the paths in the .rdb/.rdx files.

fwiw, I tried export R_INSTALL_STAGED=false as a way to remove the paths, but that had no effect.

If this approach for .rdb/.rdx isnt acceptable, we would need to follow the approach that bazel is using, which is much more complicated, and I believe not necessary for our situation.

@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 13, 2019

Most of the packages in my project are being marked unchanged when new builds are triggered.

Not all compiled R libs are working; some still have non-reproducible elements in them, so this doesnt work for all of them.

Two cases of it not working are R-markdown and R-rmarkdown and R-testthat; annoying because they are part of the "test the tester" build cycle. But the cycle is getting detected and found much sooner with this patch, because it is working correctly for other packages in that cycle, such as R-cli.

R DESCRIPTION contains the build timestamp, and
Meta/package.rds is a binary cache of DESCRIPTION.

R/*.rd[bx] are R cache files of the interpreted R code,
and they include temporary paths.  Any change in the R caching
will occur in the R-base packages, and those changes already
trigger other changes in each built package which will ensure
rebuilds of these interpreted R caches.
@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 14, 2019

I suspect the commonality of the ones which are rebuilding constantly is they depend on packages not in my project.
But initial attempts to prove that theory havent been successful.
When I build those packages locally, the second built RPMs pass the comparison with the old ones.

@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 14, 2019

ping @bmwiedemann @olafhering , would you be able to look at this. It is sort-of blocking 140 package updates, as they will be much less efficient if the SRs are accepted while this problem exists, as they introduce more loops as more and more crucial R packages are now inter-dependent.

@olafhering
Copy link
Collaborator

I think this is good. Please do a 'osc vc' to add a changes entry, refering to this PR.

@jayvdb
Copy link
Contributor Author

jayvdb commented Aug 14, 2019

Thx @olafhering - is ^ what you wanted?

@olafhering olafhering merged commit 4dfa207 into openSUSE:master Aug 14, 2019
@bmwiedemann
Copy link
Member

I found that with this patch, the only remaining diff is in R-base for files not covered by the path glob:
/usr/lib64/R/library/boot/help/boot.rdb
/usr/lib64/R/library/boot/help/boot.rdx
/usr/lib64/R/library/boot/help/paths.rds
/usr/lib64/R/library/class/help/class.rdb
/usr/lib64/R/library/class/help/class.rdx
/usr/lib64/R/library/class/help/paths.rds
/usr/lib64/R/library/cluster/help/cluster.rdb
/usr/lib64/R/library/cluster/help/cluster.rdx
/usr/lib64/R/library/cluster/help/paths.rds
/usr/lib64/R/library/codetools/help/codetools.rdb
/usr/lib64/R/library/codetools/help/codetools.rdx
/usr/lib64/R/library/codetools/help/paths.rds
/usr/lib64/R/doc/manual/fullrefman.pdf
/usr/lib64/R/library/foreign/help/foreign.rdb
/usr/lib64/R/library/foreign/help/foreign.rdx
/usr/lib64/R/library/foreign/help/paths.rds
/usr/lib64/R/library/KernSmooth/help/KernSmooth.rdb
/usr/lib64/R/library/KernSmooth/help/KernSmooth.rdx
/usr/lib64/R/library/KernSmooth/help/paths.rds
/usr/lib64/R/library/lattice/help/lattice.rdb
/usr/lib64/R/library/lattice/help/lattice.rdx
/usr/lib64/R/library/lattice/help/paths.rds
/usr/lib64/R/library/MASS/help/MASS.rdb
/usr/lib64/R/library/MASS/help/MASS.rdx
/usr/lib64/R/library/MASS/help/paths.rds
/usr/lib64/R/library/Matrix/help/Matrix.rdb
/usr/lib64/R/library/Matrix/help/Matrix.rdx
/usr/lib64/R/library/Matrix/help/paths.rds
/usr/lib64/R/library/mgcv/help/mgcv.rdb
/usr/lib64/R/library/mgcv/help/mgcv.rdx
/usr/lib64/R/library/mgcv/help/paths.rds
/usr/lib64/R/library/nlme/help/nlme.rdb
/usr/lib64/R/library/nlme/help/nlme.rdx
/usr/lib64/R/library/nlme/help/paths.rds
/usr/lib64/R/library/nnet/help/nnet.rdb
/usr/lib64/R/library/nnet/help/nnet.rdx
/usr/lib64/R/library/nnet/help/paths.rds
/usr/lib64/R/library/rpart/help/paths.rds
/usr/lib64/R/library/rpart/help/rpart.rdb
/usr/lib64/R/library/rpart/help/rpart.rdx
/usr/lib64/R/library/spatial/help/paths.rds
/usr/lib64/R/library/spatial/help/spatial.rdb
/usr/lib64/R/library/spatial/help/spatial.rdx
/usr/lib64/R/library/survival/help/paths.rds
/usr/lib64/R/library/survival/help/survival.rdb
/usr/lib64/R/library/survival/help/survival.rdx
/usr/lib64/R/library/tools/help/tools.rdb

Long term, it would be good to not ship such unreproducible cache files in rpms (because nobody can verify what is in there)
and to normalize timestamps with SOURCE_DATE_EPOCH

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

3 participants