Skip to content

Conversation

@staticfloat
Copy link
Member

Without this change, suitesparse checksum files don't get packed
properly

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

This needs a comment on why you are re-invoking make, which is non-typical

@DilumAluthge
Copy link
Member

DilumAluthge commented May 25, 2021

Should we just rename the checksums file from suitesparse to libsuitesparse? Would that avoid the need for re-invoking make?

@staticfloat
Copy link
Member Author

which is non-typical

Yeah, I'm not totally happy with it either. I want to cause invocations of pack-checksum-libsuitesparse to actually invoke pack-checksum-suitesparse, since we implicitly assume that the name of the target (-suitesparse) has something to do with the name of the files on disk (SuiteSparse*.tar.gz). I don't know how to enforce that without shelling out to make again.

@vtjnash
Copy link
Member

vtjnash commented May 25, 2021

Why not a dependency then?

@staticfloat
Copy link
Member Author

If I just put

pack-checksum-libsuitesparse: pack-checksum-suitesparse

Then we still generate an empty libsuitesparse file, because we still run the pattern rule below. What I really want is an "alias rule", if such a thing exists, and the $(MAKE) invocation seemed the simplest way to do it.

@staticfloat
Copy link
Member Author

Okay with the latest push, the only downside now is that we clear out the SuiteSparse-${gitsha}.tar.gz checksum file that the stdlib saves.

@ViralBShah ViralBShah added building Build system, or building Julia or its dependencies sparse Sparse arrays labels May 26, 2021
@ViralBShah
Copy link
Member

I'm not sure I follow this fully, but will we now have deps/checksums/libsuitesparse for all the checksums to be consistent with all the changes? I guess deleting the tarball is ok - it will just trigger an extra download, right?

@ViralBShah ViralBShah force-pushed the sf/suitesparse_checksum_refresh branch from 67a498f to f2ae66a Compare May 29, 2021 00:07
@vtjnash vtjnash merged commit a5889a8 into master May 29, 2021
@vtjnash vtjnash deleted the sf/suitesparse_checksum_refresh branch May 29, 2021 03:47
@ViralBShah
Copy link
Member

I ran into this while updating suitesparse 5.10.1 checksums. This is likely going to lead to accidental deletion - but presumably only folks dealing with suitesparse will be updating this and can watch out for it.

shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
Without this change, suitesparse checksum files don't get packed
properly.
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Without this change, suitesparse checksum files don't get packed
properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

building Build system, or building Julia or its dependencies sparse Sparse arrays

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants