Skip to content

Commit

Permalink
fix: respect .gitignore in grafbase deploy
Browse files Browse the repository at this point in the history
A user was running into issues with `grafbase deploy` yesterday, where
deployments failed with a bizzare error very similar to [this one][1].

We eventually tracked this down to files in the users `.direnv` or `.devenv`
folders - based on the tar-rs issue _probably_ hard links of some description.
While it would be good to not choke on hardlinks, I'd argue we shouldn't have
been archiving these files in the first place: they're clearly meant to stay
local to the users machine.

So this PR switches `walkdir` for `ignore`, which will respect `.gitignore`
files etc, providing users with a sensible (and in most cases zero effort) way
to exclude files from the archive we build on `grafbase deploy`.  I think this
is a sensible default.

This _could_ have ramifications for anyone doing a `grafbase deploy` after some
kind of local build step, though I don't know how likely that is really.  I
suggest we try it and see - if anyone complains we can revisit, maybe adding a
CLI flag to change the behaviour.

[1]: alexcrichton/tar-rs#313
  • Loading branch information
obmarg committed Feb 29, 2024
1 parent 3b7fd6f commit c88123c
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 7 deletions.
43 changes: 41 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/crates/backend/Cargo.toml
Expand Up @@ -17,6 +17,7 @@ const_format = { version = "0.2", features = ["rust_1_64"] }
cynic = { workspace = true, features = ["http-reqwest"] }
dirs = "5"
http-cache-reqwest = "0.13"
ignore = "0.4"
reqwest = { workspace = true, features = [
"rustls-tls",
"stream",
Expand All @@ -35,7 +36,6 @@ tower-http = { workspace = true, features = ["trace"] }
ulid = "1"
url = "2"
urlencoding = "2"
walkdir = "2"

common = { package = "grafbase-local-common", path = "../common", version = "0.59.0" }
server = { package = "grafbase-local-server", path = "../server", version = "0.59.0" }
Expand Down
15 changes: 12 additions & 3 deletions cli/crates/backend/src/api/deploy.rs
Expand Up @@ -11,12 +11,12 @@ use common::consts::PROJECT_METADATA_FILE;
use common::environment::Project;
use cynic::http::ReqwestExt;
use cynic::{Id, MutationBuilder};
use ignore::DirEntry;
use reqwest::{header, Body, Client};
use std::ffi::OsStr;
use std::path::Path;
use tokio::fs::read_to_string;
use tokio_util::codec::{BytesCodec, FramedRead};
use walkdir::{DirEntry, WalkDir};

const ENTRY_BLACKLIST: [&str; 2] = ["node_modules", ".env"];

Expand Down Expand Up @@ -54,10 +54,19 @@ pub async fn deploy() -> Result<(), ApiError> {
.map_err(ApiError::AppendToArchive)?;
}

let walker = WalkDir::new(&project.path).min_depth(1).into_iter();
for entry in walker.filter_entry(|entry| should_traverse_entry(entry, &project.path)) {
let walker = ignore::WalkBuilder::new(&project.path)
.hidden(false)
.filter_entry(|entry| should_traverse_entry(entry, &project.path))
.build();

for entry in walker {
let entry = entry.map_err(ApiError::ReadProjectFile)?;

if entry.path() == project.path {
// walker always includes the root path in its traversal, so skip that
continue;
}

let entry_path = entry.path().to_owned();
let path_in_tar = entry_path.strip_prefix(&project.path).expect("must include prefix");
let entry_metadata = entry.metadata().map_err(ApiError::ReadProjectFile)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/crates/backend/src/api/errors.rs
Expand Up @@ -114,7 +114,7 @@ pub enum ApiError {

/// returned if a project file could not be read
#[error("could not read a project file\nCaused by: {0}")]
ReadProjectFile(walkdir::Error),
ReadProjectFile(ignore::Error),

/// returned if a project file could not be opened
#[error("could not open a project file\nCaused by: {0}")]
Expand Down

0 comments on commit c88123c

Please sign in to comment.