You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@nifi.apache.org by phrocker <gi...@git.apache.org> on 2017/11/09 15:30:57 UTC

[GitHub] nifi-minifi-cpp pull request #181: MINIFICPP-297: Remove Boost dependencies ...

GitHub user phrocker opened a pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/181

    MINIFICPP-297: Remove Boost dependencies since we have very little re…

    …liance on it
    
    MINIFICPP-297: Ensure posix compliance
    
    Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [ ] Is there a JIRA ticket associated with this PR? Is it referenced
         in the commit message?
    
    - [ ] Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [ ] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [ ] If applicable, have you updated the LICENSE file?
    - [ ] If applicable, have you updated the NOTICE file?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFICPP-297

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi-minifi-cpp/pull/181.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #181
    
----
commit 1e4fb83511482e8048989adc9ab6e3d5e8d74d66
Author: Marc Parisi <ph...@apache.org>
Date:   2017-11-08T19:03:22Z

    MINIFICPP-297: Remove Boost dependencies since we have very little reliance on it
    
    MINIFICPP-297: Ensure posix compliance

----


---

[GitHub] nifi-minifi-cpp pull request #181: MINIFICPP-297: Remove Boost dependencies ...

Posted by achristianson <gi...@git.apache.org>.

Github user achristianson commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/181#discussion_r150386373
  
    --- Diff: libminifi/include/utils/FileSystemUtils.h ---
    @@ -0,0 +1,122 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +#ifndef LIBMINIFI_INCLUDE_UTILS_FILESYSTEMUTILS_H_
    +#define LIBMINIFI_INCLUDE_UTILS_FILESYSTEMUTILS_H_
    +
    +#ifdef BOOST_VERSION
    +#include <boost/filesystem.hpp>
    +#else
    +#include <cstdlib>
    +#endif
    +#include <cstdio>
    +#include <unistd.h>
    +#include <fcntl.h>
    +#include "io/validation.h"
    +#include "Id.h"
    +#include "StringUtils.h"
    +#ifdef WIN32
    +#define stat _stat
    +#endif
    +
    +namespace org {
    +namespace apache {
    +namespace nifi {
    +namespace minifi {
    +namespace utils {
    +
    +/**
    + * Simple implementation of some file system utilities.
    + *
    + * unique_file is not a static implementation so that we can support scope driven temporary files.
    + */
    +class FileSystemUtils {
    + public:
    +
    +  FileSystemUtils() {
    +  }
    +
    +  ~FileSystemUtils() {
    +    for (auto file : unique_files_) {
    +      unlink(file.c_str());
    +    }
    +  }
    +  std::string unique_file(const std::string &location, bool keep = false) {
    +
    +    if (!IsNullOrEmpty(location)) {
    +      std::string file_name = location + "/" + non_repeating_string_generator_.generate();
    +      while (!verify_not_exist(file_name)) {
    +        file_name = location + "/" + non_repeating_string_generator_.generate();
    +      }
    +      if (!keep)
    +        unique_files_.push_back(file_name);
    +      return file_name;
    --- End diff --
    
    Return std::move()?


---

[GitHub] nifi-minifi-cpp issue #181: MINIFICPP-297: Remove Boost dependencies since w...

Posted by achristianson <gi...@git.apache.org>.

Github user achristianson commented on the issue:

    https://github.com/apache/nifi-minifi-cpp/pull/181
  
    @phrocker my thought is to definitely keep the RAII to have simpler/safer resource management. I see the unlink in the destructor. I would just tweak the naming & organization a bit. Now I see this is actually a unique *temporary* file session, so I would put that in the name. I wouldn't get rid of FileSystemUtils, but would create a nested class i.e. FileSystemUtils::TempFileManager. Usage like this:
    
    ```c++
    {
      FileSystemUtils::TempFileManager mgr;
      auto tmp_file = mgr.unique_file("/tmp", false);
      // Do something with tmp_file
    } // All tmp files created with mgr are cleaned up (RAII)
    ```


---