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

Revert "Remove build product and update .gitignore to avoid picking it up again" #3947

Closed
wants to merge 2 commits into from

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Jul 20, 2017

This isn't correct as this file is build product. However, it isn't getting correctly generated and so we cannot build the master without it for now. Will find root cause and re-fix

This reverts commit 7d8d877.

Ralph Castain added 2 commits July 20, 2017 16:51
…t up again"

This isn't correct as this file is build product. However, it isn't getting correctly generated and so we cannot build the master without it for now. Will find root cause and re-fix

This reverts commit 7d8d877.

Signed-off-by: Ralph Castain <rhc@open-mpi.org>
…arball

Signed-off-by: Ralph Castain <rhc@open-mpi.org>
@ggouaillardet
Copy link
Contributor

opal/mca/hwloc/*/hwloc/include/private/autogen/config.h.in is automatically generated at autogen.pl time, so strictly speaking, it does not have to be in the git repo (unless we stop running autogen.sh in this directory).
now if you simply remove the file, the opal/mca/hwloc/*/hwloc/include/private/autogen directory becomes empty and it seems git simply removes it. consequently, autogen.sh will fail to create include/private/autogen/config.h.in since the directory does not exist.
there are several options here

  1. stop running autogen.sh
  2. leave opal/mca/hwloc/*/hwloc/include/private/autogen/config.h.in in the git repo
  3. create a opal/mca/hwloc/*/hwloc/include/private/autogen/README.txt that has the same content than opal/mca/hwloc/hwloc1116/hwloc/contrib/systemd/README.txt
Open MPI doesn't need this tree from hwloc.  But automake *requires*
that this directory has to be here.  So we have an empty directory
with a README in it, a) just to explain why it's here, and b) so that
hg clones won't delete the directory (because it's empty).

as far as i am concerned, i am fine running autogen.sh inside opal/mca/hwloc/hwloc2x/hwloc (and if we do not, then configure and Makefile.in must be added to the repo), so i like the third option best

Refs #3940

@artpol84 any thoughts ?

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 21, 2017

I'm working this right now - the integration is simply incorrect. It looks like it was taken from some dynamic component instead of copied from the current hwloc1116 integration. I'm fixing it, so hang tight.

@ggouaillardet
Copy link
Contributor

keep in mind that ompi "slurps" hwloc1x .m4 files and do its own configure logic.
it is not possible to slurp both hwloc1x and hwloc2x .m4 files (too many conflicts) so i simply integrated hwloc2x as a standalone component with its own autogen.sh and configure.ac

note an other option is to simply remove opal/mca/hwloc/hwloc2x (e.g. if you want hwloc v2, then use an external hwloc). this directory is only here for convenience, configure --with-hwloc=future and you do not need an external hwloc v2

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 21, 2017

yes, that was indeed the error - we never keep two copies of hwloc internally at the same time. This was what created the problem, and the workaround you did unfortunately breaks a few things. I think the best solution for tonight is to just revert your PR commit until we can properly integrate this.

@ggouaillardet
Copy link
Contributor

may i ask what #3302 did break ?
the issue that was initially reported in #3940 is about an extra file, and the same extra file is present in all branches under opal/mca/hwloc/hwloc1*/hwloc/include/private/autogen/config.h.in

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 21, 2017

It broke several downstream projects, which is why I'm working on it now. Just for future: you never ever put a non-tarball version of libevent or hwloc in our repository. Our code base isn't designed to handle that scenario.

@ggouaillardet
Copy link
Contributor

but opal/mca/hwloc/hwloc2x/hwloc was imported from a tarball, just like PMIx is !

from hwloc master, i
configure --enable-embedded-mode && make dist
and then i imported the tarball content
(full disclosure, i also manually removed hwloc_config_prefix[utils/netloc/scotch/Makefile from config/hwloc_internal.m4 so opal/mca/hwloc/hwloc2x/hwloc/autogen.sh can work)

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 21, 2017

well, you didn't wind up with a distribution tarball, my friend - it is very different

@ggouaillardet
Copy link
Contributor

humm, that is a bit too subtle for me right now.

anyway, as i stated earlier, opal/mca/hwloc/hwloc2x is only here so we can configure --with-hwloc=future. if this is causing more harms than good, and you are fine with an external hwloc, then you can simply get rid of it

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 21, 2017

Yeah, I'm just going to remove it and redo it. We want it internal for v3.1, but it has to be done correctly.

@rhc54 rhc54 closed this Jul 21, 2017
@rhc54 rhc54 deleted the topic/oops branch July 21, 2017 01:19
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.

None yet

2 participants