rover
rover copied to clipboard
apollographql
fix(build.rs) minor logic error
I was trying to build rover in nixpkgs, where we prefetch the schema and hash.id since otherwise the build would fail. I noticed though even though the file existed it would still try and download it, and would fail as expected.
It seems to be || is the wrong operator here - if the file exists already, it'll check if you're online.
@edude03: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/
Hmm I kind of get it - essentially you're trying to always have the latest schema as of the build time.
That said I'm not convinced that's ideal anyway since it introduces non determinism to the build process. I know nix is kind of esoteric but I think it's important that there is a way to ensure the build is reproducible other than turning off your internet during the build 😅
If you don't want this change what would be a better solution? Maybe a flag or environment variable to skip the online check and always use the schema and etag from disk?
Ideally I'd actually like to split this - have an update script that always fetches and a build script that uses the provided files but I recognize that might add unwanted complexity to the build process
Either way I'm happy to work with you on whatever solution you think would be appropriate
Yeah, this non-determinism is probably not ideal. We've also historically excluded the schema from git history (because of unsightly diffs) and uploaded them separately as github release artifacts to be able to reproduce old builds.
Thinking about this - I think you're right that a script to update this would be better. The schema fetch step should be added to the existing cargo xtask prep command that is run to generate a release commit. That way the schema is always updated before a release (since we already run that command before a release), and you can manually update the schema by running that command locally (there's no harm to running that command even if you're not preparing a release).
Additionally, we should probably just start committing the schema to version control as binary files so the diffs don't show up and the build script will always have something to read even without fetching an updated schema.
@edude03 - I've opened this issue outlining what needs to be done to address this. Not sure when we'll get around to it, but you're welcome to open a PR if that's interesting to you!