-
-
Notifications
You must be signed in to change notification settings - Fork 479
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
Copy over pinnings from the webpage script #7
Copy over pinnings from the webpage script #7
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
bzip2: | ||
- 1.0 | ||
cairo: | ||
- 1.14 | ||
curl: | ||
- 7.54 | ||
- 7.44 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to have an older pinning of curl
.
bzip2: | ||
- 1.0 | ||
cairo: | ||
- 1.14 | ||
curl: | ||
- 7.54 | ||
- 7.44 | ||
dbus: | ||
- 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was not present in pin_the_slow_way.py
. Given it seems pretty stable and has been at 1 a long time, I don't know if we need this, but I guess it can't hurt.
@@ -70,27 +72,29 @@ flann: | |||
fontconfig: | |||
- 2.12 | |||
freetype: | |||
- 2.8 # [not ppc64le] | |||
- 2.8.1 # [not ppc64le] | |||
gstreamer: | |||
- 1.12 | |||
gst_plugins_base: | |||
- 1.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were missing from pin_the_slow_way.py
. Though it is good to see them here.
geos: | ||
- 3.6.2 | ||
giflib: | ||
- 5.1 | ||
glib: | ||
- 2.53 | ||
- 2.55 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using glib 2.55
currently. Switching to 2.53 will undoubtedly break things based on various symbols added and one removed over time.
glpk: | ||
- 4.62 | ||
- 4.61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reading of the API/ABI report for glpk is that 4.61 and 4.62 are compatible, but 4.61 added one symbol after 4.60. Vaguely remember talking to @dougalsutherland about this some time ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that's right. We should maybe just update to the new 4.65 (which has new symbols), though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add that as a new issue? Would merely like to make sure we have copied everything over ok first. We can update them in a second pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened as issue ( #13 ).
libpng: | ||
- 1.6 | ||
- 1.6.32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a rather strange constraint. Probably mostly based on our desire to use the same pinnings in build
and run
. This was the lower end of that constraint. So think this should be ok going forward, but we may need to take the high end of the constraint ( 1.6.34
) if we want to be really conservative.
librdkafka: | ||
- 0.9.5 | ||
- 0.9.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same story as glpk
. Both 0.9.4
and 0.9.5
appear to be API/ABI compatible based on the report. So picked the low end of what we had in pin_the_slow_way.py
.
libssh2: | ||
- 1.8 | ||
libsvm: | ||
- 3.21 | ||
libtiff: | ||
- 4 | ||
- 4.0.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like libpng
, there was a rather strange constraint. Probably mostly based on our desire to use the same pinnings in build
and run
. This was the lower end of that constraint. So think this should be ok going forward, but we may need to take the high end of the constraint ( 4.0.9
) if we want to be really conservative. The latter matches defaults
based on our last annotation in pin_the_slow_way.py
.
r_base: | ||
- 3.3.2 | ||
- 3.4.1 | ||
scipy: | ||
- 0.19 | ||
snappy: | ||
- 1 | ||
- 1.1.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Historically snappy
is not that stable. At 1.1.5
symbols were added and removed plus the SONAME changed. Though 1.1.6 and 1.1.7 seem to be ok. Picked the lower end of what we had before again.
sox: | ||
- 14.4 | ||
- 14.4.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sox
is really unstable even between patch versions. So this needs to be tight. Again same as what we had.
sqlite: | ||
- 3 | ||
- 3.20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is our pinning, but not what we are using. Please see issue ( conda-forge/sqlite-feedstock#25 ) for the publication issue. We may need to go to 3.21.0
to fix it.
recipe/conda_build_config.yaml
Outdated
xerces_c: | ||
- 3 | ||
- 0.9.20 | ||
xerces-c: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if having a -
in the name is problematic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is. _
and -
are equivalent in the config file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean _
in the config file and package name with -
are equivalent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
recipe/conda_build_config.yaml
Outdated
- 3 | ||
- 0.9.20 | ||
xerces-c: | ||
- 3.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems really tight, but that is what we and defaults have apparently. Did not find API/ABI info, but did request it in issue ( lvc/upstream-tracker#29 ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the newly added API/ABI report. Probably 3.2 is sufficient, but would be good to hear from xerces-c
users. @conda-forge/xerces-c?
recipe/conda_build_config.yaml
Outdated
zeromq: | ||
- 4.2 | ||
zlib: | ||
- 1.2 | ||
- 1.2.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zlib
is kind of a mess. Pinning exactly seems safest.
There's some other stuff I skipped like |
2966f09
to
88128ec
Compare
This is a mostly accurate copy of the content from the webpage pinning script to `conda-forge-pinning`.
88128ec
to
2dce8fb
Compare
This copies over everything in cc @conda-forge/core |
Note that this alone will not be enough. All these packages need to rebuilt with |
recipe/conda_build_config.yaml
Outdated
gmp: | ||
- 6 | ||
- 6.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6
is enough. Minor versions add symbols, but doesn't remove them. So, run_exports
with a minor version as the min would fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done.
@@ -167,7 +173,7 @@ pixman: | |||
poppler: | |||
- 0.61 | |||
proj4: | |||
- 4 | |||
- 4.9.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has changed SONAME in patch releases as noted in the ABI/API report. Seems good to keep this locked down pretty tight.
@@ -167,7 +173,7 @@ pixman: | |||
poppler: | |||
- 0.61 | |||
proj4: | |||
- 4 | |||
- 4.9.3 | |||
protobuf: | |||
- 3.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should bump this to 3.5
. Also planning on making this libprotobuf
as that will be the C++ only package. Will need to keep it at 3.5
for VS 2008 support as it is the last non-C++11 release, but will be a LTS release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened as issue ( #14 ).
zlib: | ||
- 1.2 | ||
- 1.2.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will being more specific here break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the current zlib
pinning in the webpage repo. So no it shouldn't.
Some packages were implicitly using this version when they had the zlib 1.2.*
pinning. In those cases, this pinning should fix breakage. Though ultimately those packages need to be rebuilt with a tighter pinning.
Also please see my old comment for details about ABI breakage between patch versions, which motivates this tight pinning.
Anything else here or is this good to go? |
recipe/conda_build_config.yaml
Outdated
@@ -138,7 +144,7 @@ metis: | |||
mkl: | |||
- 2018 | |||
mpfr: | |||
- 3 | |||
- 3.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3
is enough when use run_exports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done.
glpk: | ||
- 4.62 | ||
- 4.61 | ||
gmp: | ||
- 6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pin_the_slow_way
had 6.1.*
; note that 6.1 added symbols. Maybe run_exports
is enough though, I haven't really looked into how all of that works yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see this (outdated) comment. FWIU run_exports
lower bounds the version based on the one used for the build. More details in this doc.
cc @isuruf
ENH patch libgfortran for v4
This is a mostly accurate copy of the content from the webpage pinning script to
conda-forge-pinning
.ref: https://github.com/conda-forge/conda-forge.github.io/blob/61cdd7fd0374c07e95afdcbd414c7985b141e4b3/scripts/pin_the_slow_way.py