Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Remove transient directories after install to allow for rebuild #311

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Sep 11, 2024

While doing multiple rebuilds it is advisable to also clean up the state of the Git submodules with git clean -f, removing the OUTPUT directory at src/fastjet/_fastjet_core/ and the top level build directory is sufficient to do local development rebuilds — i.e., the command python -m pip install --upgrade . can be run twice in a row without error.

@matthewfeickert
Copy link
Member Author

matthewfeickert commented Sep 11, 2024

This is ready for review. The CI build wheels jobs are broken in general given

fastjet/pyproject.toml

Lines 16 to 23 in f90dc72

[tool.cibuildwheel.linux]
before-all = [
"yum update -y",
"yum install -y mpfr-devel",
"curl -L https://boostorg.jfrog.io/artifactory/main/release/1.84.0/source/boost_1_84_0.tar.bz2 -o boost_1_84_0.tar.bz2",
"tar --bzip2 -xf boost_1_84_0.tar.bz2",
"mv boost_1_84_0/boost /usr/include/boost",
]

is hitting

Error: Command ['sh', '-c', 'yum update -y && yum install -y mpfr-devel && curl -L https://boostorg.jfrog.io/artifactory/main/release/1.84.0/source/boost_1_84_0.tar.bz2 -o boost_1_84_0.tar.bz2 && tar --bzip2 -xf boost_1_84_0.tar.bz2 && mv boost_1_84_0/boost /usr/include/boost'] failed with code 1.

but that can get fixed in another PR.

For this PR what matters is that I can now do

python -m pip install --upgrade --verbose . && python -m pip install --upgrade --verbose .

without erroring out.

@matthewfeickert
Copy link
Member Author

cc @kawaho

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is everything that's needed, but directories named "build" and "_*" are the sorts of things that should be cleaned up.

You should also get a review from someone who is more qualified to verify it.

* While doing multiple rebuilds it is advisable to also clean up the
  state of the Git submodules with 'git clean -f', removing the OUTPUT
  directory at src/fastjet/_fastjet_core/ and the top level build directory
  is sufficient to do local development rebuilds --- i.e., the command
  'python -m pip install --upgrade .' can be run twice in a row without error.
@matthewfeickert
Copy link
Member Author

@rkansal47 unless you have any thoughts here I'm going to merge this at the end of the day to make @kawaho's development experience easier on PR #312.

@rkansal47
Copy link
Collaborator

rkansal47 commented Sep 12, 2024

Thanks @matthewfeickert! Does this resolve #140? (Sorry, I'm not able to test the re-build myself currently.)

EDIT: Looks like it should, so LGTM!

@rkansal47 rkansal47 merged commit 55262e5 into scikit-hep:main Sep 12, 2024
12 checks passed
@matthewfeickert matthewfeickert deleted the build/allow-for-rebuilds branch September 12, 2024 23:55
@matthewfeickert
Copy link
Member Author

matthewfeickert commented Sep 12, 2024

Ah good catch @rkansal47! I hadn't even noticed Issue #140 while thinking on Issue #279.

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.

Re-building from source during development
3 participants