stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
140 stars 44 forks source link

Fix ci reply #1400

Closed serban-nicusor-toptal closed 7 months ago

serban-nicusor-toptal commented 8 months ago

Submission Checklist

Release notes

Fix CI reply functionality by removing the use of unstash for binaries builds. Made steps optional by adding additional params.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

serban-nicusor-toptal commented 8 months ago

Question @WardBrian should I replace the use of stash/unstash everywhere or leave it only for the binaries builds so they work on re-runs/custom runs?

Edit: Looks like it failed because of dune, hmmh, checking.

WardBrian commented 8 months ago

I think you should avoid running dune subst in the helper function, because on our mac builder we have a different switch name (eval $(opam env --switch=stanc-4.14.1 --set-switch))

WardBrian commented 8 months ago

I also only think it is important for the building binaries - the rest of the pipelines can still use stashing

WardBrian commented 7 months ago

@serban-nicusor-toptal can you add a dune subst step to each builder?

serban-nicusor-toptal commented 7 months ago

Oh my bad, I thought we didn't want it. Adding it back now per builder.

serban-nicusor-toptal commented 7 months ago

Removed the redundant ones! All binaries seem to build just fine https://jenkins.flatironinstitute.org/blue/organizations/jenkins/Stan%2FStanc3/detail/PR-1400/5/pipeline

WardBrian commented 7 months ago

Great!

Just to check the subst worked, can we add a stage which runs stanc --version for the built executables? At least on linux, probably not needed on all the builds

WardBrian commented 7 months ago

I realize I said stage, but it can also just be a step of the already existing build stage for linux

serban-nicusor-toptal commented 7 months ago

I think it's easier to check it as a stage https://jenkins.flatironinstitute.org/blue/organizations/jenkins/Stan%2FStanc3/detail/PR-1400/9/pipeline/473