Re: [PATCH v3 6/6] Makefile: add option to build and test libgit-rs and libgit-rs-sys

> On 2024-09-07 at 15:15:12, Sean Allred wrote:
> > Calvin Wan <calvinwan@google.com> writes:
> > > Add libgitrs, libgitrs-sys, libgitrs-test, and libgitrs-sys-test targets
> > > to their respective Makefiles so they can be built and tested without
> > > having to run cargo build/test.
> > 
> > I feel like clippy should be run as part of these somehow, but I'm not
> > sure where.
> 
> Yes, that seems like a good idea in CI.
> 
> > > +libgitrs-sys:
> > > +	$(QUIET)(\
> > > +		cd contrib/libgit-rs/libgit-sys && \
> > > +		cargo build \
> > > +	)
> > > +.PHONY: libgitrs
> > > +libgitrs:
> > > +	$(QUIET)(\
> > > +		cd contrib/libgit-rs && \
> > > +		cargo build \
> > > +	)
> > 
> > We should definitely be setting `RUSTFLAGS=-Dwarnings` as an analog to
> > `-Wall` in the C world, no? These crates should build without warnings.
> 
> I believe -Dwarnings turns warnings into errors (at least it does in my
> tests), which is equivalent to -Werror.  We don't want that because it
> breaks compiling older code with newer versions of the compiler, which
> makes it harder to bisect changes or compiler on the system compiler (or
> sometimes, other architectures or OSes).
> 
> That would be fine for clippy, however, because that would only run in
> CI, where we _would_ want to catch newer changes, but we want to
> compile nonetheless.
> -- 
> brian m. carlson (they/them or he/him)
> Toronto, Ontario, CA