conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
971 stars 1.79k forks source link

[question] Add rules to .gitattributes for other file extensions #7788

Open maksim-petukhov opened 3 years ago

maksim-petukhov commented 3 years ago

In my company we have a fork of CCI. I've tried to build libjpeg/9d on my company's CI and I'm getting different exported revision when building on Linux build agents (9dc889336bd64a534d502cd64bb3ecbe) and on Windows build agent (d77303a10a5ba2a0ab9e51536e5a0c9c). I think this is because git on Windows agents is configured with core.autocrlf=True and there is a .Mak file (and it is exported) in libjpeg package which is not included in CCI .gitattributes file. I've also scanned CCI repo file names recursively (except test_package folder) and found out that there is also .h , .c, .cpp files (e.g. fft package) and .bat, .cmd files. I think it makes sense to add all these extensions to .gitattributes.

Related #3948

SSE4 commented 3 years ago

I guess '.mak' or '.mk' extensions are not standard, and could be used by both GNU Makefiles and NMake Makefiles, as well as others. so it cannot be set as a generic rule in the root .gitattributes. I believe it should be just a specific rule for Win32.Mak within libjpeg directory, for this particular case. feel free to submit a PR adding Win32.Mak to .gitattributes.

for the general case, it's only important for text files exported via export_sources, which are usually patches or build system files. most of .bat, .cmd, .cpp, .c. and .h files are not exported, so they don't affect on recipe revision. there are very few exceptions, and for them, I suppose local .gitattributes are more than enough.

madebr commented 3 years ago

If conan is this sensitive to eol conventions, perhaps it needs a helper? This can be combined with something @SpaceIm started doing today in https://github.com/conan-io/conan-center-index/pull/7811:

    def export_sources(self):
        for patch in self.conan_data.get("patches", {}).get(self.version, []):
            self.copy(patch["patch_file"])

Perhaps we need to add something as follows?

    def export_sources(self):
        for patch in self.conan_data.get("patches", {}).get(self.version, []):
            self.copy(patch["patch_file"])
            tools.dos2unix(path=patch["patch_file"])

This way, we can decouple git settings of recipe revision hashes.

Artalus commented 11 months ago

libjpeg's issue with Win32.Mak file was still present at least at summer of 2023, when I encountered that problem in our fork of CCI.

Same problem was encountered recently with abseil here - there is a abi_trick/abi.h.in that is not EOL'ed by .gitattributes either.

Finally, I remember seeing something like that with grpc somewhere, because of its exported cmake/grpc_plugin_template.cmake.in.


I like that Conan API includes dos2unix, but using it to convert exports feels kinda fragile. Copying patches "manually" in def export_sources() already feels like unnecessary boilerplate to me; I am afraid that people would forget to put it into new recipes, the maintainers will forget to review it, and I am not sure if it can be ensured by any hooks to avoid any of this. I would definitely not mind though if exports() intestines would convert things automatically :slightly_smiling_face:

My suggestion would be to look at the .gitattributes provided by @samuel-emrys in https://github.com/conan-io/docs/issues/3415 , and attempt to enforce EOL on everything there is in the recipes/ directory, save for a few specific files/extensions where needed. Or maybe evaluate everything that is being exported alongside recipes, and bring it to a single common file extension, like *.in or *.template.


I am going to be a pain, and ping @Croydon and @jgsogo as well, since they were "in charge" of #3403 and #3468 originally and might have some thoughts on the subject too :slightly_smiling_face:

samuel-emrys commented 11 months ago

I've had good success with the .gitattributes linked above. The philosophy I've adopted is to make sure everything is LF, except those extensions which specifically require CRLF, such as .cmd, .bat and .PS1 files. These are also mostly fine with LF, but have some quirky edge cases that make CRLF a safer choice.