Bogdanp / setup-racket

A GH action for installing Racket.
MIT License
49 stars 9 forks source link

core: add snapshot-site arg to support the northwestern snap site #64

Closed Bogdanp closed 1 year ago

Bogdanp commented 1 year ago

@rfindler would you be able to add symlinks for BC Linux snapshots w/o the Linux distro in the name?

These BC URLs currently fail:

But these CS URLs work fine:

Also, this isn't required, but would be nice: the Utah snapshots don't have -test- in the filenames for the full distribution installers. It would be great if the Northwestern site could have symlinks w/o that bit in the name, so I can avoid adding this bit of logic.

rfindler commented 1 year ago

Thanks for looking into this! One idea I had after opening the issue was to have your CI code check to see which snapshot build has completed more recently and use that (by default; letting people opt in to sticking with a specific snapshot site if they need to). Generally speaking, I think that both sites have gotten pretty reliable, but when one of them has trouble, the other one tends to be working.


Also, this isn't required, but would be nice: the Utah snapshots don't have -test- in the filenames for the full distribution installers. It would be great if the Northwestern site could have symlinks w/o that bit in the name, so I can avoid adding this bit of logic.

That's because the distributions are different; the Northwestern ones include main-distribution-test (and everything that depends on). I'm open to various ideas you have for resolving this. We could ask @mflatt to include main-distribution-test or I can stop including it (probably I'd need to set up more distros to do that). Probably we do want distributions to be the same so that people who run CI actions know what they're getting, I suppose. Currently the northwestern snapshots include these packages: '("main-distribution" "main-distribution-test" "optimization-coach" "games")


These BC URLs currently fail:

* https://plt.cs.northwestern.edu/snapshots/current/installers/racket-test-current-x86_64-linux-bc.sh

* https://plt.cs.northwestern.edu/snapshots/current/installers/racket-minimal-current-x86_64-linux-bc.sh

But these CS URLs work fine:

* https://plt.cs.northwestern.edu/snapshots/current/installers/racket-test-current-x86_64-linux-cs.sh

* https://plt.cs.northwestern.edu/snapshots/current/installers/racket-minimal-current-x86_64-linux-cs.sh

This is more complicated; the code is using the #:dist-aliases argument. I don't see any difference in the configuration for the bc and cs, however, so I'm not understanding why we see any difference between these two. In particular, for the ubuntu builds, this argument is used both for bc and cs:

     #:dist-aliases (append (list (list #f #f "")
                                  (list #f "" #f)) ;; for the cross build
                            extra-aliases)

The extra-aliases is the empty list when building the racket-test distros and is (list (list "min-racket" #f #f)) when building the racket-minimal distros, so that doesn't seem relevant. If I'm reading the documentation correctly, the first element of the list, namely (list #f #f "") means that we'll drop the cs and bc from the name so that's not helpful. The second list, namely (list #f "" #f) means that we'll drop the "dist suffix" part of the name to make an alias, which is "precise" so we should get aliases like you're asking for from that, but the problem is that they'll have version numbers in them, not current in them because, as the docs say, only the first element of the list is used to make current links.

But I must be getting something wrong in my logic here because I see that replacing current in your desired urls above with the current version also has a cs link but not a bc link.

I'll have to try poking at and thinking about this more to figure out what's going on.

mflatt commented 1 year ago

At the Northwestern site, the Ubuntu builds are 64-bit CS and 32-bit BC. The 64-bit BC build is on Debian, where (list #f "" #f) is not included in #:dist-aliases. (Note that you don't want to include the other one there, (list #f #f ""), because you want the result name to refer to the Ubuntu build).

Clarification on the current links: special treatment of the first alias is about which one is linked from "as current" on the web page, but current file names are created for all aliases. I'll adjust the docs to make that more clear.

rfindler commented 1 year ago

🤦 Thanks for getting this sorted out. I was really barking up the wrong tree!

Okay, so I'll try adding a 64bit BC build and also adding a set of builds that don't have main-distribution-test (or the other pkgs), but instead just main-distribution, hopefully with the same names as on the utah site.

Bogdanp commented 1 year ago

Thanks for looking into this! One idea I had after opening the issue was to have your CI code check to see which snapshot build has completed more recently and use that (by default; letting people opt in to sticking with a specific snapshot site if they need to). Generally speaking, I think that both sites have gotten pretty reliable, but when one of them has trouble, the other one tends to be working.

I think this is a good idea, but I'll need some more help from the snapshot sites to implement the recency check. Ideally, there would be a structured file (JSON would be easiest for this action) with metadata about the build so I have something to compare between the two. For now, I've changed snapshot_site to default to auto, and it will select the first live snapshot site, preferring Utah.

rfindler commented 1 year ago

I think this is a good idea, but I'll need some more help from the snapshot sites to implement the recency check.

How about comparing the contents of this file: https://plt.cs.northwestern.edu/snapshots/current/stamp.txt to https://users.cs.utah.edu/plt/snapshots/current/stamp.txt ?

Bogdanp commented 1 year ago

That works! I've pushed a change to use that file. I think once the build changes are ready on the Northwestern side, this is good to go.

rfindler commented 1 year ago

I think once the build changes are ready on the Northwestern side, this is good to go.

The first attempt at the build with the new configuration completed and I got the configuration wrong, sadly. I think I see what I did wrong and I've fixed the configuration; hopefully the next time we'll get all the builds we need.

rfindler commented 1 year ago

It looks like the build completed successfully this time. There are various aliases too, but here are the names of the builds that I think that the CI wants to use:

racket-current-x86_64-linux-bc.sh
racket-current-x86_64-linux-cs.sh
racket-current-i386-linux-bc.sh

racket-minimal-current-i386-linux-bc.sh
racket-minimal-current-x86_64-linux-bc.sh
racket-minimal-current-x86_64-linux-cs.sh

Does that look right? Am I missing any?


Also, it occurs to me that sometimes, when builds fail, the relevant file just won't be there, even though the rest of the site will be. So it probably makes sense to use the stamp.txt file to disambiguate between builds specifically when the file exists on both sites. At least at northwestern, from time to time, one of the VMs acts up and needs to be rebooted; the rest of the build might complete fine in that situation and there might be a stamp.txt file generated, but some specific files just won't be there (this happened with the debian builds at northwestern last night).

Bogdanp commented 1 year ago

Does that look right? Am I missing any?

It looks like these are still missing:

Also, it occurs to me that sometimes, when builds fail, the relevant file just won't be there, even though the rest of the site will be. So it probably makes sense to use the stamp.txt file to disambiguate between builds specifically when the file exists on both sites. At least at northwestern, from time to time, one of the VMs acts up and needs to be rebooted; the rest of the build might complete fine in that situation and there might be a stamp.txt file generated, but some specific files just won't be there (this happened with the debian builds at northwestern last night).

That makes sense. I'll update the code to issue head requests for the files in addition to the stamps.

rfindler commented 1 year ago

Okay, I've added those four builds yesterday, but they won't appear until the current run finishes and a new build starts (assuming I didnt mess up something when editing the configuration again!).

But it occurs to me that, with the checks for the existence of the files that you're doing, it might be okay not to wait for the snapshot builds to all finish?

rfindler commented 1 year ago

Okay, it looks like things worked and we've got those four builds on the northwestern snapshot site now.

Bogdanp commented 1 year ago

Yup! Everything looks good now. Thanks for all your help and suggestions!

rfindler commented 1 year ago

Thank you for making these changes! Hopefully the northwestern site will be more useful to folks and and CI will have a bit more redundancy.

On Wed, Mar 15, 2023 at 3:29 PM Bogdan Popa @.***> wrote:

Yup! Everything looks good now. Thanks for all your help and suggestions!

— Reply to this email directly, view it on GitHub https://github.com/Bogdanp/setup-racket/pull/64#issuecomment-1470796382, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBNMGY3O6PD5O7UOQHTQTW4IRDZANCNFSM6AAAAAAVTMK2OA . You are receiving this because you were mentioned.Message ID: @.***>