Skip to content

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Sep 25, 2018

Stack:
    :black_circle:  #12046 Don't record Git version automatically via cmake  💛

This /sounds/ like a good idea in theory, but a feature
like this must be implemented very carefully, because if
you just plop the Git version in a header (that is included
by every file in your project, as macros.h is), then every
time you do a 'git pull', you will do a FULL rebuild, because
macros.h is going to regenerate to a new version and of course
you have to rebuild a source file if a header file changes.

I don't have time to implement it correctly, so I'm axing
the feature instead. If you want git versions in, e.g.,
nightly builds, please explicitly specify that when you feed
in the version.

Differential Revision: D10030556

Differential Revision: D10030556
Differential Version: 58797606
zdevito pushed a commit to zdevito/ATen that referenced this pull request Sep 25, 2018
Summary:
Pull Request resolved: pytorch/pytorch#12046

This /sounds/ like a good idea in theory, but a feature
like this must be implemented very carefully, because if
you just plop the Git version in a header (that is included
by every file in your project, as macros.h is), then every
time you do a 'git pull', you will do a FULL rebuild, because
macros.h is going to regenerate to a new version and of course
you have to rebuild a source file if a header file changes.

I don't have time to implement it correctly, so I'm axing
the feature instead. If you want git versions in, e.g.,
nightly builds, please explicitly specify that when you feed
in the version.

Reviewed By: pjh5

Differential Revision: D10030556

fbshipit-source-id: 499d001c7b8ccd4ef15ce10dd6591c300c7df27d
@soumith soumith deleted the export-D10030556 branch February 21, 2019 12:10
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants